Netty: preserve caught exception in the context instead of calling end() (#4413)

This commit is contained in:
Mateusz Rzeszutek 2021-10-19 22:40:11 +02:00 committed by GitHub
parent 13f28dca2a
commit 2e97a4d3d5
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
17 changed files with 87 additions and 57 deletions

View File

@ -21,6 +21,8 @@ dependencies {
compileOnly("com.google.auto.value:auto-value-annotations")
annotationProcessor("com.google.auto.value:auto-value")
implementation(project(":instrumentation:netty:netty-common:javaagent"))
compileOnly("io.netty:netty:3.8.0.Final")
testLibrary("io.netty:netty:3.8.0.Final")

View File

@ -12,7 +12,7 @@ import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
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.v3_8.server.NettyServerErrorHandler;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyErrorHolder;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;
@ -39,7 +39,7 @@ public class DefaultChannelPipelineInstrumentation implements TypeInstrumentatio
@Advice.OnMethodEnter
public static void onEnter(@Advice.Argument(1) Throwable throwable) {
if (throwable != null) {
NettyServerErrorHandler.onError(Java8BytecodeBridge.currentContext(), throwable);
NettyErrorHolder.set(Java8BytecodeBridge.currentContext(), throwable);
}
}
}

View File

@ -9,6 +9,7 @@ import static io.opentelemetry.javaagent.instrumentation.netty.v3_8.client.Netty
import io.opentelemetry.context.Scope;
import io.opentelemetry.instrumentation.api.field.VirtualField;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyErrorHolder;
import org.jboss.netty.channel.Channel;
import org.jboss.netty.channel.ChannelHandlerContext;
import org.jboss.netty.channel.MessageEvent;
@ -35,7 +36,7 @@ public class HttpClientResponseTracingHandler extends SimpleChannelUpstreamHandl
requestAndContexts.context(),
requestAndContexts.request(),
(HttpResponse) msg.getMessage(),
null);
NettyErrorHolder.getOrDefault(requestAndContexts.context(), null));
requestContextsField.set(ctx.getChannel(), null);
}

View File

@ -11,6 +11,7 @@ import io.opentelemetry.instrumentation.api.instrumenter.PeerServiceAttributesEx
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.NettyErrorHolder;
import io.opentelemetry.javaagent.instrumentation.netty.v3_8.HttpRequestAndChannel;
import org.jboss.netty.handler.codec.http.HttpResponse;
@ -35,6 +36,8 @@ final class NettyClientSingletons {
.addAttributesExtractor(
PeerServiceAttributesExtractor.create(netClientAttributesExtractor))
.addRequestMetrics(HttpClientMetrics.get())
.addContextCustomizer(
(context, requestAndChannel, startAttributes) -> NettyErrorHolder.init(context))
.newClientInstrumenter(new HttpRequestHeadersSetter());
}

View File

@ -10,6 +10,7 @@ import static io.opentelemetry.javaagent.instrumentation.netty.v3_8.server.Netty
import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.instrumentation.api.field.VirtualField;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyErrorHolder;
import io.opentelemetry.javaagent.instrumentation.netty.v3_8.HttpRequestAndChannel;
import org.jboss.netty.channel.Channel;
import org.jboss.netty.channel.ChannelHandlerContext;
@ -35,12 +36,15 @@ public class HttpServerResponseTracingHandler extends SimpleChannelDownstreamHan
HttpRequestAndChannel request = requestAndContext.request();
HttpResponse response = (HttpResponse) msg.getMessage();
Throwable error = null;
try (Scope ignored = context.makeCurrent()) {
ctx.sendDownstream(msg);
instrumenter().end(context, request, response, null);
} catch (Throwable throwable) {
instrumenter().end(context, request, response, throwable);
throw throwable;
} catch (Throwable t) {
error = t;
throw t;
} finally {
error = NettyErrorHolder.getOrDefault(context, error);
instrumenter().end(context, request, response, error);
}
}
}

View File

