Deprecate CallDepth.reset() and get() (#3511)

* Deprecate CallDepth.reset() and get()

* Don't pass CallDepth around
This commit is contained in:
Trask Stalnaker 2021-07-07 09:24:50 -07:00 committed by GitHub
parent 238427ba6a
commit b304cc2912
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
21 changed files with 81 additions and 71 deletions

View File

@ -109,6 +109,7 @@ public class ClassLoaderInstrumentation implements TypeInstrumentation {
// back to this instrumentation over and over, causing a StackOverflowError // back to this instrumentation over and over, causing a StackOverflowError
CallDepth callDepth = CallDepth.forClass(ClassLoader.class); CallDepth callDepth = CallDepth.forClass(ClassLoader.class);
if (callDepth.getAndIncrement() > 0) { if (callDepth.getAndIncrement() > 0) {
callDepth.decrementAndGet();
return null; return null;
} }
@ -128,7 +129,7 @@ public class ClassLoaderInstrumentation implements TypeInstrumentation {
// ends up calling a ClassFileTransformer which ends up calling loadClass() further down the // ends up calling a ClassFileTransformer which ends up calling loadClass() further down the
// stack on one of our bootstrap packages (since the call depth check would then suppress // stack on one of our bootstrap packages (since the call depth check would then suppress
// the nested loadClass instrumentation) // the nested loadClass instrumentation)
callDepth.reset(); callDepth.decrementAndGet();
} }
return null; return null;
} }

View File

