-
Notifications
You must be signed in to change notification settings - Fork 850
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Addresses opencensus-shim integration #6875
Changes from all commits
0baeb3c
413664b
416b807
9a792c2
1e496b5
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,20 @@ | ||
plugins { | ||
id("otel.javaagent-instrumentation") | ||
} | ||
|
||
muzzle { | ||
pass { | ||
group.set("io.opentelemetry") | ||
module.set("opentelemetry-opencensus-shim") | ||
versions.set("[1.18.0-alpha,)") | ||
} | ||
} | ||
|
||
dependencies { | ||
compileOnly("io.opentelemetry:opentelemetry-opencensus-shim") | ||
compileOnly(project(":opentelemetry-api-shaded-for-instrumenting", configuration = "shadow")) | ||
|
||
testImplementation(project(":instrumentation:opencensus-shim:testing")) | ||
testInstrumentation(project(":instrumentation:opencensus-shim:testing")) | ||
testImplementation(project(":opentelemetry-api-shaded-for-instrumenting", configuration = "shadow")) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,138 @@ | ||
/* | ||
* Copyright The OpenTelemetry Authors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package io.opentelemetry.opencensusshim; | ||
|
||
import application.io.opentelemetry.api.trace.Span; | ||
import application.io.opentelemetry.context.Context; | ||
import application.io.opentelemetry.context.ContextKey; | ||
import application.io.opentelemetry.context.ImplicitContextKeyed; | ||
import application.io.opentelemetry.context.Scope; | ||
import com.google.errorprone.annotations.MustBeClosed; | ||
import java.lang.reflect.Field; | ||
import java.util.concurrent.Callable; | ||
import java.util.concurrent.Executor; | ||
import java.util.concurrent.ExecutorService; | ||
import java.util.concurrent.ScheduledExecutorService; | ||
import java.util.function.BiConsumer; | ||
import java.util.function.BiFunction; | ||
import java.util.function.Consumer; | ||
import java.util.function.Function; | ||
import java.util.function.Supplier; | ||
import javax.annotation.Nullable; | ||
|
||
public final class ContextExtractor implements Context { | ||
|
||
private static final Field otelSpanField; | ||
|
||
static { | ||
try { | ||
otelSpanField = OpenTelemetrySpanImpl.class.getDeclaredField("otelSpan"); | ||
} catch (NoSuchFieldException e) { | ||
throw new IllegalStateException(e); | ||
} | ||
otelSpanField.setAccessible(true); | ||
} | ||
|
||
private final Context ref; | ||
|
||
public ContextExtractor(Context appCtx) { | ||
this.ref = appCtx; | ||
} | ||
|
||
@Override | ||
public <V> Context with(ContextKey<V> k1, V v1) { | ||
if (v1 instanceof OpenTelemetrySpanImpl) { | ||
Span otelSpan = null; | ||
try { | ||
otelSpan = (Span) otelSpanField.get(v1); | ||
} catch (IllegalAccessException e) { | ||
throw new IllegalStateException(e); | ||
} | ||
return ref.with(otelSpan); | ||
} | ||
return ref.with(k1, v1); | ||
} | ||
|
||
@Override | ||
public Context with(ImplicitContextKeyed value) { | ||
if (value instanceof OpenTelemetrySpanImpl) { | ||
Span otelSpan = null; | ||
try { | ||
otelSpan = (Span) otelSpanField.get(value); | ||
} catch (IllegalAccessException e) { | ||
throw new IllegalStateException(e); | ||
} | ||
return ref.with(otelSpan); | ||
} | ||
return ref.with(value); | ||
} | ||
|
||
// | ||
// delegates | ||
// | ||
|
||
@Nullable | ||
@Override | ||
public <V> V get(ContextKey<V> key) { | ||
return ref.get(key); | ||
} | ||
|
||
@Override | ||
@MustBeClosed | ||
public Scope makeCurrent() { | ||
return ref.makeCurrent(); | ||
} | ||
|
||
@Override | ||
public Runnable wrap(Runnable runnable) { | ||
return ref.wrap(runnable); | ||
} | ||
|
||
@Override | ||
public <T> Callable<T> wrap(Callable<T> callable) { | ||
return ref.wrap(callable); | ||
} | ||
|
||
@Override | ||
public Executor wrap(Executor executor) { | ||
return ref.wrap(executor); | ||
} | ||
|
||
@Override | ||
public ExecutorService wrap(ExecutorService executor) { | ||
return ref.wrap(executor); | ||
} | ||
|
||
@Override | ||
public ScheduledExecutorService wrap(ScheduledExecutorService executor) { | ||
return ref.wrap(executor); | ||
} | ||
|
||
@Override | ||
public <T, U> Function<T, U> wrapFunction(Function<T, U> function) { | ||
return ref.wrapFunction(function); | ||
} | ||
|
||
@Override | ||
public <T, U, V> BiFunction<T, U, V> wrapFunction(BiFunction<T, U, V> function) { | ||
return ref.wrapFunction(function); | ||
} | ||
|
||
@Override | ||
public <T> Consumer<T> wrapConsumer(Consumer<T> consumer) { | ||
return ref.wrapConsumer(consumer); | ||
} | ||
|
||
@Override | ||
public <T, U> BiConsumer<T, U> wrapConsumer(BiConsumer<T, U> consumer) { | ||
return ref.wrapConsumer(consumer); | ||
} | ||
|
||
@Override | ||
public <T> Supplier<T> wrapSupplier(Supplier<T> supplier) { | ||
return ref.wrapSupplier(supplier); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,52 @@ | ||
/* | ||
* Copyright The OpenTelemetry Authors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package io.opentelemetry.opencensusshim; | ||
|
||
import static net.bytebuddy.matcher.ElementMatchers.isConstructor; | ||
import static net.bytebuddy.matcher.ElementMatchers.named; | ||
import static net.bytebuddy.matcher.ElementMatchers.takesArgument; | ||
|
||
import application.io.opentelemetry.context.Context; | ||
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; | ||
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer; | ||
import net.bytebuddy.asm.Advice; | ||
import net.bytebuddy.description.type.TypeDescription; | ||
import net.bytebuddy.matcher.ElementMatcher; | ||
|
||
/** | ||
* Doctors the {@link OpenTelemetryCtx} constructor to provide a delegating wrapper for the provided | ||
* {@link Context} which very specifically singles out the {@link OpenTelemetrySpanImpl} shortcoming | ||
* upon calling the {@link Context#with} methods. | ||
* | ||
* <p>The "with" methods then extract the internal otel span and pass it along for processing as | ||
* normal as, sadly, the Java Agent instrumentations all <i>require</i> the agent-generated | ||
* <i>instance</i>: interface conformance is simply not enough when the java agent is in the mix as | ||
* it uses data stored in specialized instances of {@link | ||
* application.io.opentelemetry.api.trace.Span} to perform its duties. | ||
*/ | ||
public class OpenTelemetryCtxInstrumentation implements TypeInstrumentation { | ||
@Override | ||
public ElementMatcher<TypeDescription> typeMatcher() { | ||
return named("io.opentelemetry.opencensusshim.OpenTelemetryCtx"); | ||
} | ||
|
||
@Override | ||
public void transform(TypeTransformer transformer) { | ||
transformer.applyAdviceToMethod( | ||
isConstructor() | ||
.and(takesArgument(0, named("application.io.opentelemetry.context.Context"))), | ||
OpenTelemetryCtxInstrumentation.class.getName() + "$HandleConstruction"); | ||
} | ||
|
||
@SuppressWarnings({"unused", "OtelPrivateConstructorForUtilityClass"}) | ||
public static class HandleConstruction { | ||
|
||
@Advice.OnMethodEnter(suppress = Throwable.class) | ||
public static void constructor(@Advice.Argument(value = 0, readOnly = false) Context context) { | ||
context = new ContextExtractor(context); | ||
} | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,31 @@ | ||
/* | ||
* Copyright The OpenTelemetry Authors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package io.opentelemetry.opencensusshim; | ||
|
||
import static java.util.Collections.singletonList; | ||
|
||
import com.google.auto.service.AutoService; | ||
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule; | ||
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation; | ||
import java.util.List; | ||
|
||
@AutoService(InstrumentationModule.class) | ||
public class OpencensusShimInstrumentationModule extends InstrumentationModule { | ||
|
||
public OpencensusShimInstrumentationModule() { | ||
super("opencensus-shim"); | ||
} | ||
|
||
@Override | ||
public List<TypeInstrumentation> typeInstrumentations() { | ||
return singletonList(new OpenTelemetryCtxInstrumentation()); | ||
} | ||
|
||
@Override | ||
public boolean isHelperClass(String className) { | ||
return className.equals("io.opentelemetry.opencensusshim.ContextExtractor"); | ||
} | ||
Comment on lines
+27
to
+30
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. OH I think I'm piecing that together now -- I don't think I ever knew that was required to make that work. I tried to move this as you suggested, but unfortunately, this one's a bit of a pickle: the |
||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
/* | ||
* Copyright The OpenTelemetry Authors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package io.opentelemetry.opencensusshim; | ||
|
||
import io.opentelemetry.instrumentation.testing.junit.AgentInstrumentationExtension; | ||
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; | ||
import org.junit.jupiter.api.extension.RegisterExtension; | ||
|
||
public class OpenCensusShimJavaAgentTest extends AbstractOpenCensusShimTest { | ||
|
||
@RegisterExtension | ||
static final InstrumentationExtension testing = AgentInstrumentationExtension.create(); | ||
|
||
@Override | ||
protected InstrumentationExtension testing() { | ||
return testing; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,10 @@ | ||
plugins { | ||
id("otel.library-instrumentation") | ||
id("otel.nullaway-conventions") | ||
} | ||
|
||
dependencies { | ||
api("io.opentelemetry:opentelemetry-opencensus-shim") | ||
|
||
api(project(":testing-common")) | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,58 @@ | ||
/* | ||
* Copyright The OpenTelemetry Authors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package io.opentelemetry.opencensusshim; | ||
|
||
import static io.opentelemetry.sdk.testing.assertj.OpenTelemetryAssertions.equalTo; | ||
|
||
import io.opencensus.trace.AttributeValue; | ||
import io.opencensus.trace.Tracing; | ||
import io.opentelemetry.api.common.AttributeKey; | ||
import io.opentelemetry.api.trace.Span; | ||
import io.opentelemetry.api.trace.SpanKind; | ||
import io.opentelemetry.api.trace.Tracer; | ||
import io.opentelemetry.context.Scope; | ||
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; | ||
import org.junit.jupiter.api.Test; | ||
|
||
public abstract class AbstractOpenCensusShimTest { | ||
|
||
protected abstract InstrumentationExtension testing(); | ||
|
||
@Test | ||
void testCrossOtelOcBoundary() { | ||
Tracer tracer = testing().getOpenTelemetry().getTracer("test"); | ||
Span span = tracer.spanBuilder("test-span").setSpanKind(SpanKind.INTERNAL).startSpan(); | ||
Scope scope = span.makeCurrent(); | ||
try { | ||
io.opencensus.trace.Tracer ocTracer = Tracing.getTracer(); | ||
io.opencensus.trace.Span internal = ocTracer.spanBuilder("internal").startSpan(); | ||
io.opencensus.common.Scope ocScope = ocTracer.withSpan(internal); | ||
try { | ||
ocTracer | ||
.getCurrentSpan() | ||
.putAttribute("internal-only", AttributeValue.booleanAttributeValue(true)); | ||
} finally { | ||
ocScope.close(); | ||
} | ||
internal.end(); | ||
} finally { | ||
scope.close(); | ||
} | ||
span.end(); | ||
|
||
testing() | ||
.waitAndAssertTraces( | ||
traceAssert -> | ||
traceAssert.hasSpansSatisfyingExactly( | ||
spanAssert -> spanAssert.hasName("test-span").hasNoParent(), | ||
spanAssert -> | ||
spanAssert | ||
.hasName("internal") | ||
.hasParentSpanId(span.getSpanContext().getSpanId()) | ||
.hasAttributesSatisfyingExactly( | ||
equalTo(AttributeKey.booleanKey("internal-only"), true)))); | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,21 @@ | ||
/* | ||
* Copyright The OpenTelemetry Authors | ||
* SPDX-License-Identifier: Apache-2.0 | ||
*/ | ||
|
||
package io.opentelemetry.opencensusshim; | ||
|
||
import io.opentelemetry.instrumentation.testing.junit.InstrumentationExtension; | ||
import io.opentelemetry.instrumentation.testing.junit.LibraryInstrumentationExtension; | ||
import org.junit.jupiter.api.extension.RegisterExtension; | ||
|
||
public class OpenCensusShimRegressionTest extends AbstractOpenCensusShimTest { | ||
|
||
@RegisterExtension | ||
static final InstrumentationExtension testing = LibraryInstrumentationExtension.create(); | ||
|
||
@Override | ||
protected InstrumentationExtension testing() { | ||
return testing; | ||
} | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -488,6 +488,9 @@ hideFromDependabot(":instrumentation:vibur-dbcp-11.0:library") | |
hideFromDependabot(":instrumentation:vibur-dbcp-11.0:testing") | ||
hideFromDependabot(":instrumentation:wicket-8.0:javaagent") | ||
|
||
include(":instrumentation:opencensus-shim:javaagent") | ||
include(":instrumentation:opencensus-shim:testing") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. since there will only ever be There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That makes sense, but is there a way to test the "regression" test (should be renamed baseline maybe?) as a library? I didn't see a way to run some tests against the library plugin ( |
||
|
||
// benchmark | ||
include(":benchmark-overhead-jmh") | ||
include(":benchmark-jfr-analyzer") | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.