-
Notifications
You must be signed in to change notification settings - Fork 1.5k
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
otelcol/exporter/kafka: key jaeger messages on traceid #2855
otelcol/exporter/kafka: key jaeger messages on traceid #2855
Conversation
|
Codecov Report
@@ Coverage Diff @@
## main #2855 +/- ##
=======================================
Coverage 92.04% 92.04%
=======================================
Files 278 278
Lines 15432 15434 +2
=======================================
+ Hits 14204 14206 +2
Misses 845 845
Partials 383 383
Continue to review full report at Codecov.
|
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.
/cc @jpkrohling
I might take a while to review this one, if any other Jaeger maintainers feel like they can review this one, I'll give my approval by proxy :-) |
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.
lgtm
Is this a breaking change? Should we protect by a config flag? |
I do not believe this to be a breaking change. While, this does affect placement of messages into kafka partitions, for all practical purposes we go from random to slightly less random. |
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.
Approving based on the review from @albertteoh
Description: exporter/kafkaexporter: key jaeger spans by traceid, emulating jaeger-collector behavior. This allows for "grouping by traceid" for kafka consumers. For reference: jaeger kafka producer.
Link to tracking Issue: N/A
Testing: Updated tests, inspected resulting messages produced into kafka to match up with the expectation.
Documentation: Updated kafkaexporter documentation to reflect behavior