@ -1,23 +0,0 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.javaagent.instrumentation.netty.v3_8.server;
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.api.trace.StatusCode;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.instrumenter.ErrorCauseExtractor;
public final class NettyServerErrorHandler {
// copied from BaseTracer#onException()
public static void onError(Context context, Throwable error) {
Span span = Span.fromContext(context);
span.setStatus(StatusCode.ERROR);
span.recordException(ErrorCauseExtractor.jdk().extractCause(error));
}
private NettyServerErrorHandler() {}
}

View File

@ -10,6 +10,7 @@ import io.opentelemetry.instrumentation.api.instrumenter.Instrumenter;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpServerMetrics;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanNameExtractor;
import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanStatusExtractor;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyErrorHolder;
import io.opentelemetry.javaagent.instrumentation.netty.v3_8.HttpRequestAndChannel;
import org.jboss.netty.handler.codec.http.HttpResponse;
@ -30,6 +31,8 @@ final class NettyServerSingletons {
.addAttributesExtractor(httpServerAttributesExtractor)
.addAttributesExtractor(new NettyNetServerAttributesExtractor())
.addRequestMetrics(HttpServerMetrics.get())
.addContextCustomizer(
(context, requestAndChannel, startAttributes) -> NettyErrorHolder.init(context))
.newServerInstrumenter(new NettyHeadersGetter());
}

View File

@ -6,5 +6,7 @@ dependencies {
compileOnly("com.google.auto.value:auto-value-annotations")
annotationProcessor("com.google.auto.value:auto-value")
api(project(":instrumentation:netty:netty-common:javaagent"))
compileOnly("io.netty:netty-codec-http:4.0.0.Final")
}

View File

@ -1,23 +0,0 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.javaagent.instrumentation.netty.common;
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.api.trace.StatusCode;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.instrumenter.ErrorCauseExtractor;
public final class NettyErrorHandler {
// copied from BaseTracer#onException()
public static void onError(Context context, Throwable error) {
Span span = Span.fromContext(context);
span.setStatus(StatusCode.ERROR);
span.recordException(ErrorCauseExtractor.jdk().extractCause(error));
}
private NettyErrorHandler() {}
}

View File

@ -14,6 +14,7 @@ import io.opentelemetry.instrumentation.api.instrumenter.http.HttpSpanStatusExtr
import io.opentelemetry.instrumentation.api.servlet.ServerSpanNaming;
import io.opentelemetry.javaagent.instrumentation.netty.common.HttpRequestAndChannel;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyCommonNetAttributesExtractor;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyErrorHolder;
public final class NettyServerInstrumenterFactory {
@ -33,6 +34,7 @@ public final class NettyServerInstrumenterFactory {
.addRequestMetrics(HttpServerMetrics.get())
.addContextCustomizer(
(context, request, attributes) -> {
context = NettyErrorHolder.init(context);
// netty is not exactly a "container", but it's the best match out of these
return ServerSpanNaming.init(context, ServerSpanNaming.Source.CONTAINER);
})

View File

@ -17,7 +17,7 @@ import io.opentelemetry.context.Context;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import io.opentelemetry.javaagent.instrumentation.netty.common.HttpRequestAndChannel;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyErrorHandler;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyErrorHolder;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.matcher.ElementMatcher;
@ -62,7 +62,7 @@ public class AbstractChannelHandlerContextInstrumentation implements TypeInstrum
Context serverContext = ctx.channel().attr(AttributeKeys.SERVER_CONTEXT).get();
if (serverContext != null) {
NettyErrorHandler.onError(serverContext, throwable);
NettyErrorHolder.set(serverContext, throwable);
}
}
}

View File

@ -15,6 +15,7 @@ import io.netty.handler.codec.http.HttpResponse;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.javaagent.instrumentation.netty.common.HttpRequestAndChannel;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyErrorHolder;
import io.opentelemetry.javaagent.instrumentation.netty.v4_0.AttributeKeys;
import javax.annotation.Nullable;
@ -41,6 +42,7 @@ public class HttpServerResponseTracingHandler extends ChannelOutboundHandlerAdap
private static void end(Channel channel, HttpResponse response, @Nullable Throwable error) {
Context context = channel.attr(AttributeKeys.SERVER_CONTEXT).getAndRemove();
HttpRequestAndChannel request = channel.attr(AttributeKeys.SERVER_REQUEST).getAndRemove();
error = NettyErrorHolder.getOrDefault(context, error);
instrumenter().end(context, request, response, error);
}
}

