Propagate sampling random value #4153

adinauer · 2025-02-10T09:28:51Z

📜 Description

Propagate sampling random value to downstream systems (sentry-sample_rand in baggage) and Sentry (sample_rand in envelope header).

Use incoming sentry-sample_rand.

Backfill sampling random value in case it is missing.

💡 Motivation and Context

Closes #4115
Closes #4032

💚 How did you test it?

📝 Checklist

I added tests to verify the changes.
No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
I updated the docs if needed.
I updated the wizard if needed.
Review from the native team if needed.
No breaking change or entry added to the changelog.
No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

github-actions · 2025-02-10T09:29:25Z

	Messages
📖	Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 85768d6

github-actions · 2025-02-10T09:38:20Z

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	434.67 ms	500.98 ms	66.31 ms
Size	1.58 MiB	2.21 MiB	640.91 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`c2c78de`	415.28 ms	505.08 ms	89.80 ms

App size

Revision	Plain	With Sentry	Diff
`c2c78de`	1.58 MiB	2.21 MiB	640.27 KiB

Previous results on branch: feat/sample_rand

Startup times

Revision	Plain	With Sentry	Diff
`b5d63c8`	384.39 ms	443.98 ms	59.59 ms
`b674478`	399.19 ms	482.67 ms	83.48 ms
`b4d2996`	319.66 ms	369.94 ms	50.28 ms
`473d6ab`	446.68 ms	464.90 ms	18.22 ms
`7c29bb1`	647.18 ms	767.43 ms	120.25 ms
`4e41178`	388.33 ms	531.18 ms	142.86 ms

App size

Revision	Plain	With Sentry	Diff
`b5d63c8`	1.58 MiB	2.21 MiB	641.09 KiB
`b674478`	1.58 MiB	2.21 MiB	640.91 KiB
`b4d2996`	1.58 MiB	2.21 MiB	640.92 KiB
`473d6ab`	1.58 MiB	2.21 MiB	640.93 KiB
`7c29bb1`	1.58 MiB	2.21 MiB	640.92 KiB
`4e41178`	1.58 MiB	2.21 MiB	641.07 KiB

adinauer · 2025-02-10T09:29:56Z

...ry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSentryPropagator.java

-        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


Left this as a TODO for the future. We could start looking at span attributes to retrieve the URL of the downstream system.

adinauer · 2025-02-10T09:30:33Z

...ry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSentryPropagator.java

