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

Trace DNS resolution in Netty 4.1 #4587

Merged
merged 3 commits into from
Nov 5, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
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 @@ -7,7 +7,7 @@

import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.hasClassesNamed;
import static io.opentelemetry.javaagent.extension.matcher.AgentElementMatchers.implementsInterface;
import static io.opentelemetry.javaagent.instrumentation.netty.v3_8.client.NettyClientSingletons.connectInstrumenter;
import static io.opentelemetry.javaagent.instrumentation.netty.v3_8.client.NettyClientSingletons.connectionInstrumenter;
import static net.bytebuddy.matcher.ElementMatchers.isMethod;
import static net.bytebuddy.matcher.ElementMatchers.named;
import static net.bytebuddy.matcher.ElementMatchers.returns;
Expand All @@ -19,7 +19,7 @@
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectRequest;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectionRequest;
import io.opentelemetry.javaagent.instrumentation.netty.v3_8.client.ConnectionListener;
import java.net.SocketAddress;
import net.bytebuddy.asm.Advice;
Expand Down Expand Up @@ -58,7 +58,7 @@ public static void onEnter(
@Advice.This Channel channel,
@Advice.Argument(0) SocketAddress remoteAddress,
@Advice.Local("otelParentContext") Context parentContext,
@Advice.Local("otelRequest") NettyConnectRequest request) {
@Advice.Local("otelRequest") NettyConnectionRequest request) {

parentContext = Java8BytecodeBridge.currentContext();
Span span = Java8BytecodeBridge.spanFromContext(parentContext);
Expand All @@ -73,24 +73,24 @@ public static void onEnter(
}
virtualField.set(channel, new NettyConnectionContext(parentContext));

request = NettyConnectRequest.create(remoteAddress);
request = NettyConnectionRequest.connect(remoteAddress);
}

