Spring Kafka library instrumentation #6283
Conversation
| links( | ||
| producer1.get().getSpanContext(), | ||
| producer2.get().getSpanContext())) |
There was a problem hiding this comment.
OTLP drops the isRemote flag on span context, which makes the agent vs library instrumentation tests behave differently: agent tests have a agent CL -> OTLP data -> text CL conversion that clears out the flag, while in library instrumentation tests the value of the flag is preserved, and simply using producer1.get().getSpanContext() here makes the test fail (cause it's not a remote span).
@anuraaga @jack-berg should we perhaps modify the assertions so that they ignore the flag? Or at least provide an easy possibility to do so? WDYT?
| // 2.7.0 has a bug that makes decorating a Kafka Producer impossible | ||
| testImplementation("org.springframework.kafka:spring-kafka:2.7.1") |
There was a problem hiding this comment.
I support the spring-kafka-2.7 naming even though it only supports 2.7.1+
There was a problem hiding this comment.
It actually kinda does support 2.7 -- spring-kafka-2.7 only instruments the consumer part, and the problem is within the producer-specific spring code 😄
| .getClass() | ||
| .getName() | ||
| .equals( | ||
| "io.opentelemetry.instrumentation.spring.kafka.v2_7.InstrumentedBatchInterceptor")) { |
There was a problem hiding this comment.
what would break if you'd use InstrumentedBatchInterceptor.class.getName() or interceptor.getClass() != InstrumentedBatchInterceptor.class here? Did you choose to use class name here to keep InstrumentedBatchInterceptor package private?
There was a problem hiding this comment.
If it is do keep the class package private WDYT about using something like telemetry().isInstrumentedBatchInterceptor(interceptor)
There was a problem hiding this comment.
Did you choose to use class name here to keep
InstrumentedBatchInterceptorpackage private?
Yep, exactly that.
If it is do keep the class package private WDYT about using something like
telemetry().isInstrumentedBatchInterceptor(interceptor)
I'd rather not add more public surface than needed. I decided to move these two interceptors to an internal package.
* Spring Kafka library instrumentation * Merge and fix prior merge Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
* Spring Kafka library instrumentation * Merge and fix prior merge Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
* Spring Kafka library instrumentation * Merge and fix prior merge Co-authored-by: Trask Stalnaker <trask.stalnaker@gmail.com>
A prerequisite for #4636