@@ -125,11 +130,6 @@ public <C> Context extract(
          .getLogger()
          .log(SentryLevel.DEBUG, "Continuing Sentry trace %s", sentryTraceHeader.getTraceId());

-      final @NotNull PropagationContext propagationContext =


Since this causes baggage to be frozen immediately, we no longer want to do this in the propagator.

adinauer · 2025-02-10T09:31:18Z

...ry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSentryPropagator.java

-    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 =


This ensures baggage is frozen when sending out headers. Not freezing was a bug.

adinauer · 2025-02-10T09:34:57Z

...sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSentrySpanProcessor.java

@@ -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();


We don't want to send the wrong baggage into the OtelSpanWrapper ctor further down.

adinauer · 2025-02-10T09:35:27Z

...lemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/OtelSpanWrapper.java

@@ -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;


No need to keep this as a property here. We can always ask the context for it.

adinauer · 2025-02-10T09:43:14Z

sentry/src/main/java/io/sentry/Baggage.java

@@ -123,7 +128,12 @@ public static Baggage fromHeader(
        thirdPartyKeyValueStrings.isEmpty()
            ? null
            : StringUtils.join(",", thirdPartyKeyValueStrings);
-    return new Baggage(keyValues, thirdPartyHeader, mutable, logger);
+    /*


When creating Baggage from headers we now ignore the sample random value entry and instead of freezing right away we just mark the baggage for being frozen. This shouldFreeze flag is then checked when passing baggage into either PropagationContext or TransactionContext and frozen if true.

adinauer · 2025-02-10T09:48:35Z

sentry/src/main/java/io/sentry/SentryTracer.java

@@ -81,12 +80,6 @@ public SentryTracer(
    this.transactionNameSource = context.getTransactionNameSource();
    this.transactionOptions = transactionOptions;

-    if (context.getBaggage() != null) {


We're relying on TransactionContext to bring in baggage here. Instead of holding on directly, we're using context to access it.

adinauer · 2025-02-10T09:51:11Z

sentry/src/main/java/io/sentry/TransactionContext.java

@@ -90,6 +93,7 @@ public TransactionContext(
    this.name = Objects.requireNonNull(name, "name is required");
    this.transactionNameSource = transactionNameSource;
    this.setSamplingDecision(samplingDecision);
+    this.baggage = TracingUtils.ensureBaggage(null, samplingDecision);


Takes care of creating a new Baggage if missing, backfilling sample random value and freezing baggage if marked for freezing.

We can't force customers to go through Sentry.continueTrace which would create a PropagationContext so we also have to handle this here. This is something we should improve when replacing the Performance API.

adinauer · 2025-02-10T09:52:30Z

sentry/src/main/java/io/sentry/PropagationContext.java

@@ -56,7 +60,7 @@ public static PropagationContext fromHeaders(

  private @Nullable Boolean sampled;

-  private @Nullable Baggage baggage;
+  private final @NotNull Baggage baggage;


Changed this to non null, so we don't have to handle null everywhere we access this.

adinauer · 2025-02-10T10:37:06Z

sentry/src/main/java/io/sentry/PropagationContext.java

@@ -67,18 +71,10 @@ public PropagationContext(final @NotNull PropagationContext propagationContext)
        propagationContext.getTraceId(),
        propagationContext.getSpanId(),
        propagationContext.getParentSpanId(),
-        cloneBaggage(propagationContext.getBaggage()),


We were cloning baggage here for each scope fork meaning a non frozen baggage would never freeze. This might no longer be a problem after having done a lot more changes now. However I don't see a reason to clone / separate any pre-existing baggage. If anything we should make sure a new one is created when isolation scope is forked.

sentry/src/main/java/io/sentry/Baggage.java

sentry/src/test/java/io/sentry/BaggageTest.kt

sentry/src/main/java/io/sentry/TransactionContext.java

sentry/src/test/java/io/sentry/util/SampleRateUtilTest.kt

sentry/src/main/java/io/sentry/Baggage.java

sentry/src/main/java/io/sentry/OutboxSender.java

sentry/src/main/java/io/sentry/Baggage.java

sentry/src/main/java/io/sentry/TraceContext.java

sentry/src/main/java/io/sentry/util/TracingUtils.java

adinauer requested review from romtsn, stefanosiano, markushi and lcian as code owners February 10, 2025 09:28

adinauer commented Feb 10, 2025

View reviewed changes

sentry/src/main/java/io/sentry/Baggage.java Show resolved Hide resolved

adinauer mentioned this pull request Feb 10, 2025

Java SDK 8.2.0 #4114

Closed

lcian approved these changes Feb 11, 2025

View reviewed changes

adinauer added 13 commits February 12, 2025 08:22

wip

b573655

wip2

6cb3b54

wip3

15c9c8c

format + api

417ddf0

cleanup

3145877

revert change to demo

a0bd766

make baggage final again

f9bec8d

remove ObjectToString suppressing

8add76d

remove outdated comment

b03791e

remove getPropagationContext from Scopes again

18b1093

remove noop baggage and propagation context again

0cb6e0d

fix outbox sender; test; cleanup api file

c239a32

review changes

d3adc66

adinauer force-pushed the feat/sample_rand branch from bf75222 to d3adc66 Compare February 12, 2025 07:22

lbloder reviewed Feb 12, 2025

View reviewed changes

adinauer added 2 commits February 12, 2025 15:04

fix test for old span processor

8c18a63

review feedback

80884f5

lbloder approved these changes Feb 12, 2025

View reviewed changes

getsentry-bot and others added 2 commits February 12, 2025 14:44

Format code

7e8ea91

Merge branch 'main' into feat/sample_rand

ad356d8

changelog

85768d6

adinauer merged commit dc85168 into main Feb 12, 2025
35 checks passed

adinauer deleted the feat/sample_rand branch February 12, 2025 16:50

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Propagate sampling random value #4153

Propagate sampling random value #4153

adinauer commented Feb 10, 2025

github-actions bot commented Feb 10, 2025 •

edited

Loading

github-actions bot commented Feb 10, 2025 •

edited

Loading

Baseline results on branch: main

Startup times

App size

Previous results on branch: feat/sample_rand

Startup times

App size

adinauer Feb 10, 2025

adinauer Feb 10, 2025

adinauer Feb 10, 2025

adinauer Feb 10, 2025

adinauer Feb 10, 2025

adinauer Feb 10, 2025

adinauer Feb 10, 2025

adinauer Feb 10, 2025

adinauer Feb 10, 2025

adinauer Feb 10, 2025

Propagate sampling random value #4153

Propagate sampling random value #4153

Conversation

adinauer commented Feb 10, 2025

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

🔮 Next steps

github-actions bot commented Feb 10, 2025 • edited Loading

github-actions bot commented Feb 10, 2025 • edited Loading

Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: feat/sample_rand

Startup times

App size

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

github-actions bot commented Feb 10, 2025 •

edited

Loading

github-actions bot commented Feb 10, 2025 •

edited

Loading