-
-
Notifications
You must be signed in to change notification settings - Fork 465
Propagate sampling random value #4153
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
Changes from all commits
b573655
6cb3b54
15c9c8c
417ddf0
3145877
a0bd766
f9bec8d
8add76d
b03791e
18b1093
0cb6e0d
c239a32
d3adc66
8c18a63
80884f5
7e8ea91
ad356d8
85768d6
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 |
|---|---|---|
|
|
@@ -13,12 +13,12 @@ | |
| import io.sentry.Baggage; | ||
| import io.sentry.BaggageHeader; | ||
| import io.sentry.IScopes; | ||
| import io.sentry.PropagationContext; | ||
| import io.sentry.ScopesAdapter; | ||
| import io.sentry.Sentry; | ||
| import io.sentry.SentryLevel; | ||
| import io.sentry.SentryTraceHeader; | ||
| import io.sentry.exception.InvalidSentryTraceHeaderException; | ||
| import io.sentry.util.TracingUtils; | ||
| import java.util.Arrays; | ||
| import java.util.Collection; | ||
| import java.util.Collections; | ||
|
|
@@ -73,12 +73,17 @@ public <C> void inject(final Context context, final C carrier, final TextMapSett | |
| return; | ||
| } | ||
|
|
||
| final @NotNull SentryTraceHeader sentryTraceHeader = sentrySpan.toSentryTrace(); | ||
| setter.set(carrier, sentryTraceHeader.getName(), sentryTraceHeader.getValue()); | ||
| final @Nullable BaggageHeader baggageHeader = | ||
| sentrySpan.toBaggageHeader(Collections.emptyList()); | ||
| if (baggageHeader != null) { | ||
| setter.set(carrier, baggageHeader.getName(), baggageHeader.getValue()); | ||
| // TODO can we use traceIfAllowed? do we have the URL here? need to access span attrs | ||
| final @Nullable TracingUtils.TracingHeaders tracingHeaders = | ||
|
Member
Author
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. This ensures baggage is frozen when sending out headers. Not freezing was a bug. |
||
| TracingUtils.trace(scopes, Collections.emptyList(), sentrySpan); | ||
|
|
||
| if (tracingHeaders != null) { | ||
| final @NotNull SentryTraceHeader sentryTraceHeader = tracingHeaders.getSentryTraceHeader(); | ||
| setter.set(carrier, sentryTraceHeader.getName(), sentryTraceHeader.getValue()); | ||
| final @Nullable BaggageHeader baggageHeader = tracingHeaders.getBaggageHeader(); | ||
| if (baggageHeader != null) { | ||
| setter.set(carrier, baggageHeader.getName(), baggageHeader.getValue()); | ||
| } | ||
| } | ||
| } | ||
|
|
||
|
|
@@ -125,11 +130,6 @@ public <C> Context extract( | |
| .getLogger() | ||
| .log(SentryLevel.DEBUG, "Continuing Sentry trace %s", sentryTraceHeader.getTraceId()); | ||
|
|
||
| final @NotNull PropagationContext propagationContext = | ||
|
Member
Author
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. Since this causes baggage to be frozen immediately, we no longer want to do this in the propagator. |
||
| PropagationContext.fromHeaders( | ||
| scopes.getOptions().getLogger(), sentryTraceString, baggageString); | ||
| scopesToUse.getIsolationScope().setPropagationContext(propagationContext); | ||
|
|
||
| return modifiedContext; | ||
| } catch (InvalidSentryTraceHeaderException e) { | ||
| scopes | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -70,15 +70,10 @@ public void onStart(final @NotNull Context parentContext, final @NotNull ReadWri | |
| baggage = baggageFromContext; | ||
| } | ||
|
|
||
| final @Nullable Boolean baggageMutable = | ||
| otelSpan.getAttribute(InternalSemanticAttributes.BAGGAGE_MUTABLE); | ||
| final @Nullable String baggageString = | ||
| otelSpan.getAttribute(InternalSemanticAttributes.BAGGAGE); | ||
| if (baggageString != null) { | ||
| baggage = Baggage.fromHeader(baggageString); | ||
| if (baggageMutable == true) { | ||
| baggage.freeze(); | ||
| } | ||
| } | ||
|
|
||
| final @Nullable Boolean sampled = isSampled(otelSpan, samplingDecision); | ||
|
|
@@ -87,6 +82,8 @@ public void onStart(final @NotNull Context parentContext, final @NotNull ReadWri | |
| new PropagationContext( | ||
| new SentryId(traceId), sentrySpanId, sentryParentSpanId, baggage, sampled); | ||
|
|
||
| baggage = propagationContext.getBaggage(); | ||
|
Member
Author
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. We don't want to send the wrong baggage into the |
||
|
|
||
| updatePropagationContext(scopes, propagationContext); | ||
| } | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -64,7 +64,6 @@ public final class OtelSpanWrapper implements IOtelSpanWrapper { | |
| private final @NotNull Contexts contexts = new Contexts(); | ||
| private @Nullable String transactionName; | ||
| private @Nullable TransactionNameSource transactionNameSource; | ||
| private final @Nullable Baggage baggage; | ||
|
Member
Author
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. No need to keep this as a property here. We can always ask the context for it. |
||
| private final @NotNull AutoClosableReentrantLock lock = new AutoClosableReentrantLock(); | ||
|
|
||
| private final @NotNull Map<String, Object> data = new ConcurrentHashMap<>(); | ||
|
|
@@ -86,17 +85,12 @@ public OtelSpanWrapper( | |
| this.scopes = Objects.requireNonNull(scopes, "scopes are required"); | ||
| this.span = new WeakReference<>(span); | ||
| this.startTimestamp = startTimestamp; | ||
|
|
||
| if (parentSpan != null) { | ||
| this.baggage = parentSpan.getSpanContext().getBaggage(); | ||
| } else if (baggage != null) { | ||
| this.baggage = baggage; | ||
| } else { | ||
| this.baggage = null; | ||
| } | ||
|
|
||
| final @Nullable Baggage baggageToUse = | ||
| baggage != null | ||
| ? baggage | ||
| : (parentSpan != null ? parentSpan.getSpanContext().getBaggage() : null); | ||
| this.context = | ||
| new OtelSpanContext(span, samplingDecision, parentSpan, parentSpanId, this.baggage); | ||
| new OtelSpanContext(span, samplingDecision, parentSpan, parentSpanId, baggageToUse); | ||
| } | ||
|
|
||
| @Override | ||
|
|
@@ -207,15 +201,16 @@ public OtelSpanWrapper( | |
| @Override | ||
| public @Nullable TraceContext traceContext() { | ||
| if (scopes.getOptions().isTraceSampling()) { | ||
| final @Nullable Baggage baggage = context.getBaggage(); | ||
| if (baggage != null) { | ||
| updateBaggageValues(); | ||
| updateBaggageValues(baggage); | ||
| return baggage.toTraceContext(); | ||
| } | ||
| } | ||
| return null; | ||
| } | ||
|
|
||
| private void updateBaggageValues() { | ||
| private void updateBaggageValues(final @NotNull Baggage baggage) { | ||
| try (final @NotNull ISentryLifecycleToken ignored = lock.acquire()) { | ||
| if (baggage != null && baggage.isMutable()) { | ||
| final AtomicReference<SentryId> replayIdAtomicReference = new AtomicReference<>(); | ||
|
|
@@ -238,8 +233,9 @@ private void updateBaggageValues() { | |
| @Override | ||
| public @Nullable BaggageHeader toBaggageHeader(@Nullable List<String> thirdPartyBaggageHeaders) { | ||
| if (scopes.getOptions().isTraceSampling()) { | ||
| final @Nullable Baggage baggage = context.getBaggage(); | ||
| if (baggage != null) { | ||
| updateBaggageValues(); | ||
| updateBaggageValues(baggage); | ||
| return BaggageHeader.fromBaggageAndOutgoingHeader(baggage, thirdPartyBaggageHeaders); | ||
| } | ||
| } | ||
|
|
||
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.
Left this as a TODO for the future. We could start looking at span attributes to retrieve the URL of the downstream system.