Combine two netty context attributes into one (#9160)

This commit is contained in:
Trask Stalnaker 2023-08-10 03:39:25 -07:00 committed by GitHub
parent 06fdceaddc
commit 849e0d5229
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
9 changed files with 89 additions and 60 deletions

View File

@ -17,6 +17,7 @@ import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.netty.common.internal.NettyErrorHolder;
import io.opentelemetry.instrumentation.netty.v4.common.HttpRequestAndChannel;
import io.opentelemetry.instrumentation.netty.v4_1.internal.AttributeKeys;
import io.opentelemetry.instrumentation.netty.v4_1.internal.ServerContext;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import java.util.Deque;
@ -60,10 +61,10 @@ public class AbstractChannelHandlerContextInstrumentation implements TypeInstrum
instrumenter().end(clientContext, request, null, throwable);
return;
}
Deque<Context> contexts = ctx.channel().attr(AttributeKeys.SERVER_CONTEXT).get();
Context serverContext = contexts != null ? contexts.peekFirst() : null;
Deque<ServerContext> serverContexts = ctx.channel().attr(AttributeKeys.SERVER_CONTEXT).get();
ServerContext serverContext = serverContexts != null ? serverContexts.peekFirst() : null;
if (serverContext != null) {
NettyErrorHolder.set(serverContext, throwable);
NettyErrorHolder.set(serverContext.context(), throwable);
}
}
}

View File

@ -7,5 +7,8 @@ dependencies {
implementation(project(":instrumentation:netty:netty-4-common:library"))
implementation(project(":instrumentation:netty:netty-common:library"))
compileOnly("com.google.auto.value:auto-value-annotations")
annotationProcessor("com.google.auto.value:auto-value")
testImplementation(project(":instrumentation:netty:netty-4.1:testing"))
}

View File

