Skip to content

Commit

Permalink
Remove some deprecated BaseTracer#end(Span) usages (part 2) (#2418)
Browse files Browse the repository at this point in the history
  • Loading branch information
Mateusz Rzeszutek committed Feb 26, 2021
1 parent 0f32ed4 commit 605485b
Show file tree
Hide file tree
Showing 25 changed files with 171 additions and 184 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@

import static io.opentelemetry.javaagent.instrumentation.grizzly.GrizzlyHttpServerTracer.tracer;

import io.opentelemetry.api.trace.Span;
import io.opentelemetry.context.Context;
import net.bytebuddy.asm.Advice;
import org.glassfish.grizzly.filterchain.FilterChainContext;

Expand All @@ -16,9 +16,9 @@ public class DefaultFilterChainAdvice {
@Advice.OnMethodEnter(suppress = Throwable.class)
public static void onFail(
@Advice.Argument(0) FilterChainContext ctx, @Advice.Argument(1) Throwable throwable) {
Span span = tracer().getServerSpan(ctx);
if (span != null) {
tracer().endExceptionally(span, throwable);
Context context = tracer().getServerContext(ctx);
if (context != null) {
tracer().endExceptionally(context, throwable);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@
import static net.bytebuddy.matcher.ElementMatchers.returns;
import static net.bytebuddy.matcher.ElementMatchers.takesArguments;

import io.opentelemetry.api.trace.Span;
import io.opentelemetry.context.Context;
import io.opentelemetry.javaagent.instrumentation.api.ContextStore;
import io.opentelemetry.javaagent.instrumentation.api.InstrumentationContext;
Expand Down Expand Up @@ -60,17 +59,17 @@ public static class SessionFactoryAdvice {
@Advice.OnMethodExit(suppress = Throwable.class)
public static void openSession(@Advice.Return Object session) {

Context context = Java8BytecodeBridge.currentContext();
Span span = tracer().startSpan(context, "Session");
Context parentContext = Java8BytecodeBridge.currentContext();
Context context = tracer().startSpan(parentContext, "Session");

if (session instanceof Session) {
ContextStore<Session, Context> contextStore =
InstrumentationContext.get(Session.class, Context.class);
contextStore.putIfAbsent((Session) session, context.with(span));
contextStore.putIfAbsent((Session) session, context);
} else if (session instanceof StatelessSession) {
ContextStore<StatelessSession, Context> contextStore =
InstrumentationContext.get(StatelessSession.class, Context.class);
contextStore.putIfAbsent((StatelessSession) session, context.with(span));
contextStore.putIfAbsent((StatelessSession) session, context);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
import static net.bytebuddy.matcher.ElementMatchers.takesArguments;

import io.opentelemetry.api.trace.Span;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.javaagent.instrumentation.api.CallDepthThreadLocalMap;
Expand Down Expand Up @@ -163,8 +162,7 @@ public static void startMethod(
}

if (!SCOPE_ONLY_METHODS.contains(name)) {
Span span = tracer().startSpan(sessionContext, "Session." + name, entity);
spanContext = sessionContext.with(span);
spanContext = tracer().startSpan(sessionContext, "Session." + name, entity);
scope = spanContext.makeCurrent();
} else {
scope = sessionContext.makeCurrent();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
import static net.bytebuddy.matcher.ElementMatchers.returns;
import static net.bytebuddy.matcher.ElementMatchers.takesArguments;

import io.opentelemetry.api.trace.Span;
import io.opentelemetry.context.Context;
import io.opentelemetry.javaagent.instrumentation.api.ContextStore;
import io.opentelemetry.javaagent.instrumentation.api.InstrumentationContext;
Expand Down Expand Up @@ -55,12 +54,12 @@ public static class SessionFactoryAdvice {
@Advice.OnMethodExit(suppress = Throwable.class)
public static void openSession(@Advice.Return SharedSessionContract session) {

Context context = Java8BytecodeBridge.currentContext();
Span span = tracer().startSpan(context, "Session");
Context parentContext = Java8BytecodeBridge.currentContext();
Context context = tracer().startSpan(parentContext, "Session");

ContextStore<SharedSessionContract, Context> contextStore =
InstrumentationContext.get(SharedSessionContract.class, Context.class);
contextStore.putIfAbsent(session, context.with(span));
contextStore.putIfAbsent(session, context);
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;
import static net.bytebuddy.matcher.ElementMatchers.takesArguments;

import io.opentelemetry.api.trace.Span;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.javaagent.instrumentation.api.CallDepthThreadLocalMap;
Expand Down Expand Up @@ -145,8 +144,7 @@ public static void startMethod(
}

if (!SCOPE_ONLY_METHODS.contains(name)) {
Span span = tracer().startSpan(sessionContext, "Session." + name, entity);
spanContext = sessionContext.with(span);
spanContext = tracer().startSpan(sessionContext, "Session." + name, entity);
scope = spanContext.makeCurrent();
} else {
scope = sessionContext.makeCurrent();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@

package io.opentelemetry.javaagent.instrumentation.hibernate;

import io.opentelemetry.api.trace.Span;
import io.opentelemetry.api.trace.SpanKind;
import io.opentelemetry.context.Context;
import io.opentelemetry.instrumentation.api.tracer.BaseTracer;
import java.lang.annotation.Annotation;
Expand All @@ -20,12 +20,12 @@ public static HibernateTracer tracer() {
return TRACER;
}

public Span startSpan(Context context, String operationName, Object entity) {
return startSpan(context, spanNameForOperation(operationName, entity));
public Context startSpan(Context parentContext, String operationName, Object entity) {
return startSpan(parentContext, spanNameForOperation(operationName, entity));
}

public Span startSpan(Context context, String spanName) {
return tracer.spanBuilder(spanName).setParent(context).startSpan();
public Context startSpan(Context parentContext, String spanName) {
return startSpan(parentContext, spanName, SpanKind.INTERNAL);
}

private String spanNameForOperation(String operationName, Object entity) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,7 @@ public static <TARGET, ENTITY> Context startSpanFrom(
return null; // This method call is being traced already.
}

Span span = tracer().startSpan(sessionContext, operationName, entity);
return sessionContext.with(span);
return tracer().startSpan(sessionContext, operationName, entity);
}

public static void end(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import static net.bytebuddy.matcher.ElementMatchers.named;
import static net.bytebuddy.matcher.ElementMatchers.takesArguments;

import io.opentelemetry.api.trace.Span;
import io.opentelemetry.context.Context;
import io.opentelemetry.javaagent.tooling.TypeInstrumentation;
import java.util.HashMap;
import java.util.Map;
Expand Down Expand Up @@ -65,12 +65,12 @@ public static void stopSpan(
}
destination = tracer().extractDestination(message, null);

Span span = tracer().startConsumerSpan(destination, "receive", message, startTime);
Context context = tracer().startConsumerSpan(destination, "receive", message, startTime);

if (throwable != null) {
tracer().endExceptionally(span, throwable);
tracer().endExceptionally(context, throwable);
} else {
tracer().end(span);
tracer().end(context);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@
import static net.bytebuddy.matcher.ElementMatchers.named;
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;

import io.opentelemetry.api.trace.Span;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.javaagent.tooling.TypeInstrumentation;
import java.util.Map;
Expand Down Expand Up @@ -47,26 +47,26 @@ public static class MessageListenerAdvice {
@Advice.OnMethodEnter(suppress = Throwable.class)
public static void onEnter(
@Advice.Argument(0) Message message,
@Advice.Local("otelSpan") Span span,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {

MessageDestination destination = tracer().extractDestination(message, null);
span =
context =
tracer().startConsumerSpan(destination, "process", message, System.currentTimeMillis());
scope = span.makeCurrent();
scope = context.makeCurrent();
}

@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void stopSpan(
@Advice.Local("otelSpan") Span span,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope,
@Advice.Thrown Throwable throwable) {
scope.close();

if (throwable != null) {
tracer().endExceptionally(span, throwable);
tracer().endExceptionally(context, throwable);
} else {
tracer().end(span);
tracer().end(context);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@
import static net.bytebuddy.matcher.ElementMatchers.named;
import static net.bytebuddy.matcher.ElementMatchers.takesArgument;

import io.opentelemetry.api.trace.Span;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.javaagent.instrumentation.api.CallDepthThreadLocalMap;
import io.opentelemetry.javaagent.tooling.TypeInstrumentation;
Expand Down Expand Up @@ -60,7 +60,7 @@ public static class ProducerAdvice {
public static void onEnter(
@Advice.Argument(0) Message message,
@Advice.This MessageProducer producer,
@Advice.Local("otelSpan") Span span,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {
int callDepth = CallDepthThreadLocalMap.incrementCallDepth(MessageProducer.class);
if (callDepth > 0) {
Expand All @@ -76,13 +76,15 @@ public static void onEnter(

MessageDestination messageDestination =
tracer().extractDestination(message, defaultDestination);
span = tracer().startProducerSpan(messageDestination, message);
scope = tracer().startProducerScope(span, message);
context = tracer().startProducerSpan(messageDestination, message);
// TODO: why are we propagating context only in this advice class? the other one does not
// inject current span context into JMS message
scope = tracer().startProducerScope(context, message);
}

@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void stopSpan(
@Advice.Local("otelSpan") Span span,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope,
@Advice.Thrown Throwable throwable) {
if (scope == null) {
Expand All @@ -92,9 +94,9 @@ public static void stopSpan(
CallDepthThreadLocalMap.reset(MessageProducer.class);

if (throwable != null) {
tracer().endExceptionally(span, throwable);
tracer().endExceptionally(context, throwable);
} else {
tracer().end(span);
tracer().end(context);
}
}
}
Expand All @@ -105,21 +107,21 @@ public static class ProducerWithDestinationAdvice {
public static void onEnter(
@Advice.Argument(0) Destination destination,
@Advice.Argument(1) Message message,
@Advice.Local("otelSpan") Span span,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope) {
int callDepth = CallDepthThreadLocalMap.incrementCallDepth(MessageProducer.class);
if (callDepth > 0) {
return;
}

MessageDestination messageDestination = tracer().extractDestination(message, destination);
span = tracer().startProducerSpan(messageDestination, message);
scope = span.makeCurrent();
context = tracer().startProducerSpan(messageDestination, message);
scope = context.makeCurrent();
}

@Advice.OnMethodExit(onThrowable = Throwable.class, suppress = Throwable.class)
public static void stopSpan(
@Advice.Local("otelSpan") Span span,
@Advice.Local("otelContext") Context context,
@Advice.Local("otelScope") Scope scope,
@Advice.Thrown Throwable throwable) {
if (scope == null) {
Expand All @@ -128,9 +130,9 @@ public static void stopSpan(
CallDepthThreadLocalMap.reset(MessageProducer.class);

if (throwable != null) {
tracer().endExceptionally(span, throwable);
tracer().endExceptionally(context, throwable);
} else {
tracer().end(span);
tracer().end(context);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,7 @@
import static io.opentelemetry.javaagent.instrumentation.jms.MessageInjectAdapter.SETTER;

import io.opentelemetry.api.GlobalOpenTelemetry;
import io.opentelemetry.api.trace.Span;
import io.opentelemetry.api.trace.SpanBuilder;
import io.opentelemetry.api.trace.SpanContext;
import io.opentelemetry.context.Context;
import io.opentelemetry.context.Scope;
import io.opentelemetry.instrumentation.api.tracer.BaseTracer;
Expand Down Expand Up @@ -41,7 +39,7 @@ public static JmsTracer tracer() {
return TRACER;
}

public Span startConsumerSpan(
public Context startConsumerSpan(
MessageDestination destination, String operation, Message message, long startTime) {
SpanBuilder spanBuilder =
tracer
Expand All @@ -50,32 +48,28 @@ public Span startConsumerSpan(
.setStartTimestamp(startTime, TimeUnit.MILLISECONDS)
.setAttribute(SemanticAttributes.MESSAGING_OPERATION, operation);

Context parentContext = Context.root();
if (message != null && "process".equals(operation)) {
// TODO use BaseTracer.extract() which has context leak detection
// (and fix the context leak that it is currently detecting when running Jms2Test)
Context context =
parentContext =
GlobalOpenTelemetry.getPropagators()
.getTextMapPropagator()
.extract(Context.root(), message, GETTER);
SpanContext spanContext = Span.fromContext(context).getSpanContext();
if (spanContext.isValid()) {
spanBuilder.setParent(context);
}
}
spanBuilder.setParent(parentContext);

Span span = spanBuilder.startSpan();
afterStart(span, destination, message);
return span;
afterStart(spanBuilder, destination, message);
return parentContext.with(spanBuilder.startSpan());
}

public Span startProducerSpan(MessageDestination destination, Message message) {
Span span = tracer.spanBuilder(spanName(destination, "send")).setSpanKind(PRODUCER).startSpan();
public Context startProducerSpan(MessageDestination destination, Message message) {
SpanBuilder span = tracer.spanBuilder(spanName(destination, "send")).setSpanKind(PRODUCER);
afterStart(span, destination, message);
return span;
return Context.current().with(span.startSpan());
}

public Scope startProducerScope(Span span, Message message) {
Context context = Context.current().with(span);
public Scope startProducerScope(Context context, Message message) {
GlobalOpenTelemetry.getPropagators().getTextMapPropagator().inject(context, message, SETTER);
return context.makeCurrent();
}
Expand Down Expand Up @@ -136,7 +130,7 @@ public static MessageDestination extractMessageDestination(Destination destinati
return MessageDestination.UNKNOWN;
}

private void afterStart(Span span, MessageDestination destination, Message message) {
private void afterStart(SpanBuilder span, MessageDestination destination, Message message) {
span.setAttribute(SemanticAttributes.MESSAGING_SYSTEM, "jms");
span.setAttribute(SemanticAttributes.MESSAGING_DESTINATION_KIND, destination.destinationKind);
if (destination.temporary) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,14 +23,15 @@ public static KafkaConsumerTracer tracer() {
return TRACER;
}

public Span startSpan(ConsumerRecord<?, ?> record) {
public Context startSpan(ConsumerRecord<?, ?> record) {
long now = System.currentTimeMillis();

Context parentContext = extractParent(record);
Span span =
tracer
.spanBuilder(spanNameOnConsume(record))
.setSpanKind(CONSUMER)
.setParent(extractParent(record))
.setParent(parentContext)
.setStartTimestamp(now, TimeUnit.MILLISECONDS)
.setAttribute(SemanticAttributes.MESSAGING_SYSTEM, "kafka")
.setAttribute(SemanticAttributes.MESSAGING_DESTINATION, record.topic())
Expand All @@ -42,7 +43,7 @@ public Span startSpan(ConsumerRecord<?, ?> record) {
.startSpan();

onConsume(span, now, record);
return span;
return parentContext.with(span);
}

private Context extractParent(ConsumerRecord<?, ?> record) {
Expand Down

0 comments on commit 605485b

Please sign in to comment.