@ -82,10 +82,9 @@ public class JaxRsAnnotationsInstrumentation implements TypeInstrumentation {
@Advice.Local("otelCallDepth") CallDepth callDepth, @Advice.Local("otelCallDepth") CallDepth callDepth,
@Advice.Local("otelContext") Context context, @Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) { @Advice.Local("otelScope") Scope scope) {
if (scope == null) { if (callDepth.decrementAndGet() > 0) {
return; return;
} }
callDepth.reset();
scope.close(); scope.close();
if (throwable == null) { if (throwable == null) {

View File

@ -75,6 +75,11 @@ public class JaxRsAnnotationsInstrumentation implements TypeInstrumentation {
@Advice.Local("otelContext") Context context, @Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope, @Advice.Local("otelScope") Scope scope,
@Advice.Local("otelAsyncResponse") AsyncResponse asyncResponse) { @Advice.Local("otelAsyncResponse") AsyncResponse asyncResponse) {
callDepth = CallDepth.forClass(Path.class);
if (callDepth.getAndIncrement() > 0) {
return;
}
ContextStore<AsyncResponse, Context> contextStore = null; ContextStore<AsyncResponse, Context> contextStore = null;
for (Object arg : args) { for (Object arg : args) {
if (arg instanceof AsyncResponse) { if (arg instanceof AsyncResponse) {
@ -93,11 +98,6 @@ public class JaxRsAnnotationsInstrumentation implements TypeInstrumentation {
} }
} }
callDepth = CallDepth.forClass(Path.class);
if (callDepth.getAndIncrement() > 0) {
return;
}
context = tracer().startSpan(target.getClass(), method); context = tracer().startSpan(target.getClass(), method);
if (contextStore != null && asyncResponse != null) { if (contextStore != null && asyncResponse != null) {
@ -115,10 +115,13 @@ public class JaxRsAnnotationsInstrumentation implements TypeInstrumentation {
@Advice.Local("otelContext") Context context, @Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope, @Advice.Local("otelScope") Scope scope,
@Advice.Local("otelAsyncResponse") AsyncResponse asyncResponse) { @Advice.Local("otelAsyncResponse") AsyncResponse asyncResponse) {
if (callDepth.decrementAndGet() > 0) {
return;
}
if (context == null || scope == null) { if (context == null || scope == null) {
return; return;
} }
callDepth.reset();
if (throwable != null) { if (throwable != null) {
tracer().endExceptionally(context, throwable); tracer().endExceptionally(context, throwable);

View File

@ -68,10 +68,9 @@ public class WebServiceProviderInstrumentation implements TypeInstrumentation {
@Advice.Local("otelCallDepth") CallDepth callDepth, @Advice.Local("otelCallDepth") CallDepth callDepth,
@Advice.Local("otelContext") Context context, @Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) { @Advice.Local("otelScope") Scope scope) {
if (scope == null) { if (callDepth.decrementAndGet() > 0) {
return; return;
} }
callDepth.reset();
scope.close(); scope.close();
if (throwable == null) { if (throwable == null) {

View File

@ -79,10 +79,9 @@ public class JwsAnnotationsInstrumentation implements TypeInstrumentation {
@Advice.Local("otelCallDepth") CallDepth callDepth, @Advice.Local("otelCallDepth") CallDepth callDepth,
@Advice.Local("otelContext") Context context, @Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) { @Advice.Local("otelScope") Scope scope) {
if (scope == null) { if (callDepth.decrementAndGet() > 0) {
return; return;
} }
callDepth.reset();
scope.close(); scope.close();
if (throwable == null) { if (throwable == null) {

View File

@ -85,13 +85,14 @@ public class PreparedStatementInstrumentation implements TypeInstrumentation {
@Advice.Local("otelRequest") DbRequest request, @Advice.Local("otelRequest") DbRequest request,
@Advice.Local("otelContext") Context context, @Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) { @Advice.Local("otelScope") Scope scope) {
if (scope == null) { if (callDepth.decrementAndGet() > 0) {
return; return;
} }
callDepth.reset();
if (scope != null) {
scope.close(); scope.close();
instrumenter().end(context, request, null, throwable); instrumenter().end(context, request, null, throwable);
} }
} }
}
} }

View File

@ -85,13 +85,14 @@ public class StatementInstrumentation implements TypeInstrumentation {
@Advice.Local("otelRequest") DbRequest request, @Advice.Local("otelRequest") DbRequest request,
@Advice.Local("otelContext") Context context, @Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) { @Advice.Local("otelScope") Scope scope) {
if (scope == null) { if (callDepth.decrementAndGet() > 0) {
return; return;
} }
callDepth.reset();
if (scope != null) {
scope.close(); scope.close();
instrumenter().end(context, request, null, throwable); instrumenter().end(context, request, null, throwable);
} }
} }
}
} }

View File

@ -92,15 +92,16 @@ public class JmsMessageProducerInstrumentation implements TypeInstrumentation {
@Advice.Local("otelContext") Context context, @Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope, @Advice.Local("otelScope") Scope scope,
@Advice.Thrown Throwable throwable) { @Advice.Thrown Throwable throwable) {
if (scope == null) { if (callDepth.decrementAndGet() > 0) {
return; return;
} }
callDepth.reset();
if (scope != null) {
scope.close(); scope.close();
producerInstrumenter().end(context, request, null, throwable); producerInstrumenter().end(context, request, null, throwable);
} }
} }
}
@SuppressWarnings("unused") @SuppressWarnings("unused")
public static class ProducerWithDestinationAdvice { public static class ProducerWithDestinationAdvice {
@ -135,13 +136,14 @@ public class JmsMessageProducerInstrumentation implements TypeInstrumentation {
@Advice.Local("otelContext") Context context, @Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope, @Advice.Local("otelScope") Scope scope,
@Advice.Thrown Throwable throwable) { @Advice.Thrown Throwable throwable) {
if (scope == null) { if (callDepth.decrementAndGet() > 0) {
return; return;
} }
callDepth.reset();
if (scope != null) {
scope.close(); scope.close();
producerInstrumenter().end(context, request, null, throwable); producerInstrumenter().end(context, request, null, throwable);
} }
} }
}
} }

View File

