Skip to content

Conversation

markushi
Copy link
Member

@markushi markushi commented Aug 28, 2025

📜 Description

Implements support for adding the w3c traceparent HTTP header.

Based on https://develop.sentry.dev/sdk/telemetry/traces and https://develop.sentry.dev/sdk/telemetry/traces/distributed-tracing

💡 Motivation and Context

Implements #4678

💚 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

Copy link
Contributor

github-actions bot commented Aug 28, 2025

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

Generated by 🚫 dangerJS against d00f61e

Copy link
Contributor

github-actions bot commented Aug 28, 2025

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 384.94 ms 421.71 ms 36.78 ms
Size 1.58 MiB 2.10 MiB 532.31 KiB

Baseline results on branch: main

Startup times

Revision Plain With Sentry Diff
b750b96 421.25 ms 444.09 ms 22.84 ms
674d437 355.28 ms 504.18 ms 148.90 ms
ee747ae 358.21 ms 389.41 ms 31.20 ms
ee747ae 396.82 ms 441.67 ms 44.86 ms
85d7417 347.21 ms 394.35 ms 47.15 ms
3699cd5 423.60 ms 495.52 ms 71.92 ms
ee747ae 386.94 ms 431.43 ms 44.49 ms
ee747ae 554.98 ms 611.50 ms 56.52 ms
ee747ae 382.73 ms 435.41 ms 52.68 ms
ee747ae 374.71 ms 455.18 ms 80.47 ms

App size

Revision Plain With Sentry Diff
b750b96 1.58 MiB 2.10 MiB 533.20 KiB
674d437 1.58 MiB 2.10 MiB 530.94 KiB
ee747ae 1.58 MiB 2.10 MiB 530.95 KiB
ee747ae 1.58 MiB 2.10 MiB 530.95 KiB
85d7417 1.58 MiB 2.10 MiB 533.44 KiB
3699cd5 1.58 MiB 2.10 MiB 533.45 KiB
ee747ae 1.58 MiB 2.10 MiB 530.95 KiB
ee747ae 1.58 MiB 2.10 MiB 530.95 KiB
ee747ae 1.58 MiB 2.10 MiB 530.95 KiB
ee747ae 1.58 MiB 2.10 MiB 530.95 KiB

Previous results on branch: feat/w3c-trace-header

Startup times

Revision Plain With Sentry Diff
065c8b3 401.33 ms 456.09 ms 54.76 ms
032a83e 372.32 ms 420.27 ms 47.95 ms
3b2c046 366.37 ms 455.58 ms 89.22 ms
88b4829 484.45 ms 577.06 ms 92.61 ms
2149101 353.42 ms 411.27 ms 57.84 ms
7e94d68 372.78 ms 450.06 ms 77.28 ms
6aed485 373.33 ms 439.26 ms 65.93 ms
2ee7842 383.95 ms 440.12 ms 56.17 ms
80398b6 390.02 ms 441.79 ms 51.77 ms

App size

Revision Plain With Sentry Diff
065c8b3 1.58 MiB 2.10 MiB 533.39 KiB
032a83e 1.58 MiB 2.10 MiB 533.16 KiB
3b2c046 1.58 MiB 2.10 MiB 533.43 KiB
88b4829 1.58 MiB 2.10 MiB 533.42 KiB
2149101 1.58 MiB 2.10 MiB 533.43 KiB
7e94d68 1.58 MiB 2.10 MiB 533.16 KiB
6aed485 1.58 MiB 2.10 MiB 533.39 KiB
2ee7842 1.58 MiB 2.10 MiB 533.38 KiB
80398b6 1.58 MiB 2.10 MiB 533.39 KiB

@markushi markushi marked this pull request as ready for review August 29, 2025 06:06
cursor[bot]

This comment was marked as outdated.

@alexander-alderman-webb
Copy link
Contributor

Would it make sense to add unit tests for the Ktor and Apollo integrations as well, similar to the new ones in the OkHttp integration?

cursor[bot]

This comment was marked as outdated.

Copy link
Member

@lcian lcian left a comment

Choose a reason for hiding this comment

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

Changes look good to me.

I'm wondering if we should take this option into account in OtelSentryPropagator::inject too.
On one hand, it would make things more consistent.
On the other, OTEL users can just use the W3C propagator which will both extract from incoming requests and inject into outgoing ones. We wouldn't want to conflict with it. Also our approach would only inject which might not be what users expect.

So, overall I think the current approach to exclude our OTEL propagator is fine.

@lcian
Copy link
Member

lcian commented Sep 8, 2025

@markushi after discussing this with @adinauer we think it's better to add this to the OtelSentryPropagator::inject method too.
The OTEL and Sentry trace and span ids should always be in sync by design, so there should be no problem of injecting different values.
Could you please add it to this (or a separate) PR?

@adinauer
Copy link
Member

adinauer commented Sep 8, 2025

We just discussed in daily. Adding support for traceparent on JVM side (Desktop / Backend) isn't a priority at the moment. We can also skip it for now if you prefer.

@markushi markushi enabled auto-merge (squash) September 10, 2025 06:18
@markushi markushi merged commit 278b42e into main Sep 10, 2025
44 checks passed
@markushi markushi deleted the feat/w3c-trace-header branch September 10, 2025 06:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants