Skip to content
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.

Sign up for GitHub

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

Merged
merged 7 commits into from
Aug 29, 2023
Merged

[3.x] - Simple to Batch Span exporter #7419

Changes from 1 commit
Commits
Show all changes
7 commits
Select commit Hold shift + click to select a range
16a88ab
Simple to Batch Span exporter
dalexandrov Aug 22, 2023
f789aaa
Add config for Span processor
dalexandrov Aug 23, 2023
395b52b
Add tests
dalexandrov Aug 23, 2023
e58757a
Set Enum in Method
dalexandrov Aug 24, 2023
abefdd1
Apply comments
dalexandrov Aug 26, 2023
c492c60
Remove millis
dalexandrov Aug 28, 2023
be09549
Fix Duration
dalexandrov Aug 29, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 57 additions & 4 deletions tracing/jaeger/src/main/java/io/helidon/tracing/jaeger/JaegerTracerBuilder.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -47,7 +47,7 @@
import io.opentelemetry.sdk.OpenTelemetrySdk;
import io.opentelemetry.sdk.resources.Resource;
import io.opentelemetry.sdk.trace.SdkTracerProvider;
import io.opentelemetry.sdk.trace.export.SimpleSpanProcessor;
import io.opentelemetry.sdk.trace.export.BatchSpanProcessor;
import io.opentelemetry.sdk.trace.export.SpanExporter;
import io.opentelemetry.sdk.trace.samplers.Sampler;
import io.opentelemetry.semconv.resource.attributes.ResourceAttributes;
Expand Down Expand Up @@ -154,6 +154,10 @@ public class JaegerTracerBuilder implements TracerBuilder<JaegerTracerBuilder> {
static final String DEFAULT_HTTP_HOST = "localhost";
static final int DEFAULT_HTTP_PORT = 14250;

static final long DEFAULT_SCHEDULE_DELAY_MILLIS = 30_000;
static final int DEFAULT_MAX_QUEUE_SIZE = 2048;
static final int DEFAULT_MAX_EXPORT_BATCH_SIZE = 512;

private final Map<String, String> tags = new HashMap<>();
// this is a backward incompatible change, but the correct choice is Jaeger, not B3
private final Set<PropagationFormat> propagationFormats = EnumSet.noneOf(PropagationFormat.class);
Expand All @@ -165,13 +169,17 @@ public class JaegerTracerBuilder implements TracerBuilder<JaegerTracerBuilder> {
private Number samplerParam = 1;
private boolean enabled = DEFAULT_ENABLED;
private boolean global = true;
private Duration exporterTimeout = Duration.ofSeconds(10);
private byte[] privateKey;
private byte[] certificate;
private byte[] trustedCertificates;
private String path;
private Config config;

private Duration exporterTimeout = Duration.ofSeconds(10);
private Duration scheduleDelayMillis = Duration.ofMillis(DEFAULT_SCHEDULE_DELAY_MILLIS);
private int maxQueueSize = DEFAULT_MAX_QUEUE_SIZE;
private int maxExportBatchSize = DEFAULT_MAX_EXPORT_BATCH_SIZE;

/**
* Default constructor, does not modify any state.
*/
Expand Down Expand Up @@ -264,7 +272,6 @@ public JaegerTracerBuilder config(Config config) {
config.get("path").asString().ifPresent(this::collectorPath);
config.get("sampler-type").asString().as(SamplerType::create).ifPresent(this::samplerType);
config.get("sampler-param").asDouble().ifPresent(this::samplerParam);
config.get("exporter-timeout-millis").asLong().ifPresent(it -> exporterTimeout(Duration.ofMillis(it)));
config.get("private-key-pem").as(io.helidon.common.configurable.Resource::create).ifPresent(this::privateKey);
config.get("client-cert-pem").as(io.helidon.common.configurable.Resource::create).ifPresent(this::clientCertificate);
config.get("trusted-cert-pem").as(io.helidon.common.configurable.Resource::create).ifPresent(this::trustedCertificates);
Expand Down Expand Up @@ -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)));
Copy link
Member

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)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

config.get("schedule-delay-millis").asInt().ifPresent(it -> scheduleDelayMillis(Duration.ofMillis(it)));
config.get("max-queue-size").asInt().ifPresent(this::maxQueueSize);
config.get("max-export-batch-size").asInt().ifPresent(this::maxExportBatchSize);

return this;
}

Expand Down Expand Up @@ -350,6 +362,42 @@ public JaegerTracerBuilder exporterTimeout(Duration exporterTimeout) {
return this;
}

/**
* Schedule Delay of exporter requests.
*
* @param scheduleDelayMillis timeout to use
* @return updated builder
*/
@ConfiguredOption(key = "schedule-delay-millis", value = "5000")
public JaegerTracerBuilder scheduleDelayMillis(Duration scheduleDelayMillis) {
this.scheduleDelayMillis = scheduleDelayMillis;
return this;
}

/**
* Maximum Queue Size of exporter requests.
*
* @param maxQueueSize to use
* @return updated builder
*/
@ConfiguredOption(key = "max-queue-size", value = "2048")
public JaegerTracerBuilder maxQueueSize(int maxQueueSize) {
this.maxQueueSize = maxQueueSize;
return this;
}

/**
* Maximum Export Batch Size of exporter requests.
*
* @param maxExportBatchSize to use
* @return updated builder
*/
@ConfiguredOption(key = "max-export-batch-size", value = "512")
public JaegerTracerBuilder maxExportBatchSize(int maxExportBatchSize) {
this.maxExportBatchSize = maxExportBatchSize;
return this;
}

@Override
public JaegerTracerBuilder enabled(boolean enabled) {
this.enabled = enabled;
Expand Down Expand Up @@ -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)
Copy link
Member

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)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

fixed

.setScheduleDelay(scheduleDelayMillis)
.setMaxQueueSize(maxQueueSize)
.setMaxExportBatchSize(maxExportBatchSize)
.setExporterTimeout(exporterTimeout)
.build())
.setSampler(sampler)
.setResource(serviceName)
.build())
Expand Down