-
Notifications
You must be signed in to change notification settings - Fork 580
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
[3.x] - Simple to Batch Span exporter #7419
[3.x] - Simple to Batch Span exporter #7419
Conversation
Signed-off-by: Dmitry Aleksandrov <[email protected]>
@@ -299,6 +306,11 @@ public JaegerTracerBuilder config(Config config) { | |||
|
|||
config.get("global").asBoolean().ifPresent(this::registerGlobal); | |||
|
|||
config.get("exporter-timeout-millis").asLong().ifPresent(it -> exporterTimeout(Duration.ofMillis(it))); |
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.
switch to Duration instead of millis
- so the property should be exporter-timeout
(same for schedule delay)
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.
fixed
@@ -462,7 +510,12 @@ public Tracer build() { | |||
Resource serviceName = Resource.create(attributesBuilder.build()); | |||
OpenTelemetry ot = OpenTelemetrySdk.builder() | |||
.setTracerProvider(SdkTracerProvider.builder() | |||
.addSpanProcessor(SimpleSpanProcessor.create(exporter)) | |||
.addSpanProcessor(BatchSpanProcessor.builder(exporter) |
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.
It would be better to have a choice than a hardcoded option. Esp. for testing the SimpleSpanProcessor
is better.
So maybe introduce span-processor-type
that would be simple
or batch
(not sure if other supported).
Then use the one chosen (and we can have batch
as default, if that is better for production)
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.
fixed
Signed-off-by: Dmitry Aleksandrov <[email protected]>
Signed-off-by: Dmitry Aleksandrov <[email protected]>
* @return updated builder | ||
*/ | ||
@ConfiguredOption(key = "span-processor-type", value = "batch") | ||
public JaegerTracerBuilder spanProcessorType(String spanProcessorType) { |
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.
You should use the SpanProcessorType
as the parameter here.
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.
fixed
Signed-off-by: Dmitry Aleksandrov <[email protected]>
Signed-off-by: Dmitry Aleksandrov <[email protected]>
Signed-off-by: Dmitry Aleksandrov <[email protected]>
Signed-off-by: Dmitry Aleksandrov <[email protected]>
Resolves internal request to switch to Batch Span Exporter to Jaeger tracer. This should significantly improve performance.
Doc impact: new config properties