search

opentracing-contrib / java-spring-rabbitmq

OpenTracing RabbitMQ instrumentation
Apache License 2.0
19 stars 21 forks source link

Is it OK to set two parents to newly created span/scope? #40

Open bsavov opened 5 years ago

bsavov commented 5 years ago

Hello, I'm trying to feed wavefront (using https://github.com/wavefrontHQ/wavefront-opentracing-sdk-java), but it seem when consume message from rabbitMQ, the newly created, for the purpose, span/scope has 2 parents (same data, different instance IDs). Looking at the code: https://github.com/opentracing-contrib/java-spring-rabbitmq/blob/c7cf9c61934c2b5ffbcf59890666cdc1904754b2/opentracing-spring-rabbitmq/src/main/java/io/opentracing/contrib/spring/rabbitmq/RabbitMqTracingUtils.java#L72 I wonder what is the reason to add two parents to the same scope builder. Shouldn't be only one of them? In wavefront the span is reported with two parents like:

span.kind | "producer" component | "rabbitmq" spanId | "740b92bf-feb9-4de2-afea-dc673cc1b76d" traceId | "127460d5-fcf5-4215-abe6-2f4d7cd7a6dc" parent | "[3a2073d0-dfcf-4c35-857c-b618419088d9, 3a2073d0-dfcf-4c35-857c-b618419088d9]"

(same id, as you can see), but it seems they cannot do proper correlation and I do not get good sequence in the visual tool.

Would it make sense to add only one of them? Or probably I'm doing something not quite as it shall be, for example where 'existingSpanContext' come from?

ask4gilles commented 5 years ago

@bsavov can you share a minimal project in order to reproduce the issue ?

bsavov commented 5 years ago

Our app is quite complex, so shall think, also find so time, if possible to isolate the part with spans/Scope. Just as a note it is WebFlux Springboot (2.1.5) based App. Thought by explaining my findings would be possible to get some insight what might be wrong or whether it is correct to set parent/s twice on same scope.

ask4gilles commented 5 years ago

@bsavov Can you retry with the latest version?

bsavov commented 5 years ago

still the same with 2.0.5 sample:

span.kind "producer" component "rabbitmq" spanId "7585ee37-2562-414e-9eee-51c072bae608" traceId "25a6d5ac-c9de-4330-af31-31f220397b73" parent "[b44ca531-ecdd-46c6-acd4-4c21dc416158, b44ca531-ecdd-46c6-acd4-4c21dc416158]"

Looks like both are presented: existingSpanContext and messageProperties.getHeaders() with sample headers:

{ebs.sampling-decision=true, messageConsumed=true, uberctx-parent-id=b442cedf-2ab7-418e-8b17-15db1f367cb9, uber-trace-id=12283a6e145b472b963ba49dc9760945:8526230c971043a09ff9976e6cc8bd69:b442cedf-2ab7-418e-8b17-15db1f367cb9:1, uberctx-user-id=some-user, messageSent=true, acknowledgmentCallback=AmqpAckCallback, id=2d56bcb4-10f9-f8ca-939b-1dfc7d613d66, contentType=application/octet-stream, sampling-decision=true, timestamp=1566170685760}

which apparently successfully extracts spanContext from the message headers and adding (same) parent context twice

bsavov commented 5 years ago

update: same story with 2.0.6-SNAPSHOT And again: wonder why it is needed to add parent twice https://github.com/opentracing-contrib/java-spring-rabbitmq/blob/master/opentracing-spring-rabbitmq/src/main/java/io/opentracing/contrib/spring/rabbitmq/RabbitMqTracingUtils.java#L65 Why don't chose one to prevail, ether existingSpanContext or what is coming from headers?