open-telemetry
/
opentelemetry-java-instrumentation
mirror of https://github.com/open-telemetry/opentelemetry-java-instrumentation.git
1
0
Fork 0
Code Issues Packages Projects Releases Wiki Activity

Clean up BaseTracer, part 1 (#2159) 

* Move context leak debugging to ContextPropagationDebug
* Remove getCurrentSpan()
* Add end(Context, ...) & endExceptionally(Context, ...) methods - they're supposed to replace the ones that accept Spans in the future
This commit is contained in:
Mateusz Rzeszutek 2021-02-02 09:12:46 +01:00 committed by GitHub
parent b7ad3d2307
commit 7a3f345c18
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
10 changed files with 62 additions and 91 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

42
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/context/ContextPropagationDebug.java
View File

@ -5,18 +5,26 @@
package io.opentelemetry.instrumentation.api.context;
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.ContextKey;
import java.util.Iterator;
import java.util.List;
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;
public final class ContextPropagationDebug {
private static final Logger log = LoggerFactory.getLogger(ContextPropagationDebug.class);
// locations where the context was propagated to another thread (tracking multiple steps is
// helpful in akka where there is so much recursive async spawning of new work)
private static final ContextKey<List<StackTraceElement[]>> THREAD_PROPAGATION_LOCATIONS =
ContextKey.named("thread-propagation-locations");
private static final boolean THREAD_PROPAGATION_DEBUGGER =
Boolean.getBoolean("otel.threadPropagationDebugger");
private static final boolean FAIL_ON_CONTEXT_LEAK =
Boolean.getBoolean("otel.internal.failOnContextLeak");
public static boolean isThreadPropagationDebuggerEnabled() {
return THREAD_PROPAGATION_DEBUGGER;
@ -30,5 +38,39 @@ public final class ContextPropagationDebug {
return context.with(THREAD_PROPAGATION_LOCATIONS, locations);
}
public static void debugContextLeakIfEnabled() {
if (!isThreadPropagationDebuggerEnabled()) {
return;
}
Context current = Context.current();
if (current != Context.root()) {
log.error("Unexpected non-root current context found when extracting remote context!");
Span currentSpan = Span.fromContextOrNull(current);
if (currentSpan != null) {
log.error("It contains this span: {}", currentSpan);
}
List<StackTraceElement[]> locations = ContextPropagationDebug.getLocations(current);
if (locations != null) {
StringBuilder sb = new StringBuilder();
Iterator<StackTraceElement[]> i = locations.iterator();
while (i.hasNext()) {
for (StackTraceElement ste : i.next()) {
sb.append("\n");
sb.append(ste);
}
if (i.hasNext()) {
sb.append("\nwhich was propagated from:");
}
}
log.error("a context leak was detected. it was propagated from:{}", sb);
}
if (FAIL_ON_CONTEXT_LEAK) {
throw new IllegalStateException("Context leak detected");
}
}
}
private ContextPropagationDebug() {}
}

60
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/BaseTracer.java
View File

@ -17,8 +17,6 @@ import io.opentelemetry.context.propagation.TextMapPropagator;
import io.opentelemetry.instrumentation.api.InstrumentationVersion;
import io.opentelemetry.instrumentation.api.context.ContextPropagationDebug;
import java.lang.reflect.Method;
import java.util.Iterator;
import java.util.List;
import java.util.concurrent.ExecutionException;
import java.util.concurrent.TimeUnit;
import org.slf4j.Logger;
@ -27,9 +25,6 @@ import org.slf4j.LoggerFactory;
public abstract class BaseTracer {
private static final Logger log = LoggerFactory.getLogger(HttpServerTracer.class);
private static final boolean FAIL_ON_CONTEXT_LEAK =
Boolean.getBoolean("otel.internal.failOnContextLeak");
// Keeps track of the server span for the current trace.
// TODO(anuraaga): Should probably be renamed to local root key since it could be a consumer span
// or other non-server root.
@ -76,10 +71,6 @@ public abstract class BaseTracer {
return Context.current().with(span).makeCurrent();
}
public Span getCurrentSpan() {
return Span.current();
}
protected final boolean shouldStartSpan(Kind proposedKind, Context context) {
switch (proposedKind) {
case CLIENT:
@ -146,6 +137,14 @@ public abstract class BaseTracer {
return className;
}
public void end(Context context) {
end(context, -1);
}
public void end(Context context, long endTimeNanos) {
end(Span.fromContext(context), endTimeNanos);
}
public void end(Span span) {
end(span, -1);
}
@ -158,6 +157,14 @@ public abstract class BaseTracer {
}
}
public void endExceptionally(Context context, Throwable throwable) {
endExceptionally(context, throwable, -1);
}
public void endExceptionally(Context context, Throwable throwable, long endTimeNanos) {
endExceptionally(Span.fromContext(context), throwable, endTimeNanos);
}
public void endExceptionally(Span span, Throwable throwable) {
endExceptionally(span, throwable, -1);
}
@ -181,9 +188,8 @@ public abstract class BaseTracer {
}
public static <C> Context extract(C carrier, TextMapPropagator.Getter<C> getter) {
if (ContextPropagationDebug.isThreadPropagationDebuggerEnabled()) {
debugContextLeak();
}
ContextPropagationDebug.debugContextLeakIfEnabled();
// Using Context.ROOT here may be quite unexpected, but the reason is simple.
// We want either span context extracted from the carrier or invalid one.
// We DO NOT want any span context potentially lingering in the current context.
@ -192,36 +198,6 @@ public abstract class BaseTracer {
.extract(Context.root(), carrier, getter);
}
private static void debugContextLeak() {
Context current = Context.current();
if (current != Context.root()) {
log.error("Unexpected non-root current context found when extracting remote context!");
Span currentSpan = Span.fromContextOrNull(current);
if (currentSpan != null) {
log.error("It contains this span: {}", currentSpan);
}
List<StackTraceElement[]> locations = ContextPropagationDebug.getLocations(current);
if (locations != null) {
StringBuilder sb = new StringBuilder();
Iterator<StackTraceElement[]> i = locations.iterator();
while (i.hasNext()) {
for (StackTraceElement ste : i.next()) {
sb.append("\n");
sb.append(ste);
}
if (i.hasNext()) {
sb.append("\nwhich was propagated from:");
}
}
log.error("a context leak was detected. it was propagated from:{}", sb);
}
if (FAIL_ON_CONTEXT_LEAK) {
throw new IllegalStateException("Context leak detected");
}
}
}
/** Returns span of type SERVER from the current context or <code>null</code> if not found. */
public static Span getCurrentServerSpan() {
return getCurrentServerSpan(Context.current());

9
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/DatabaseClientTracer.java
View File

@ -51,15 +51,6 @@ public abstract class DatabaseClientTracer<CONNECTION, QUERY> extends BaseTracer
return withClientSpan(parentContext, span);
}
@Override
public Span getCurrentSpan() {
return Span.current();
}
public void end(Context context) {
Span.fromContext(context).end();
}
public void endExceptionally(Context context, Throwable throwable) {
Span span = Span.fromContext(context);
onError(span, throwable);

10
instrumentation-api/src/main/java/io/opentelemetry/instrumentation/api/tracer/HttpClientTracer.java
View File

@ -103,11 +103,6 @@ public abstract class HttpClientTracer<REQUEST, CARRIER, RESPONSE> extends BaseT
super.end(span, endTimeNanos);
}
public void end(Context context) {
Span span = Span.fromContext(context);
super.end(span);
}
public void endExceptionally(Context context, RESPONSE response, Throwable throwable) {
endExceptionally(context, response, throwable, -1);
}
@ -119,11 +114,6 @@ public abstract class HttpClientTracer<REQUEST, CARRIER, RESPONSE> extends BaseT
super.endExceptionally(span, throwable, endTimeNanos);
}
public void endExceptionally(Context context, Throwable throwable) {
Span span = Span.fromContext(context);
super.endExceptionally(span, throwable, -1);
}
// TODO (trask) see if we can reduce the number of end..() variants
// see https://github.com/open-telemetry/opentelemetry-java-instrumentation
// /pull/1893#discussion_r542111699

4
instrumentation/apache-camel-2.20/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/apachecamel/CamelRoutePolicy.java
View File

@ -38,7 +38,7 @@ final class CamelRoutePolicy extends RoutePolicySupport {
private Span spanOnExchangeBegin(
Route route, Exchange exchange, SpanDecorator sd, Span.Kind spanKind) {
Span activeSpan = CamelTracer.TRACER.getCurrentSpan();
Span activeSpan = Span.current();
String name = sd.getOperationName(exchange, route.getEndpoint(), CamelDirection.INBOUND);
SpanBuilder builder = CamelTracer.TRACER.spanBuilder(name);
builder.setSpanKind(spanKind);
@ -52,7 +52,7 @@ final class CamelRoutePolicy extends RoutePolicySupport {
}
private Span.Kind spanKind(SpanDecorator sd) {
Span activeSpan = CamelTracer.TRACER.getCurrentSpan();
Span activeSpan = Span.current();
// if there's an active span, this is not a root span which we always mark as INTERNAL
return (activeSpan.getSpanContext().isValid() ? Span.Kind.INTERNAL : sd.getReceiverSpanKind());
}

8
instrumentation/spring/spring-batch-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/batch/chunk/ChunkExecutionTracer.java
View File

@ -43,14 +43,6 @@ public class ChunkExecutionTracer extends BaseTracer {
}
}
public void end(Context context) {
end(Span.fromContext(context));
}
public void endExceptionally(Context context, Throwable throwable) {
endExceptionally(Span.fromContext(context), throwable);
}
@Override
protected String getInstrumentationName() {
return "io.opentelemetry.javaagent.spring-batch";

8
instrumentation/spring/spring-batch-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/batch/item/ItemTracer.java
View File

@ -68,14 +68,6 @@ public class ItemTracer extends BaseTracer {
return currentContext.with(span);
}
public void end(Context context) {
end(Span.fromContext(context));
}
public void endExceptionally(Context context, Throwable throwable) {
endExceptionally(Span.fromContext(context), throwable);
}
@Override
protected String getInstrumentationName() {
return "io.opentelemetry.javaagent.spring-batch";

4
instrumentation/spring/spring-batch-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/batch/job/JobExecutionTracer.java
View File

@ -25,10 +25,6 @@ public class JobExecutionTracer extends BaseTracer {
return Context.current().with(span);
}
public void end(Context context) {
end(Span.fromContext(context));
}
@Override
protected String getInstrumentationName() {
return "io.opentelemetry.javaagent.spring-batch";

4
instrumentation/spring/spring-batch-3.0/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/spring/batch/step/StepExecutionTracer.java
View File

@ -26,10 +26,6 @@ public class StepExecutionTracer extends BaseTracer {
return Context.current().with(span);
}
public void end(Context context) {
end(Span.fromContext(context));
}
@Override
protected String getInstrumentationName() {
return "io.opentelemetry.javaagent.spring-batch";

4
instrumentation/twilio-6.6/javaagent/src/main/java/io/opentelemetry/javaagent/instrumentation/twilio/TwilioTracer.java
View File

@ -100,10 +100,6 @@ public class TwilioTracer extends BaseTracer {
super.end(span);
}
public void endExceptionally(Context context, Throwable throwable) {
super.endExceptionally(Span.fromContext(context), throwable);
}
/**
* Helper method for calling a getter using reflection. This will be slow, so only use when
* required.