@ -18,7 +18,7 @@ public final class AttributeKeys {
// this is the context that has the server span
//
// note: this attribute key is also used by ratpack instrumentation
public static final AttributeKey<Deque<Context>> SERVER_CONTEXT =
public static final AttributeKey<Deque<ServerContext>> SERVER_CONTEXT =
AttributeKey.valueOf(AttributeKeys.class, "server-context");
public static final AttributeKey<Context> CLIENT_CONTEXT =

View File

@ -0,0 +1,31 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.instrumentation.netty.v4_1.internal;
import com.google.auto.value.AutoValue;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.netty.v4.common.HttpRequestAndChannel;
/**
* A tuple of an {@link Context} and a {@link HttpRequestAndChannel}.
*
* <p>This class is internal and is hence not for public use. Its APIs are unstable and can change
* at any time.
*/
@AutoValue
public abstract class ServerContext {
/** Create a new {@link ServerContext}. */
public static ServerContext create(Context context, HttpRequestAndChannel request) {
return new AutoValue_ServerContext(context, request);
}
/** Returns the {@link Context}. */
public abstract Context context();
/** Returns the {@link HttpRequestAndChannel}. */
public abstract HttpRequestAndChannel request();
}

View File

@ -17,6 +17,7 @@ import io.opentelemetry.context.Scope;
import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.instrumentation.netty.v4.common.HttpRequestAndChannel;
import io.opentelemetry.instrumentation.netty.v4_1.internal.AttributeKeys;
import io.opentelemetry.instrumentation.netty.v4_1.internal.ServerContext;
import java.util.ArrayDeque;
import java.util.Deque;
@ -26,9 +27,6 @@ import java.util.Deque;
*/
public class HttpServerRequestTracingHandler extends ChannelInboundHandlerAdapter {
static final AttributeKey<Deque<HttpRequestAndChannel>> HTTP_SERVER_REQUEST =
AttributeKey.valueOf(HttpServerRequestTracingHandler.class, "http-server-request");
private final Instrumenter<HttpRequestAndChannel, HttpResponse> instrumenter;
public HttpServerRequestTracingHandler(
@ -39,14 +37,14 @@ public class HttpServerRequestTracingHandler extends ChannelInboundHandlerAdapte
@Override
public void channelRead(ChannelHandlerContext ctx, Object msg) throws Exception {
Channel channel = ctx.channel();
Deque<Context> contexts = getOrCreate(channel, AttributeKeys.SERVER_CONTEXT);
Deque<ServerContext> serverContexts = getOrCreate(channel, AttributeKeys.SERVER_CONTEXT);
if (!(msg instanceof HttpRequest)) {
Context serverContext = contexts.peekLast();
ServerContext serverContext = serverContexts.peekLast();
if (serverContext == null) {
super.channelRead(ctx, msg);
} else {
try (Scope ignored = serverContext.makeCurrent()) {
try (Scope ignored = serverContext.context().makeCurrent()) {
super.channelRead(ctx, msg);
}
}
@ -61,16 +59,15 @@ public class HttpServerRequestTracingHandler extends ChannelInboundHandlerAdapte
}
Context context = instrumenter.start(parentContext, request);
contexts.addLast(context);
Deque<HttpRequestAndChannel> requests = getOrCreate(channel, HTTP_SERVER_REQUEST);
requests.addLast(request);
serverContexts.addLast(ServerContext.create(context, request));
try (Scope ignored = context.makeCurrent()) {
super.channelRead(ctx, msg);
// the span is ended normally in HttpServerResponseTracingHandler
} catch (Throwable throwable) {
// make sure to remove the server context on end() call
instrumenter.end(contexts.removeLast(), requests.removeLast(), null, throwable);
ServerContext serverContext = serverContexts.removeLast();
instrumenter.end(serverContext.context(), serverContext.request(), null, throwable);
throw throwable;
}
}
@ -78,20 +75,16 @@ public class HttpServerRequestTracingHandler extends ChannelInboundHandlerAdapte
@Override
public void channelInactive(ChannelHandlerContext ctx) {
// connection was closed, close all remaining requests
Attribute<Deque<Context>> contextAttr = ctx.channel().attr(AttributeKeys.SERVER_CONTEXT);
Deque<Context> contexts = contextAttr.get();
Attribute<Deque<HttpRequestAndChannel>> requestAttr = ctx.channel().attr(HTTP_SERVER_REQUEST);
Deque<HttpRequestAndChannel> requests = requestAttr.get();
Attribute<Deque<ServerContext>> contextAttr = ctx.channel().attr(AttributeKeys.SERVER_CONTEXT);
Deque<ServerContext> serverContexts = contextAttr.get();
if (contexts == null || requests == null) {
if (serverContexts == null) {
return;
}
while (!contexts.isEmpty() || !requests.isEmpty()) {
Context context = contexts.pollFirst();
HttpRequestAndChannel request = requests.pollFirst();
instrumenter.end(context, request, null, null);
ServerContext serverContext;
while ((serverContext = serverContexts.pollFirst()) != null) {
instrumenter.end(serverContext.context(), serverContext.request(), null, null);
}
}

View File

@ -5,8 +5,6 @@
package io.opentelemetry.instrumentation.netty.v4_1.internal.server;
import static io.opentelemetry.instrumentation.netty.v4_1.internal.server.HttpServerRequestTracingHandler.HTTP_SERVER_REQUEST;
import io.netty.channel.ChannelFuture;
import io.netty.channel.ChannelHandlerContext;
import io.netty.channel.ChannelOutboundHandlerAdapter;
@ -22,6 +20,7 @@ import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.instrumentation.netty.common.internal.NettyErrorHolder;
import io.opentelemetry.instrumentation.netty.v4.common.HttpRequestAndChannel;
import io.opentelemetry.instrumentation.netty.v4_1.internal.AttributeKeys;
import io.opentelemetry.instrumentation.netty.v4_1.internal.ServerContext;
import java.util.Deque;
import javax.annotation.Nullable;
@ -46,17 +45,15 @@ public class HttpServerResponseTracingHandler extends ChannelOutboundHandlerAdap
@Override
public void write(ChannelHandlerContext ctx, Object msg, ChannelPromise prm) throws Exception {
Attribute<Deque<Context>> contextAttr = ctx.channel().attr(AttributeKeys.SERVER_CONTEXT);
Attribute<Deque<ServerContext>> serverContextAttr =
ctx.channel().attr(AttributeKeys.SERVER_CONTEXT);
Deque<Context> contexts = contextAttr.get();
Context context = contexts != null ? contexts.peekFirst() : null;
if (context == null) {
Deque<ServerContext> serverContexts = serverContextAttr.get();
ServerContext serverContext = serverContexts != null ? serverContexts.peekFirst() : null;
if (serverContext == null) {
super.write(ctx, msg, prm);
return;
}
Attribute<Deque<HttpRequestAndChannel>> requestAttr = ctx.channel().attr(HTTP_SERVER_REQUEST);
Deque<HttpRequestAndChannel> requests = requestAttr.get();
HttpRequestAndChannel request = requests != null ? requests.peekFirst() : null;
ChannelPromise writePromise;
@ -73,35 +70,39 @@ public class HttpServerResponseTracingHandler extends ChannelOutboundHandlerAdap
// Going to finish the span after the write of the last content finishes.
if (msg instanceof FullHttpResponse) {
// Headers and body all sent together, we have the response information in the msg.
beforeCommitHandler.handle(context, (HttpResponse) msg);
contexts.removeFirst();
requests.removeFirst();
beforeCommitHandler.handle(serverContext.context(), (HttpResponse) msg);
serverContexts.removeFirst();
writePromise.addListener(
future -> end(context, request, (FullHttpResponse) msg, writePromise));
future ->
end(
serverContext.context(),
serverContext.request(),
(FullHttpResponse) msg,
writePromise));
} else {
// Body sent after headers. We stored the response information in the context when
// encountering HttpResponse (which was not FullHttpResponse since it's not
// LastHttpContent).
contexts.removeFirst();
requests.removeFirst();
serverContexts.removeFirst();
HttpResponse response = ctx.channel().attr(HTTP_SERVER_RESPONSE).getAndSet(null);
writePromise.addListener(future -> end(context, request, response, writePromise));
writePromise.addListener(
future ->
end(serverContext.context(), serverContext.request(), response, writePromise));
}
} else {
writePromise = prm;
if (msg instanceof HttpResponse) {
// Headers before body has been sent, store them to use when finishing the span.
beforeCommitHandler.handle(context, (HttpResponse) msg);
beforeCommitHandler.handle(serverContext.context(), (HttpResponse) msg);
ctx.channel().attr(HTTP_SERVER_RESPONSE).set((HttpResponse) msg);
}
}
try (Scope ignored = context.makeCurrent()) {
try (Scope ignored = serverContext.context().makeCurrent()) {
super.write(ctx, msg, writePromise);
} catch (Throwable throwable) {
contexts.removeFirst();
requests.removeFirst();
end(context, request, null, throwable);
serverContexts.removeFirst();
end(serverContext.context(), serverContext.request(), null, throwable);
throw throwable;
}
}

View File

@ -12,6 +12,7 @@ import static io.opentelemetry.javaagent.instrumentation.ratpack.RatpackSingleto
import io.netty.util.Attribute;
import io.opentelemetry.context.Scope;
import io.opentelemetry.instrumentation.netty.v4_1.internal.AttributeKeys;
import io.opentelemetry.instrumentation.netty.v4_1.internal.ServerContext;
import io.opentelemetry.javaagent.bootstrap.Java8BytecodeBridge;
import java.util.Deque;
import ratpack.handling.Context;
@ -25,17 +26,16 @@ public final class TracingHandler implements Handler {
@Override
public void handle(Context ctx) {
Attribute<Deque<io.opentelemetry.context.Context>> serverContextAttribute =
Attribute<Deque<ServerContext>> serverContextAttribute =
ctx.getDirectChannelAccess().getChannel().attr(AttributeKeys.SERVER_CONTEXT);
Deque<io.opentelemetry.context.Context> contexts = serverContextAttribute.get();
io.opentelemetry.context.Context serverSpanContext =
contexts != null ? contexts.peekFirst() : null;
Deque<ServerContext> serverContexts = serverContextAttribute.get();
ServerContext serverContext = serverContexts != null ? serverContexts.peekFirst() : null;
// Must use context from channel, as executor instrumentation is not accurate - Ratpack
// internally queues events and then drains them in batches, causing executor instrumentation to
// attach the same context to a batch of events from different requests.
io.opentelemetry.context.Context parentOtelContext =
serverSpanContext != null ? serverSpanContext : Java8BytecodeBridge.currentContext();
serverContext != null ? serverContext.context() : Java8BytecodeBridge.currentContext();
io.opentelemetry.context.Context callbackContext;
if (instrumenter().shouldStart(parentOtelContext, INITIAL_SPAN_NAME)) {

View File

@ -9,9 +9,9 @@ import static net.bytebuddy.matcher.ElementMatchers.named;
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
import io.netty.channel.Channel;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.instrumentation.netty.v4_1.internal.AttributeKeys;
import io.opentelemetry.instrumentation.netty.v4_1.internal.ServerContext;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import java.util.Deque;
@ -39,10 +39,10 @@ public class ContextHandlerInstrumentation implements TypeInstrumentation {
@Advice.OnMethodEnter(suppress = Throwable.class)
public static Scope onEnter(@Advice.Argument(0) Channel channel) {
// set context to the first unprocessed request
Deque<Context> contexts = channel.attr(AttributeKeys.SERVER_CONTEXT).get();
Context context = contexts != null ? contexts.peekFirst() : null;
if (context != null) {
return context.makeCurrent();
Deque<ServerContext> serverContextx = channel.attr(AttributeKeys.SERVER_CONTEXT).get();
ServerContext serverContext = serverContextx != null ? serverContextx.peekFirst() : null;
if (serverContext != null) {
return serverContext.context().makeCurrent();
}
return null;
}

View File

@ -9,9 +9,9 @@ import static net.bytebuddy.matcher.ElementMatchers.named;
import static net.bytebuddy.matcher.ElementMatchers.takesNoArguments;
import io.netty.channel.ChannelHandlerContext;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.instrumentation.netty.v4_1.internal.AttributeKeys;
import io.opentelemetry.instrumentation.netty.v4_1.internal.ServerContext;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import java.util.Deque;
@ -40,11 +40,11 @@ public class HttpTrafficHandlerInstrumentation implements TypeInstrumentation {
public static Scope onEnter(
@Advice.FieldValue("ctx") ChannelHandlerContext channelHandlerContext) {
// set context to the first unprocessed request
Deque<Context> contexts =
Deque<ServerContext> serverContexts =
channelHandlerContext.channel().attr(AttributeKeys.SERVER_CONTEXT).get();
Context context = contexts != null ? contexts.peekFirst() : null;
if (context != null) {
return context.makeCurrent();
ServerContext serverContext = serverContexts != null ? serverContexts.peekFirst() : null;
if (serverContext != null) {
return serverContext.context().makeCurrent();
}
return null;
}