-
Notifications
You must be signed in to change notification settings - Fork 936
Issue 7869 - Support the new W3C random flag #8012
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
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||
|---|---|---|---|---|---|---|
|
|
@@ -76,6 +76,18 @@ static TraceFlags fromByte(byte traceFlagsByte) { | |||||
| */ | ||||||
| boolean isSampled(); | ||||||
|
|
||||||
| /** | ||||||
| * Returns {@code true} if the TraceId accompanying this {@link TraceFlags} is known to be | ||||||
| * generated by a truly random Id generator, otherwise {@code false}. Providing default | ||||||
| * implementation just to maintain compatibility. | ||||||
| * | ||||||
| * @return {@code true} if the samplingrandomTraceId bit is on for this {@link TraceFlags}, | ||||||
| * otherwise {@code false}. | ||||||
| */ | ||||||
| default boolean isTraceIdRandom() { | ||||||
| return false; | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Returns the lowercase hex (base16) representation of this {@link TraceFlags}. | ||||||
| * | ||||||
|
|
@@ -89,4 +101,26 @@ static TraceFlags fromByte(byte traceFlagsByte) { | |||||
| * @return the byte representation of the {@link TraceFlags}. | ||||||
| */ | ||||||
| byte asByte(); | ||||||
|
|
||||||
| /** | ||||||
| * Returns a new instance of {@link TraceFlags} whose value is the result of a bitwise OR between | ||||||
| * this object and the SAMPLED bit. This operation does not modify this object/ | ||||||
| * | ||||||
| * @return a new {@link TraceFlags} object representing {@code this | SAMPLED_BIT} | ||||||
| */ | ||||||
| default TraceFlags withSampledBit() { | ||||||
| byte newByte = (byte) (asByte() | ImmutableTraceFlags.SAMPLED_BIT); | ||||||
| return ImmutableTraceFlags.fromByte(newByte); | ||||||
| } | ||||||
|
|
||||||
| /** | ||||||
| * Returns a new instance of {@link TraceFlags} whose value is the result of a bitwise OR between | ||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. #nit: new may imply object construction. Same advice applies to
Suggested change
|
||||||
| * this object and the RANDOM_TRACE_ID bit. This operation does not modify this object. | ||||||
| * | ||||||
| * @return a new {@link TraceFlags} object representing {@code this | RANDOM_TRACE_ID_BIT} | ||||||
| */ | ||||||
| default TraceFlags withRandomTraceIdBit() { | ||||||
| byte newByte = (byte) (asByte() | ImmutableTraceFlags.RANDOM_TRACE_ID_BIT); | ||||||
| return ImmutableTraceFlags.fromByte(newByte); | ||||||
| } | ||||||
| } | ||||||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,6 @@ | ||
| Comparing source compatibility of opentelemetry-api-1.59.0-SNAPSHOT.jar against opentelemetry-api-1.58.0.jar | ||
| No changes. | ||
| *** MODIFIED INTERFACE: PUBLIC ABSTRACT io.opentelemetry.api.trace.TraceFlags (not serializable) | ||
| === CLASS FILE FORMAT VERSION: 52.0 <- 52.0 | ||
| +++ NEW METHOD: PUBLIC(+) boolean isTraceIdRandom() | ||
| +++ NEW METHOD: PUBLIC(+) io.opentelemetry.api.trace.TraceFlags withRandomTraceIdBit() | ||
| +++ NEW METHOD: PUBLIC(+) io.opentelemetry.api.trace.TraceFlags withSampledBit() |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,2 +1,4 @@ | ||
| Comparing source compatibility of opentelemetry-sdk-trace-1.59.0-SNAPSHOT.jar against opentelemetry-sdk-trace-1.58.0.jar | ||
| No changes. | ||
| *** MODIFIED INTERFACE: PUBLIC ABSTRACT io.opentelemetry.sdk.trace.IdGenerator (not serializable) | ||
| === CLASS FILE FORMAT VERSION: 52.0 <- 52.0 | ||
| +++ NEW METHOD: PUBLIC(+) boolean generatesRandomTraceIds() |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,8 +16,10 @@ | |
| import io.opentelemetry.api.trace.Span; | ||
| import io.opentelemetry.api.trace.SpanBuilder; | ||
| import io.opentelemetry.api.trace.SpanContext; | ||
| import io.opentelemetry.api.trace.SpanId; | ||
| import io.opentelemetry.api.trace.SpanKind; | ||
| import io.opentelemetry.api.trace.TraceFlags; | ||
| import io.opentelemetry.api.trace.TraceId; | ||
| import io.opentelemetry.api.trace.TraceState; | ||
| import io.opentelemetry.context.Context; | ||
| import io.opentelemetry.sdk.common.InstrumentationScopeInfo; | ||
|
|
@@ -170,14 +172,29 @@ public Span startSpan() { | |
| Span parentSpan = Span.fromContext(parentContext); | ||
| SpanContext parentSpanContext = parentSpan.getSpanContext(); | ||
| String traceId; | ||
| boolean isTraceIdRandom; | ||
| IdGenerator idGenerator = tracerSharedState.getIdGenerator(); | ||
| String spanId = idGenerator.generateSpanId(); | ||
|
|
||
| Context parentContextForSampler = parentContext; | ||
| if (!parentSpanContext.isValid()) { | ||
| // New root span. | ||
| traceId = idGenerator.generateTraceId(); | ||
| if (idGenerator.generatesRandomTraceIds()) { | ||
| isTraceIdRandom = true; | ||
| // Replace parentContext for sampling with a temporary one with RANDOM_TRACE_ID bit set | ||
| parentContextForSampler = | ||
| preparePrimordialContext( | ||
| parentContext, | ||
| TraceFlags.getDefault().withRandomTraceIdBit(), | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Similar to this comment, we should use singletons here to avoid allocations on the hot path. Ideally the entire primordial |
||
| TraceState.getDefault()); | ||
| } else { | ||
| isTraceIdRandom = false; | ||
| } | ||
| } else { | ||
| // New child span. | ||
| traceId = parentSpanContext.getTraceId(); | ||
| isTraceIdRandom = parentSpanContext.getTraceFlags().isTraceIdRandom(); | ||
| } | ||
| List<LinkData> currentLinks = links; | ||
| List<LinkData> immutableLinks = | ||
|
|
@@ -190,7 +207,12 @@ public Span startSpan() { | |
| tracerSharedState | ||
| .getSampler() | ||
| .shouldSample( | ||
| parentContext, traceId, spanName, spanKind, immutableAttributes, immutableLinks); | ||
| parentContextForSampler, | ||
| traceId, | ||
| spanName, | ||
| spanKind, | ||
| immutableAttributes, | ||
| immutableLinks); | ||
| SamplingDecision samplingDecision = samplingResult.getDecision(); | ||
|
|
||
| TraceState samplingResultTraceState = | ||
|
|
@@ -199,7 +221,7 @@ public Span startSpan() { | |
| ImmutableSpanContext.create( | ||
| traceId, | ||
| spanId, | ||
| isSampled(samplingDecision) ? TraceFlags.getSampled() : TraceFlags.getDefault(), | ||
| newTraceFlags(isTraceIdRandom, isSampled(samplingDecision)), | ||
jack-berg marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| samplingResultTraceState, | ||
| /* remote= */ false, | ||
| tracerSharedState.isIdGeneratorSafeToSkipIdValidation()); | ||
|
|
@@ -239,6 +261,29 @@ public Span startSpan() { | |
| recordEndSpanMetrics); | ||
| } | ||
|
|
||
| /* | ||
| * A primordial context can be passed as the parent context for a root span | ||
| * if a non-default TraceFlags or TraceState need to be passed to the sampler | ||
| */ | ||
| private static Context preparePrimordialContext( | ||
| Context parentContext, TraceFlags traceFlags, TraceState traceState) { | ||
| SpanContext spanContext = | ||
| SpanContext.create(TraceId.getInvalid(), SpanId.getInvalid(), traceFlags, traceState); | ||
| Span span = Span.wrap(spanContext); | ||
| return span.storeInContext(parentContext); | ||
| } | ||
|
|
||
| private static TraceFlags newTraceFlags(boolean randomTraceId, boolean sampled) { | ||
| TraceFlags traceFlags = TraceFlags.getDefault(); | ||
| if (randomTraceId) { | ||
| traceFlags = traceFlags.withRandomTraceIdBit(); | ||
| } | ||
| if (sampled) { | ||
| traceFlags = traceFlags.withSampledBit(); | ||
| } | ||
| return traceFlags; | ||
| } | ||
|
|
||
| private AttributesMap attributes() { | ||
| AttributesMap attributes = this.attributes; | ||
| if (attributes == null) { | ||
|
|
||
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.
Is
samplingrandomTraceIdsupposed to reference a constant?