-
Notifications
You must be signed in to change notification settings - Fork 348
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
Change how Kafka is configured for collector and ingester #390
Conversation
Signed-off-by: Gary Brown <[email protected]>
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.
Once the unit tests are fixed, this is ready to be merged.
collector: | ||
options: | ||
kafka: # <1> | ||
producer: |
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.
This is the strongest argument in favor of strong typing for CLI options, IMO
README.adoc
Outdated
@@ -262,6 +269,8 @@ spec: | |||
<1> Identifies the kafka configuration used by the collector, to produce the messages, and the ingester to consume the messages | |||
<2> The deadlock interval can be disabled to avoid the ingester being terminated when no messages arrive within the default 1 minute period | |||
|
|||
NOTE: A Kafka environment can be configured using link:https://strimzi.io/[Strimzi's Kafka operator]. |
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.
TIP
instead of NOTE
?
https://asciidoctor.org/docs/user-manual/#admonition
Signed-off-by: Gary Brown <[email protected]>
Codecov Report
@@ Coverage Diff @@
## master #390 +/- ##
==========================================
- Coverage 89.72% 89.71% -0.01%
==========================================
Files 64 64
Lines 3094 3092 -2
==========================================
- Hits 2776 2774 -2
Misses 216 216
Partials 102 102
Continue to review full report at Codecov.
|
The Kafka options were changed in Jaeger 1.11, to use specific properties for consumer and producer sides. The existing shared properties have been deprecated.
With the shared properties, it was possible for have a simplified streaming CR that only defined the kafka properties in storage and/or ingester components - however now the properties are specific to the producer/consumer, they have to be explicitly listed in the relevant component's options.
Unfortunately there is no way to make this backward compatible, as otherwise unexpected properties are passed to the collector. So this will need to be a breaking change - although only affects the streaming stragegy and kafka options.
Tested using strimzi. Next step will be to provide some e2e tests for streaming.
Signed-off-by: Gary Brown [email protected]