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

          .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

          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

  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

            ? 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

    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

    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

  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

        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 lcian, markushi, romtsn and stefanosiano 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

Uh oh!

Propagate sampling random value #4153

Propagate sampling random value #4153

Uh oh!

Conversation

adinauer commented Feb 10, 2025

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

🔮 Next steps

Uh oh!

github-actions bot commented Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

github-actions bot commented Feb 10, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: feat/sample_rand

Startup times

App size

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

github-actions bot commented Feb 10, 2025 •

edited

Loading

github-actions bot commented Feb 10, 2025 •

edited

Loading