New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Tracing kafka collector #3244
Tracing kafka collector #3244
Conversation
985597b
to
3317a7f
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
sounds good. I think we have a rate limit now on self-tracing so it should be safe to trace collectors!
zipkin-server/src/main/java/zipkin2/server/internal/brave/ZipkinSelfTracingConfiguration.java
Outdated
Show resolved
Hide resolved
final KafkaTracing kafkaTracing = KafkaTracing.newBuilder(maybeTracing.get()) | ||
.remoteServiceName("zipkin-kafka") | ||
.build(); | ||
builder.consumerSupplier(props -> kafkaTracing.consumer(new KafkaConsumer<>(props))); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
maybe add another inject parameter for consumerSupplier instead? you can then make a bean that is conditionally wrapped with KafkaTracing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
gave a try, let me know how it looks now
5b7a252
to
466b5af
Compare
@openzipkin/armeria what would be the best way to handle this scenario: When self-tracing is enabled,
Not sure where this should be handled, e.g. transparently at the https://armeria.dev/docs/advanced-unit-testing#using-a-fake-context-to-emulate-an-incoming-request mentions this scenario, but the module implementing the kafka consumer does not have an armeria dependency to create a fake context. |
Fix #2872
Changes to enable self-tracing on kafka-collector. As mentioned on the issue,
RequestContextCurrentTraceContext
causes an issue to propagate traces.From Armeria Slack: