opentracing-contrib / java-jdbc

OpenTracing Instrumentation for JDBC
Apache License 2.0
82 stars 56 forks source link

Instrumentation does not add span to existing span context #43

Closed DannyNoam closed 4 years ago

DannyNoam commented 5 years ago

Hi there,

I'm relatively new to OpenTracing, and started using this library to instrument JDBC calls in our application. The way our application works is that it consumes an event from Kafka, and makes a database call to insert a new row in our database.

Consuming an event from Kafka successfully creates a span, and I would expect the JDBC call to create a follows_from reference to the previous span. However, it looks like this library does not cater for adding follows_from or child_of references whatsoever, and assumes that a database call is always a completely new trace in isolation.

Am I misunderstanding this?

Cheers, Danny

malafeev commented 5 years ago

This library adds parent span if there is an active one. I expect that when you receive an even from Kafka you don't have an active span during jdbc call. (kafka span is closed, not activated).

DannyNoam commented 5 years ago

Thanks for the response! You're right - the previous span has finished, and there is no active span. My understanding is that, in my example, there are two distinct operations:

[ Span: Consume event from kafka ] ----> [ Span: JDBC query ]

Rather than:

[ Span: Consume event from kafka and make JDBC query ]

If we conflate the two spans as one, then the span will both contain information about Kafka and JDBC, and it feels a bit wrong. What's your thoughts on this?

Cheers

malafeev commented 5 years ago

spans should be separate, but they should have relation. If you have access to kafka record then you can extract kafka span context:

SpanContext kafkaSpanContext = TracingKafkaUtils.extractSpanContext(record.headers(), 
     tracer);
Span child = tracer.buildSpan("call").addReference(References.FOLLOWS_FROM, 
     kafkaSpanContext).start();
try(Scope scope = tracer.activateSpan(child)) {
      // Here make jdbc call
} finally {
      child.finish();
}
yurishkuro commented 5 years ago

@DannyNoam what kind of instrumentation are you using for Kafka?

I have a working example using opentracing-kafka-spring (https://github.com/PacktPublishing/Mastering-Distributed-Tracing/tree/master/Chapter05), but it requires manual creation of the consumer handler span, similar to what @malafeev showed.

Hohenheimsenberg commented 5 years ago

I'm having issues instrumenting my traces with this library too. Shouldn't JdbcTracingUtils.buildSpan() have something like this?:

...
Span parentSpan = tracer.activeSpan();
if( parentSpan != null){
    Tracer.SpanBuilder spanBuilder = tracer.buildSpan(operationName)
            .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CLIENT)
            .asChildOf(parentSpan);
}else{
    Tracer.SpanBuilder spanBuilder = tracer.buildSpan(operationName)
            .withTag(Tags.SPAN_KIND.getKey(), Tags.SPAN_KIND_CLIENT);
}
...

Currently it doesn't have a reference to its parent.

malafeev commented 5 years ago

@Hohenheimsenberg it's not needed to set explicitly parent span via asChildOf. could you verify that there is an active span before you execute a query?

Hohenheimsenberg commented 5 years ago

@Hohenheimsenberg it's not needed to set explicitly parent span via asChildOf. could you verify that there is an active span before you execute a query?

You are right, if there is an active span it automatically creates it 'asChildOf' the active one. I was debugin it wrong.