@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") NettyConnectRequest request) {
@Advice.Local("otelRequest") NettyConnectionRequest request) {

if (request == null) {
return;
}

if (error != null) {
if (connectInstrumenter().shouldStart(parentContext, request)) {
Context context = connectInstrumenter().start(parentContext, request);
connectInstrumenter().end(context, request, null, error);
if (connectionInstrumenter().shouldStart(parentContext, request)) {
Context context = connectionInstrumenter().start(parentContext, request);
connectionInstrumenter().end(context, request, null, error);
}
} else {
channelFuture.addListener(new ConnectionListener(parentContext, request));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,29 +5,29 @@

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

import static io.opentelemetry.javaagent.instrumentation.netty.v3_8.client.NettyClientSingletons.connectInstrumenter;
import static io.opentelemetry.javaagent.instrumentation.netty.v3_8.client.NettyClientSingletons.connectionInstrumenter;

import io.opentelemetry.context.Context;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectRequest;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectionRequest;
import org.jboss.netty.channel.ChannelFuture;
import org.jboss.netty.channel.ChannelFutureListener;

public final class ConnectionListener implements ChannelFutureListener {

private final Context parentContext;
private final NettyConnectRequest request;
private final NettyConnectionRequest request;

public ConnectionListener(Context parentContext, NettyConnectRequest request) {
public ConnectionListener(Context parentContext, NettyConnectionRequest request) {
this.parentContext = parentContext;
this.request = request;
}

@Override
public void operationComplete(ChannelFuture future) {
Throwable cause = future.getCause();
if (cause != null && connectInstrumenter().shouldStart(parentContext, request)) {
Context context = connectInstrumenter().start(parentContext, request);
connectInstrumenter().end(context, request, future.getChannel(), cause);
if (cause != null && connectionInstrumenter().shouldStart(parentContext, request)) {
Context context = connectionInstrumenter().start(parentContext, request);
connectionInstrumenter().end(context, request, future.getChannel(), cause);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpClientMetrics;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanNameExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanStatusExtractor;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectRequest;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectionRequest;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyErrorHolder;
import io.opentelemetry.javaagent.instrumentation.netty.v3_8.HttpRequestAndChannel;
import org.jboss.netty.channel.Channel;
Expand All @@ -23,7 +23,7 @@ public final class NettyClientSingletons {
private static final String INSTRUMENTATION_NAME = "io.opentelemetry.netty-3.8";

private static final Instrumenter<HttpRequestAndChannel, HttpResponse> INSTRUMENTER;
private static final Instrumenter<NettyConnectRequest, Channel> CONNECT_INSTRUMENTER;
private static final Instrumenter<NettyConnectionRequest, Channel> CONNECTION_INSTRUMENTER;

static {
NettyHttpClientAttributesExtractor httpClientAttributesExtractor =
Expand All @@ -48,9 +48,9 @@ public final class NettyClientSingletons {

NettyConnectNetAttributesExtractor nettyConnectAttributesExtractor =
new NettyConnectNetAttributesExtractor();
CONNECT_INSTRUMENTER =
Instrumenter.<NettyConnectRequest, Channel>builder(
GlobalOpenTelemetry.get(), INSTRUMENTATION_NAME, rq -> "CONNECT")
CONNECTION_INSTRUMENTER =
Instrumenter.<NettyConnectionRequest, Channel>builder(
GlobalOpenTelemetry.get(), INSTRUMENTATION_NAME, NettyConnectionRequest::spanName)
.addAttributesExtractor(nettyConnectAttributesExtractor)
.addAttributesExtractor(
PeerServiceAttributesExtractor.create(nettyConnectAttributesExtractor))
Expand All @@ -64,8 +64,8 @@ public static Instrumenter<HttpRequestAndChannel, HttpResponse> instrumenter() {
return INSTRUMENTER;
}

public static Instrumenter<NettyConnectRequest, Channel> connectInstrumenter() {
return CONNECT_INSTRUMENTER;
public static Instrumenter<NettyConnectionRequest, Channel> connectionInstrumenter() {
return CONNECTION_INSTRUMENTER;
}

private NettyClientSingletons() {}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,19 +9,19 @@
import static io.opentelemetry.semconv.trace.attributes.SemanticAttributes.NetTransportValues.IP_UDP;

import io.opentelemetry.instrumentation.api.instrumenter.net.InetSocketAddressNetClientAttributesExtractor;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectRequest;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectionRequest;
import java.net.InetSocketAddress;
import java.net.SocketAddress;
import javax.annotation.Nullable;
import org.jboss.netty.channel.Channel;
import org.jboss.netty.channel.socket.DatagramChannel;

final class NettyConnectNetAttributesExtractor
extends InetSocketAddressNetClientAttributesExtractor<NettyConnectRequest, Channel> {
extends InetSocketAddressNetClientAttributesExtractor<NettyConnectionRequest, Channel> {

@Nullable
@Override
public InetSocketAddress getAddress(NettyConnectRequest request, @Nullable Channel channel) {
public InetSocketAddress getAddress(NettyConnectionRequest request, @Nullable Channel channel) {
SocketAddress remoteAddress = null;
if (channel != null) {
remoteAddress = channel.getRemoteAddress();
Expand All @@ -37,7 +37,7 @@ public InetSocketAddress getAddress(NettyConnectRequest request, @Nullable Chann
}

@Override
public String transport(NettyConnectRequest request, @Nullable Channel channel) {
public String transport(NettyConnectionRequest request, @Nullable Channel channel) {
return channel instanceof DatagramChannel ? IP_UDP : IP_TCP;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,15 +10,15 @@
import io.netty.util.concurrent.Future;
import io.netty.util.concurrent.GenericFutureListener;
import io.opentelemetry.context.Context;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectRequest;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectionRequest;

public class ConnectionCompleteListener implements GenericFutureListener<Future<Void>> {
private final NettyConnectInstrumenter instrumenter;
private final NettyConnectionInstrumenter instrumenter;
private final Context context;
private final NettyConnectRequest request;
private final NettyConnectionRequest request;

public ConnectionCompleteListener(
NettyConnectInstrumenter instrumenter, Context context, NettyConnectRequest request) {
NettyConnectionInstrumenter instrumenter, Context context, NettyConnectionRequest request) {
this.instrumenter = instrumenter;
this.context = context;
this.request = request;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanNameExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanStatusExtractor;
import io.opentelemetry.javaagent.instrumentation.netty.common.HttpRequestAndChannel;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectRequest;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectionRequest;

public final class NettyClientInstrumenterFactory {

Expand Down Expand Up @@ -46,12 +46,12 @@ public Instrumenter<HttpRequestAndChannel, HttpResponse> createHttpInstrumenter(
.newClientInstrumenter(new HttpRequestHeadersSetter());
}

public NettyConnectInstrumenter createConnectInstrumenter() {
public NettyConnectionInstrumenter createConnectionInstrumenter() {
NettyConnectNetAttributesExtractor netAttributesExtractor =
new NettyConnectNetAttributesExtractor();
Instrumenter<NettyConnectRequest, Channel> instrumenter =
Instrumenter.<NettyConnectRequest, Channel>builder(
GlobalOpenTelemetry.get(), instrumentationName, rq -> "CONNECT")
Instrumenter<NettyConnectionRequest, Channel> instrumenter =
Instrumenter.<NettyConnectionRequest, Channel>builder(
GlobalOpenTelemetry.get(), instrumentationName, NettyConnectionRequest::spanName)
Comment on lines +52 to +54
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I sort of see these as two different instrumenters, one for resolve and one for connect, but ok with this too

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

That was my first idea too, but then I noticed that they're basically the same; so I ended up with just reusing it and changing just the span name.

.addAttributesExtractor(netAttributesExtractor)
.addAttributesExtractor(PeerServiceAttributesExtractor.create(netAttributesExtractor))
.setTimeExtractors(
Expand All @@ -63,7 +63,7 @@ public NettyConnectInstrumenter createConnectInstrumenter() {
: SpanKindExtractor.alwaysClient());

return alwaysCreateConnectSpan
? new NettyConnectInstrumenterImpl(instrumenter)
: new NettyErrorOnlyConnectInstrumenter(instrumenter);
? new NettyConnectionInstrumenterImpl(instrumenter)
: new NettyErrorOnlyConnectionInstrumenter(instrumenter);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,17 +11,17 @@
import io.netty.channel.Channel;
import io.netty.channel.socket.DatagramChannel;
import io.opentelemetry.instrumentation.api.instrumenter.net.InetSocketAddressNetClientAttributesExtractor;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectRequest;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectionRequest;
import java.net.InetSocketAddress;
import java.net.SocketAddress;
import javax.annotation.Nullable;

final class NettyConnectNetAttributesExtractor
extends InetSocketAddressNetClientAttributesExtractor<NettyConnectRequest, Channel> {
extends InetSocketAddressNetClientAttributesExtractor<NettyConnectionRequest, Channel> {

@Nullable
@Override
public InetSocketAddress getAddress(NettyConnectRequest request, @Nullable Channel channel) {
public InetSocketAddress getAddress(NettyConnectionRequest request, @Nullable Channel channel) {
SocketAddress remoteAddress = null;
if (channel != null) {
remoteAddress = channel.remoteAddress();
Expand All @@ -37,7 +37,7 @@ public InetSocketAddress getAddress(NettyConnectRequest request, @Nullable Chann
}

@Override
public String transport(NettyConnectRequest request, @Nullable Channel channel) {
public String transport(NettyConnectionRequest request, @Nullable Channel channel) {
return channel instanceof DatagramChannel ? IP_UDP : IP_TCP;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -7,15 +7,15 @@

import io.netty.channel.Channel;
import io.opentelemetry.context.Context;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectRequest;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectionRequest;
import javax.annotation.Nullable;

public interface NettyConnectInstrumenter {
public interface NettyConnectionInstrumenter {

boolean shouldStart(Context parentContext, NettyConnectRequest request);
boolean shouldStart(Context parentContext, NettyConnectionRequest request);

Context start(Context parentContext, NettyConnectRequest request);
Context start(Context parentContext, NettyConnectionRequest request);

void end(
Context context, NettyConnectRequest request, Channel channel, @Nullable Throwable error);
Context context, NettyConnectionRequest request, Channel channel, @Nullable Throwable error);
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,30 +8,30 @@
import io.netty.channel.Channel;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectRequest;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectionRequest;
import javax.annotation.Nullable;

final class NettyConnectInstrumenterImpl implements NettyConnectInstrumenter {
final class NettyConnectionInstrumenterImpl implements NettyConnectionInstrumenter {

private final Instrumenter<NettyConnectRequest, Channel> instrumenter;
private final Instrumenter<NettyConnectionRequest, Channel> instrumenter;

NettyConnectInstrumenterImpl(Instrumenter<NettyConnectRequest, Channel> instrumenter) {
NettyConnectionInstrumenterImpl(Instrumenter<NettyConnectionRequest, Channel> instrumenter) {
this.instrumenter = instrumenter;
}

@Override
public boolean shouldStart(Context parentContext, NettyConnectRequest request) {
public boolean shouldStart(Context parentContext, NettyConnectionRequest request) {
return instrumenter.shouldStart(parentContext, request);
}

@Override
public Context start(Context parentContext, NettyConnectRequest request) {
public Context start(Context parentContext, NettyConnectionRequest request) {
return instrumenter.start(parentContext, request);
}

@Override
public void end(
Context context, NettyConnectRequest request, Channel channel, @Nullable Throwable error) {
Context context, NettyConnectionRequest request, Channel channel, @Nullable Throwable error) {
instrumenter.end(context, request, channel, error);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -8,31 +8,31 @@
import io.netty.channel.Channel;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectRequest;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyConnectionRequest;
import javax.annotation.Nullable;

final class NettyErrorOnlyConnectInstrumenter implements NettyConnectInstrumenter {
final class NettyErrorOnlyConnectionInstrumenter implements NettyConnectionInstrumenter {

private final Instrumenter<NettyConnectRequest, Channel> instrumenter;
private final Instrumenter<NettyConnectionRequest, Channel> instrumenter;

NettyErrorOnlyConnectInstrumenter(Instrumenter<NettyConnectRequest, Channel> instrumenter) {
NettyErrorOnlyConnectionInstrumenter(Instrumenter<NettyConnectionRequest, Channel> instrumenter) {
this.instrumenter = instrumenter;
}

@Override
public boolean shouldStart(Context parentContext, NettyConnectRequest request) {
public boolean shouldStart(Context parentContext, NettyConnectionRequest request) {
// the "real" check is done on end() anyway
return true;
}

@Override
public Context start(Context parentContext, NettyConnectRequest request) {
public Context start(Context parentContext, NettyConnectionRequest request) {
return parentContext;
}

@Override
public void end(
Context context, NettyConnectRequest request, Channel channel, @Nullable Throwable error) {
Context context, NettyConnectionRequest request, Channel channel, @Nullable Throwable error) {
if (error != null && instrumenter.shouldStart(context, request)) {
Context connectContext = instrumenter.start(context, request);
instrumenter.end(connectContext, request, channel, error);
Expand Down
Loading