Skip to content
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

make netty-* indy compatible #11559

Merged
merged 36 commits into from
Aug 22, 2024
Merged
Show file tree
Hide file tree
Changes from 29 commits
Commits
Show all changes
36 commits
Select commit Hold shift + click to select a range
80e04d3
netty common classloader
SylvainJuge Jun 12, 2024
b2d1c4c
introduce NettyScope for 4.x
SylvainJuge Jun 12, 2024
6cf99fd
refactor netty scope
SylvainJuge Jun 12, 2024
62745a9
add NettyScope for 3.8
SylvainJuge Jun 12, 2024
cefc148
make things private final
SylvainJuge Jun 12, 2024
fd3e0c9
return value
SylvainJuge Jun 12, 2024
701caf0
wip call depth
SylvainJuge Jun 12, 2024
aba151e
read/write arguments
SylvainJuge Jun 12, 2024
1bc40a4
wip but stil broken
SylvainJuge Jun 12, 2024
50b2f76
fix typo
SylvainJuge Jun 13, 2024
075fe14
fix AssignReturned with non inlined advices
SylvainJuge Jun 13, 2024
df7c853
restore working state + todos
SylvainJuge Jun 13, 2024
77e687a
fix formatting
SylvainJuge Jun 13, 2024
4c3396f
Merge branch 'main' of github.com:open-telemetry/opentelemetry-java-i…
SylvainJuge Jun 20, 2024
a72619f
wip
SylvainJuge Jun 21, 2024
e79ddbf
return array when needed + note for investigation
SylvainJuge Jun 21, 2024
ec823b4
Prevent advice transformations on advices already using @Advice.Assig…
JonasKunz Jun 24, 2024
8ca0cad
Merge branch 'main' of github.com:open-telemetry/opentelemetry-java-i…
SylvainJuge Jun 24, 2024
29b2d9d
spotless
SylvainJuge Jun 24, 2024
95b0f59
migrate finagle-http
SylvainJuge Jun 25, 2024
a210666
spring cloud gateway + webflux
SylvainJuge Jun 26, 2024
18a1c49
migrate ratpack
SylvainJuge Jun 26, 2024
f6ff56d
spotless
SylvainJuge Jun 26, 2024
ee2ae1b
Merge branch 'main' of github.com:open-telemetry/opentelemetry-java-i…
SylvainJuge Jun 28, 2024
06c90e4
add support for AssignReturned.ToAllArguments
SylvainJuge Jul 2, 2024
02c6a5b
Merge branch 'main' of github.com:open-telemetry/opentelemetry-java-i…
SylvainJuge Aug 14, 2024
10a3427
simplify advices
SylvainJuge Aug 14, 2024
9acdd70
Merge branch 'main' of github.com:open-telemetry/opentelemetry-java-i…
SylvainJuge Aug 19, 2024
f6d0233
remove dynamic assigner when we can
SylvainJuge Aug 19, 2024
06e07dc
Merge branch 'main' of github.com:open-telemetry/opentelemetry-java-i…
SylvainJuge Aug 20, 2024
b42189b
avoid Object in advice return value
SylvainJuge Aug 20, 2024
09e4d77
add comments from review
SylvainJuge Aug 21, 2024
cb8c5b8
add todo
SylvainJuge Aug 21, 2024
3da4520
Merge branch 'main' of github.com:open-telemetry/opentelemetry-java-i…
SylvainJuge Aug 21, 2024
94f6a28
Apply suggestions from code review
SylvainJuge Aug 22, 2024
2777b30
add AsScalar comment
SylvainJuge Aug 22, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,16 +34,16 @@ public void transform(TypeTransformer transformer) {
public static class WriteAdvice {

@Advice.OnMethodEnter(suppress = Throwable.class)
public static void methodEnter(@Advice.Local("otelScope") Scope scope) {
public static Scope methodEnter() {
Option<Context> ref = Helpers.CONTEXT_LOCAL.apply();
if (ref.isDefined()) {
scope = ref.get().makeCurrent();
return ref.get().makeCurrent();
}
return null;
}

@Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class)
public static void methodExit(
@Advice.Local("otelScope") Scope scope, @Advice.Thrown Throwable thrown) {
public static void methodExit(@Advice.Enter Scope scope, @Advice.Thrown Throwable thrown) {
if (scope != null) {
scope.close();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,13 @@
import com.google.auto.service.AutoService;
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule;
import java.util.Arrays;
import java.util.List;

@AutoService(InstrumentationModule.class)
public class FinagleHttpInstrumentationModule extends InstrumentationModule {
public class FinagleHttpInstrumentationModule extends InstrumentationModule
implements ExperimentalInstrumentationModule {

public FinagleHttpInstrumentationModule() {
super("finagle-http", "finagle-http-23.11");
Expand All @@ -27,9 +29,16 @@ public List<TypeInstrumentation> typeInstrumentations() {
}

@Override
public boolean isIndyModule() {
// injects helpers to access package-private members
return false;
public String getModuleGroup() {
// relies on netty and needs access to common netty instrumentation classes
return "netty";
}

@Override
public List<String> injectedClassNames() {
return Arrays.asList(
SylvainJuge marked this conversation as resolved.
Show resolved Hide resolved
"com.twitter.finagle.ChannelTransportHelpers",
"io.netty.channel.OpenTelemetryChannelInitializerDelegate");
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,19 +42,19 @@ public void transform(TypeTransformer transformer) {
public static class InitServerAdvice {

@Advice.OnMethodExit
public static void handleExit(
@Advice.Return(readOnly = false) ChannelInitializer<Channel> initializer) {
initializer = Helpers.wrapServer(initializer);
@Advice.AssignReturned.ToReturned
public static Object handleExit(@Advice.Return ChannelInitializer<Channel> initializer) {
SylvainJuge marked this conversation as resolved.
Show resolved Hide resolved
return Helpers.wrapServer(initializer);
}
}

@SuppressWarnings("unused")
public static class InitClientAdvice {

@Advice.OnMethodExit
public static void handleExit(
@Advice.Return(readOnly = false) ChannelInitializer<Channel> initializer) {
initializer = Helpers.wrapClient(initializer);
@Advice.AssignReturned.ToReturned
public static Object handleExit(@Advice.Return ChannelInitializer<Channel> initializer) {
SylvainJuge marked this conversation as resolved.
Show resolved Hide resolved
return Helpers.wrapClient(initializer);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.asm.Advice.AssignReturned.ToArguments.ToArgument;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;

Expand All @@ -35,13 +36,14 @@ public void transform(TypeTransformer transformer) {
public static class WrapRunnableAdvice {

@Advice.OnMethodEnter(suppress = Throwable.class)
public static void wrap(@Advice.Argument(value = 0, readOnly = false) Runnable task) {
@Advice.AssignReturned.ToArguments(@ToArgument(0))
public static Object wrap(@Advice.Argument(value = 0) Runnable task) {
SylvainJuge marked this conversation as resolved.
Show resolved Hide resolved
if (task == null) {
return;
return null;
}

Context context = Java8BytecodeBridge.currentContext();
task = ContextPropagatingRunnable.propagateContext(task, context);
return ContextPropagatingRunnable.propagateContext(task, context);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.asm.Advice.AssignReturned.ToArguments.ToArgument;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;
import scala.Function1;
Expand All @@ -34,13 +35,13 @@ public void transform(TypeTransformer transformer) {
public static class WrapFunctionAdvice {

@Advice.OnMethodEnter(suppress = Throwable.class)
public static void wrap(
@Advice.Argument(value = 1, readOnly = false) Function1<?, ?> function1) {
@Advice.AssignReturned.ToArguments(@ToArgument(1))
public static Object wrap(@Advice.Argument(value = 1) Function1<?, ?> function1) {
SylvainJuge marked this conversation as resolved.
Show resolved Hide resolved
if (function1 == null) {
return;
return null;
}

function1 = Function1Wrapper.wrap(function1);
return Function1Wrapper.wrap(function1);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,22 @@
import com.google.auto.service.AutoService;
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule;
import java.util.List;

@AutoService(InstrumentationModule.class)
public class TwitterUtilCoreInstrumentationModule extends InstrumentationModule {
public class TwitterUtilCoreInstrumentationModule extends InstrumentationModule
implements ExperimentalInstrumentationModule {

public TwitterUtilCoreInstrumentationModule() {
super("twitter-util-core");
}

@Override
public String getModuleGroup() {
return "netty";
}

@Override
public List<TypeInstrumentation> typeInstrumentations() {
return asList(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,55 +56,54 @@ public void transform(TypeTransformer transformer) {
public static class ChannelConnectAdvice {

@Advice.OnMethodEnter(suppress = Throwable.class)
public static void onEnter(
@Advice.This Channel channel,
@Advice.Argument(0) SocketAddress remoteAddress,
@Advice.Local("otelParentContext") Context parentContext,
@Advice.Local("otelRequest") NettyConnectionRequest request,
@Advice.Local("otelTimer") Timer timer) {

parentContext = Java8BytecodeBridge.currentContext();
public static NettyScope onEnter(
@Advice.This Channel channel, @Advice.Argument(0) SocketAddress remoteAddress) {

Context parentContext = Java8BytecodeBridge.currentContext();
Span span = Java8BytecodeBridge.spanFromContext(parentContext);
if (!span.getSpanContext().isValid()) {
return;
return null;
trask marked this conversation as resolved.
Show resolved Hide resolved
}

VirtualField<Channel, NettyConnectionContext> virtualField =
VirtualField.find(Channel.class, NettyConnectionContext.class);
if (virtualField.get(channel) != null) {
return;
return null;
}
virtualField.set(channel, new NettyConnectionContext(parentContext));

request = NettyConnectionRequest.connect(remoteAddress);
timer = Timer.start();
NettyConnectionRequest request = NettyConnectionRequest.connect(remoteAddress);
Timer timer = Timer.start();

return new NettyScope(parentContext, request, timer);
}

@Advice.OnMethodExit(suppress = Throwable.class, onThrowable = Throwable.class)
public static void onExit(
@Advice.Return ChannelFuture channelFuture,
@Advice.Thrown Throwable error,
@Advice.Local("otelParentContext") Context parentContext,
@Advice.Local("otelRequest") NettyConnectionRequest request,
@Advice.Local("otelTimer") Timer timer) {
@Advice.Enter NettyScope nettyScope) {

if (request == null) {
if (nettyScope == null) {
return;
}

if (error != null) {
if (connectionInstrumenter().shouldStart(parentContext, request)) {
if (connectionInstrumenter()
.shouldStart(nettyScope.getParentContext(), nettyScope.getRequest())) {
InstrumenterUtil.startAndEnd(
connectionInstrumenter(),
parentContext,
request,
nettyScope.getParentContext(),
nettyScope.getRequest(),
null,
error,
timer.startTime(),
timer.now());
nettyScope.getTimer().startTime(),
nettyScope.getTimer().now());
}
} else {
channelFuture.addListener(new ConnectionListener(parentContext, request, timer));
channelFuture.addListener(
new ConnectionListener(
nettyScope.getParentContext(), nettyScope.getRequest(), nettyScope.getTimer()));
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,25 +51,25 @@ public void transform(TypeTransformer transformer) {
public static class ChannelPipelineAdd2ArgsAdvice {

@Advice.OnMethodEnter
public static void checkDepth(
@Advice.This ChannelPipeline pipeline,
@Advice.Argument(1) ChannelHandler handler,
@Advice.Local("otelCallDepth") CallDepth callDepth) {
public static CallDepth checkDepth(
@Advice.This ChannelPipeline pipeline, @Advice.Argument(1) ChannelHandler handler) {
// Pipelines are created once as a factory and then copied multiple times using the same add
// methods as we are hooking. If our handler has already been added we need to remove it so we
// don't end up with duplicates (this throws an exception)
if (pipeline.get(handler.getClass().getName()) != null) {
pipeline.remove(handler.getClass().getName());
}
callDepth = CallDepth.forClass(ChannelPipeline.class);
CallDepth callDepth = CallDepth.forClass(ChannelPipeline.class);
callDepth.getAndIncrement();
return callDepth;
}

@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void addHandler(
@Advice.This ChannelPipeline pipeline,
@Advice.Argument(1) ChannelHandler handler,
@Advice.Local("otelCallDepth") CallDepth callDepth) {
@Advice.Enter CallDepth callDepth) {

if (callDepth.decrementAndGet() > 0) {
return;
}
Expand All @@ -82,29 +82,28 @@ public static void addHandler(
public static class ChannelPipelineAdd3ArgsAdvice {

@Advice.OnMethodEnter
public static void checkDepth(
@Advice.This ChannelPipeline pipeline,
@Advice.Argument(2) ChannelHandler handler,
@Advice.Local("otelCallDepth") CallDepth callDepth) {
public static CallDepth checkDepth(
@Advice.This ChannelPipeline pipeline, @Advice.Argument(2) ChannelHandler handler) {
// Pipelines are created once as a factory and then copied multiple times using the same add
// methods as we are hooking. If our handler has already been added we need to remove it so we
// don't end up with duplicates (this throws an exception)
if (pipeline.get(handler.getClass().getName()) != null) {
pipeline.remove(handler.getClass().getName());
}
callDepth = CallDepth.forClass(ChannelPipeline.class);
CallDepth callDepth = CallDepth.forClass(ChannelPipeline.class);
callDepth.getAndIncrement();
return callDepth;
}

@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void addHandler(
@Advice.This ChannelPipeline pipeline,
@Advice.Argument(2) ChannelHandler handler,
@Advice.Local("otelCallDepth") CallDepth callDepth) {
@Advice.Enter CallDepth callDepth) {

if (callDepth.decrementAndGet() > 0) {
return;
}

ChannelPipelineUtil.wrapHandler(pipeline, handler);
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@
import com.google.auto.service.AutoService;
import io.opentelemetry.javaagent.extension.instrumentation.InstrumentationModule;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.internal.ExperimentalInstrumentationModule;
import java.util.List;
import net.bytebuddy.matcher.ElementMatcher;

@AutoService(InstrumentationModule.class)
public class NettyInstrumentationModule extends InstrumentationModule {
public class NettyInstrumentationModule extends InstrumentationModule
implements ExperimentalInstrumentationModule {
public NettyInstrumentationModule() {
super("netty", "netty-3.8");
}
Expand All @@ -25,6 +27,11 @@ public ElementMatcher.Junction<ClassLoader> classLoaderMatcher() {
return hasClassesNamed("org.jboss.netty.handler.codec.http.HttpMessage");
}

@Override
public String getModuleGroup() {
return "netty";
}

@Override
public List<TypeInstrumentation> typeInstrumentations() {
return asList(
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/

package io.opentelemetry.javaagent.instrumentation.netty.v3_8;

import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.netty.common.internal.NettyConnectionRequest;
import io.opentelemetry.instrumentation.netty.common.internal.Timer;

/** Container used to carry state between enter and exit advices */
public class NettyScope {

private final Context parentContext;
private final NettyConnectionRequest request;
private final Timer timer;

public NettyScope(Context parentContext, NettyConnectionRequest request, Timer timer) {
this.parentContext = parentContext;
this.request = request;
this.timer = timer;
}

public Context getParentContext() {
return parentContext;
}

public NettyConnectionRequest getRequest() {
return request;
}

public Timer getTimer() {
return timer;
}
}
Loading
Loading