View File

@ -17,7 +17,7 @@ import io.opentelemetry.instrumentation.netty.v4_1.AttributeKeys;
import io.opentelemetry.javaagent.extension.instrumentation.TypeInstrumentation;
import io.opentelemetry.javaagent.extension.instrumentation.TypeTransformer;
import io.opentelemetry.javaagent.instrumentation.netty.common.HttpRequestAndChannel;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyErrorHandler;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyErrorHolder;
import io.opentelemetry.javaagent.instrumentation.netty.v4_1.client.NettyClientSingletons;
import net.bytebuddy.asm.Advice;
import net.bytebuddy.description.type.TypeDescription;
@ -60,7 +60,7 @@ public class AbstractChannelHandlerContextInstrumentation implements TypeInstrum
Context serverContext = ctx.channel().attr(AttributeKeys.SERVER_CONTEXT).get();
if (serverContext != null) {
NettyErrorHandler.onError(serverContext, throwable);
NettyErrorHolder.set(serverContext, throwable);
}
}
}

View File

@ -20,6 +20,7 @@ import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.instrumentation.netty.v4_1.AttributeKeys;
import io.opentelemetry.javaagent.instrumentation.netty.common.HttpRequestAndChannel;
import io.opentelemetry.javaagent.instrumentation.netty.common.NettyErrorHolder;
import javax.annotation.Nullable;
public class HttpServerResponseTracingHandler extends ChannelOutboundHandlerAdapter {
@ -87,6 +88,7 @@ public class HttpServerResponseTracingHandler extends ChannelOutboundHandlerAdap
Channel channel, @Nullable HttpResponse response, @Nullable Throwable error) {
Context context = channel.attr(AttributeKeys.SERVER_CONTEXT).getAndRemove();
HttpRequestAndChannel request = channel.attr(NettyServerSingletons.HTTP_REQUEST).getAndRemove();
error = NettyErrorHolder.getOrDefault(context, error);
instrumenter().end(context, request, response, error);
}
}

View File

@ -0,0 +1,3 @@
plugins {
id("otel.javaagent-instrumentation")
}

View File

@ -0,0 +1,51 @@
/*
* Copyright The OpenTelemetry Authors
* SPDX-License-Identifier: Apache-2.0
*/
package io.opentelemetry.javaagent.instrumentation.netty.common;
import static io.opentelemetry.context.ContextKey.named;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.ContextKey;
import io.opentelemetry.context.ImplicitContextKeyed;
import javax.annotation.Nullable;
public final class NettyErrorHolder implements ImplicitContextKeyed {
private static final ContextKey<NettyErrorHolder> KEY = named("opentelemetry-netty-error");
private volatile Throwable error;
private NettyErrorHolder() {}
public static Context init(Context context) {
if (context.get(KEY) != null) {
return context;
}
return context.with(new NettyErrorHolder());
}
public static void set(Context context, Throwable error) {
NettyErrorHolder holder = context.get(KEY);
if (holder != null) {
holder.error = error;
}
}
@Nullable
public static Throwable getOrDefault(Context context, @Nullable Throwable error) {
Throwable result = null;
NettyErrorHolder holder = context.get(KEY);
if (holder != null) {
result = holder.error;
}
return result == null ? error : result;
}
@Override
public Context storeInContext(Context context) {
return context.with(KEY, this);
}
}

View File

@ -256,6 +256,7 @@ include(":instrumentation:netty:netty-4.0:javaagent")
include(":instrumentation:netty:netty-4.1:library")
include(":instrumentation:netty:netty-4.1:javaagent")
include(":instrumentation:netty:netty-4-common:javaagent")
include(":instrumentation:netty:netty-common:javaagent")
include(":instrumentation:okhttp:okhttp-2.2:javaagent")
include(":instrumentation:okhttp:okhttp-3.0:javaagent")
include(":instrumentation:okhttp:okhttp-3.0:library")