@ -111,11 +111,11 @@ public class RabbitChannelInstrumentation implements TypeInstrumentation {
@Advice.Local("otelCallDepth") CallDepth callDepth, @Advice.Local("otelCallDepth") CallDepth callDepth,
@Advice.Local("otelContext") Context context, @Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) { @Advice.Local("otelScope") Scope scope) {
if (scope == null) { if (callDepth.decrementAndGet() > 0) {
return; return;
} }
scope.close(); scope.close();
callDepth.reset();
CURRENT_RABBIT_CONTEXT.remove(); CURRENT_RABBIT_CONTEXT.remove();
if (throwable != null) { if (throwable != null) {

View File

@ -65,12 +65,11 @@ public class RemoteServerInstrumentation implements TypeInstrumentation {
@Advice.Local("otelCallDepth") CallDepth callDepth, @Advice.Local("otelCallDepth") CallDepth callDepth,
@Advice.Local("otelContext") Context context, @Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) { @Advice.Local("otelScope") Scope scope) {
if (scope == null) { if (callDepth.decrementAndGet() > 0) {
return; return;
} }
scope.close();
callDepth.reset();
scope.close();
if (throwable != null) { if (throwable != null) {
RmiServerTracer.tracer().endExceptionally(context, throwable); RmiServerTracer.tracer().endExceptionally(context, throwable);
} else { } else {

View File

@ -66,13 +66,13 @@ public class Servlet2Advice {
@Advice.Local("otelContext") Context context, @Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) { @Advice.Local("otelScope") Scope scope) {
callDepth.decrementAndGet(); int depth = callDepth.decrementAndGet();
if (scope != null) { if (scope != null) {
scope.close(); scope.close();
} }
if (context == null && callDepth.get() == 0) { if (context == null && depth == 0) {
Context currentContext = Java8BytecodeBridge.currentContext(); Context currentContext = Java8BytecodeBridge.currentContext();
// Something else is managing the context, we're in the outermost level of Servlet // Something else is managing the context, we're in the outermost level of Servlet
// instrumentation and we have an uncaught throwable. Let's add it to the current span. // instrumentation and we have an uncaught throwable. Let's add it to the current span.

View File

@ -19,13 +19,13 @@ import net.bytebuddy.asm.Advice;
public class AsyncDispatchAdvice { public class AsyncDispatchAdvice {
@Advice.OnMethodEnter(suppress = Throwable.class) @Advice.OnMethodEnter(suppress = Throwable.class)
public static boolean enter( public static void enter(
@Advice.This AsyncContext context, @Advice.This AsyncContext context,
@Advice.AllArguments Object[] args, @Advice.AllArguments Object[] args,
@Advice.Local("otelCallDepth") CallDepth callDepth) { @Advice.Local("otelCallDepth") CallDepth callDepth) {
callDepth = CallDepth.forClass(AsyncContext.class); callDepth = CallDepth.forClass(AsyncContext.class);
if (callDepth.getAndIncrement() > 0) { if (callDepth.getAndIncrement() > 0) {
return false; return;
} }
ServletRequest request = context.getRequest(); ServletRequest request = context.getRequest();
@ -42,15 +42,10 @@ public class AsyncDispatchAdvice {
// processing, and nothing can be done with the request anymore after this // processing, and nothing can be done with the request anymore after this
request.setAttribute(CONTEXT_ATTRIBUTE, currentContext); request.setAttribute(CONTEXT_ATTRIBUTE, currentContext);
} }
return true;
} }
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void exit( public static void exit(@Advice.Local("otelCallDepth") CallDepth callDepth) {
@Advice.Enter boolean topLevel, @Advice.Local("otelCallDepth") CallDepth callDepth) { callDepth.decrementAndGet();
if (topLevel) {
callDepth.reset();
}
} }
} }

View File

@ -36,8 +36,10 @@ public class Servlet3Advice {
@Advice.Local("otelCallDepth") CallDepth callDepth, @Advice.Local("otelCallDepth") CallDepth callDepth,
@Advice.Local("otelContext") Context context, @Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) { @Advice.Local("otelScope") Scope scope) {
callDepth = CallDepth.forClass(AppServerBridge.getCallDepthKey()); callDepth = CallDepth.forClass(AppServerBridge.getCallDepthKey());
callDepth.getAndIncrement(); callDepth.getAndIncrement();
if (!(request instanceof HttpServletRequest) || !(response instanceof HttpServletResponse)) { if (!(request instanceof HttpServletRequest) || !(response instanceof HttpServletResponse)) {
return; return;
} }
@ -97,6 +99,8 @@ public class Servlet3Advice {
@Advice.Local("otelContext") Context context, @Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) { @Advice.Local("otelScope") Scope scope) {
boolean topLevel = callDepth.decrementAndGet() == 0;
if (!(request instanceof HttpServletRequest) || !(response instanceof HttpServletResponse)) { if (!(request instanceof HttpServletRequest) || !(response instanceof HttpServletResponse)) {
return; return;
} }
@ -106,7 +110,7 @@ public class Servlet3Advice {
(HttpServletRequest) request, (HttpServletRequest) request,
(HttpServletResponse) response, (HttpServletResponse) response,
throwable, throwable,
callDepth, topLevel,
context, context,
scope); scope);
} }

View File

@ -19,13 +19,13 @@ import net.bytebuddy.asm.Advice;
public class AsyncDispatchAdvice { public class AsyncDispatchAdvice {
@Advice.OnMethodEnter(suppress = Throwable.class) @Advice.OnMethodEnter(suppress = Throwable.class)
public static boolean enter( public static void enter(
@Advice.This AsyncContext context, @Advice.This AsyncContext context,
@Advice.AllArguments Object[] args, @Advice.AllArguments Object[] args,
@Advice.Local("otelCallDepth") CallDepth callDepth) { @Advice.Local("otelCallDepth") CallDepth callDepth) {
callDepth = CallDepth.forClass(AsyncContext.class); callDepth = CallDepth.forClass(AsyncContext.class);
if (callDepth.getAndIncrement() > 0) { if (callDepth.getAndIncrement() > 0) {
return false; return;
} }
ServletRequest request = context.getRequest(); ServletRequest request = context.getRequest();
@ -42,15 +42,10 @@ public class AsyncDispatchAdvice {
// processing, and nothing can be done with the request anymore after this // processing, and nothing can be done with the request anymore after this
request.setAttribute(CONTEXT_ATTRIBUTE, currentContext); request.setAttribute(CONTEXT_ATTRIBUTE, currentContext);
} }
return true;
} }
@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class) @Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void exit( public static void exit(@Advice.Local("otelCallDepth") CallDepth callDepth) {
@Advice.Enter boolean topLevel, @Advice.Local("otelCallDepth") CallDepth callDepth) { callDepth.decrementAndGet();
if (topLevel) {
callDepth.reset();
}
} }
} }

View File

@ -26,9 +26,11 @@ public class ResponseSendAdvice {
@Advice.Local("otelScope") Scope scope, @Advice.Local("otelScope") Scope scope,
@Advice.Local("otelCallDepth") CallDepth callDepth) { @Advice.Local("otelCallDepth") CallDepth callDepth) {
callDepth = CallDepth.forClass(HttpServletResponse.class); callDepth = CallDepth.forClass(HttpServletResponse.class);
if (callDepth.getAndIncrement() > 0) {
return;
}
// Don't want to generate a new top-level span // Don't want to generate a new top-level span
if (callDepth.getAndIncrement() == 0 if (Java8BytecodeBridge.currentSpan().getSpanContext().isValid()) {
&& Java8BytecodeBridge.currentSpan().getSpanContext().isValid()) {
context = tracer().startSpan(method); context = tracer().startSpan(method);
scope = context.makeCurrent(); scope = context.makeCurrent();
} }
@ -40,6 +42,9 @@ public class ResponseSendAdvice {
@Advice.Local("otelContext") Context context, @Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope, @Advice.Local("otelScope") Scope scope,
@Advice.Local("otelCallDepth") CallDepth callDepth) { @Advice.Local("otelCallDepth") CallDepth callDepth) {
HttpServletResponseAdviceHelper.stopSpan(tracer(), throwable, context, scope, callDepth); if (callDepth.decrementAndGet() > 0) {
return;
}
HttpServletResponseAdviceHelper.stopSpan(tracer(), throwable, context, scope);
} }
} }

View File

@ -39,6 +39,7 @@ public class JakartaServletServiceAdvice {
callDepth = CallDepth.forClass(AppServerBridge.getCallDepthKey()); callDepth = CallDepth.forClass(AppServerBridge.getCallDepthKey());
callDepth.getAndIncrement(); callDepth.getAndIncrement();
if (!(request instanceof HttpServletRequest) || !(response instanceof HttpServletResponse)) { if (!(request instanceof HttpServletRequest) || !(response instanceof HttpServletResponse)) {
return; return;
} }
@ -97,6 +98,9 @@ public class JakartaServletServiceAdvice {
@Advice.Local("otelCallDepth") CallDepth callDepth, @Advice.Local("otelCallDepth") CallDepth callDepth,
@Advice.Local("otelContext") Context context, @Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) { @Advice.Local("otelScope") Scope scope) {
boolean topLevel = callDepth.decrementAndGet() == 0;
if (!(request instanceof HttpServletRequest) || !(response instanceof HttpServletResponse)) { if (!(request instanceof HttpServletRequest) || !(response instanceof HttpServletResponse)) {
return; return;
} }
@ -106,7 +110,7 @@ public class JakartaServletServiceAdvice {
(HttpServletRequest) request, (HttpServletRequest) request,
(HttpServletResponse) response, (HttpServletResponse) response,
throwable, throwable,
callDepth, topLevel,
context, context,
scope); scope);
} }

View File

@ -8,12 +8,11 @@ package io.opentelemetry.javaagent.instrumentation.servlet.common.response;
import io.opentelemetry.context.Context; import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope; import io.opentelemetry.context.Scope;
import io.opentelemetry.instrumentation.api.tracer.BaseTracer; import io.opentelemetry.instrumentation.api.tracer.BaseTracer;
import io.opentelemetry.javaagent.instrumentation.api.CallDepth;
public class HttpServletResponseAdviceHelper { public class HttpServletResponseAdviceHelper {
public static void stopSpan( public static void stopSpan(
BaseTracer tracer, Throwable throwable, Context context, Scope scope, CallDepth callDepth) { BaseTracer tracer, Throwable throwable, Context context, Scope scope) {
if (callDepth.decrementAndGet() == 0 && context != null) { if (context != null) {
scope.close(); scope.close();
if (throwable != null) { if (throwable != null) {

View File

@ -8,7 +8,6 @@ package io.opentelemetry.javaagent.instrumentation.servlet.common.service;
import io.opentelemetry.context.Context; import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope; import io.opentelemetry.context.Scope;
import io.opentelemetry.instrumentation.servlet.ServletHttpServerTracer; import io.opentelemetry.instrumentation.servlet.ServletHttpServerTracer;
import io.opentelemetry.javaagent.instrumentation.api.CallDepth;
import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge; import io.opentelemetry.javaagent.instrumentation.api.Java8BytecodeBridge;
public class ServletAndFilterAdviceHelper { public class ServletAndFilterAdviceHelper {
@ -17,16 +16,15 @@ public class ServletAndFilterAdviceHelper {
REQUEST request, REQUEST request,
RESPONSE response, RESPONSE response,
Throwable throwable, Throwable throwable,
CallDepth callDepth, boolean topLevel,
Context context, Context context,
Scope scope) { Scope scope) {
callDepth.decrementAndGet();
if (scope != null) { if (scope != null) {
scope.close(); scope.close();
} }
if (context == null && callDepth.get() == 0) { if (context == null && topLevel) {
Context currentContext = Java8BytecodeBridge.currentContext(); Context currentContext = Java8BytecodeBridge.currentContext();
// Something else is managing the context, we're in the outermost level of Servlet // Something else is managing the context, we're in the outermost level of Servlet
// instrumentation and we have an uncaught throwable. Let's add it to the current span. // instrumentation and we have an uncaught throwable. Let's add it to the current span.

View File

@ -26,9 +26,11 @@ public class ResponseSendAdvice {
@Advice.Local("otelScope") Scope scope, @Advice.Local("otelScope") Scope scope,
@Advice.Local("otelCallDepth") CallDepth callDepth) { @Advice.Local("otelCallDepth") CallDepth callDepth) {
callDepth = CallDepth.forClass(HttpServletResponse.class); callDepth = CallDepth.forClass(HttpServletResponse.class);
if (callDepth.getAndIncrement() > 0) {
return;
}
// Don't want to generate a new top-level span // Don't want to generate a new top-level span
if (callDepth.getAndIncrement() == 0 if (Java8BytecodeBridge.currentSpan().getSpanContext().isValid()) {
&& Java8BytecodeBridge.currentSpan().getSpanContext().isValid()) {
context = tracer().startSpan(method); context = tracer().startSpan(method);
scope = context.makeCurrent(); scope = context.makeCurrent();
} }
@ -40,6 +42,9 @@ public class ResponseSendAdvice {
@Advice.Local("otelContext") Context context, @Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope, @Advice.Local("otelScope") Scope scope,
@Advice.Local("otelCallDepth") CallDepth callDepth) { @Advice.Local("otelCallDepth") CallDepth callDepth) {
HttpServletResponseAdviceHelper.stopSpan(tracer(), throwable, context, scope, callDepth); if (callDepth.decrementAndGet() > 0) {
return;
}
HttpServletResponseAdviceHelper.stopSpan(tracer(), throwable, context, scope);
} }
} }

View File

@ -71,10 +71,9 @@ public class AnnotatedMethodInstrumentation implements TypeInstrumentation {
@Advice.Local("otelCallDepth") CallDepth callDepth, @Advice.Local("otelCallDepth") CallDepth callDepth,
@Advice.Local("otelContext") Context context, @Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) { @Advice.Local("otelScope") Scope scope) {
if (scope == null) { if (callDepth.decrementAndGet() > 0) {
return; return;
} }
callDepth.reset();
scope.close(); scope.close();
if (throwable == null) { if (throwable == null) {

View File

@ -58,11 +58,13 @@ public final class CallDepth {
* Get current call depth. This method may be used by vendor distributions to extend existing * Get current call depth. This method may be used by vendor distributions to extend existing
* instrumentations. * instrumentations.
*/ */
@Deprecated
public int get() { public int get() {
return depth; return depth;
} }
/** Reset the call depth to its initial value. */ /** Reset the call depth to its initial value. */
@Deprecated
public void reset() { public void reset() {
depth = 0; depth = 0;
} }