Skip to content
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

Add OpenTelemetryLinkErrorEventProcessor for linking errors to traces created via OpenTelemetry #2418

Conversation

adinauer
Copy link
Member

@adinauer adinauer commented Dec 6, 2022

📜 Description

Retrieve current Span from OpenTelemetry, then look up the Sentry span via spanId and add traceId, spanId, parentSpanId and op to the error events context keyed "trace".

Had to move SentrySpanStorage from sentry-opentelemetry-core module to sentry module to avoid yet another module just for providing SentrySpanStorage in the bootstrap classloader.

💡 Motivation and Context

With this change, Sentry UI can now show a link from transaction to error event and vice versa.

💚 How did you test it?

Manually via Spring Boot Sample

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • No breaking changes

🔮 Next steps

@github-actions
Copy link
Contributor

github-actions bot commented Dec 6, 2022

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

Generated by 🚫 dangerJS against bc2ec93

@github-actions
Copy link
Contributor

github-actions bot commented Dec 6, 2022

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 339.38 ms 359.45 ms 20.07 ms
Size 1.73 MiB 2.32 MiB 612.36 KiB

Baseline results on branch: feat/allow-turning-off-auto-init-in-otel-agent

Startup times

Revision Plain With Sentry Diff
10bb0b0 305.47 ms 334.56 ms 29.09 ms
fd9d617 319.02 ms 351.73 ms 32.71 ms

App size

Revision Plain With Sentry Diff
10bb0b0 1.73 MiB 2.32 MiB 612.39 KiB
fd9d617 1.73 MiB 2.32 MiB 612.36 KiB

Previous results on branch: feat/link-errors-to-otel-created-traces

Startup times

Revision Plain With Sentry Diff
7c14f70 322.24 ms 375.61 ms 53.37 ms

App size

Revision Plain With Sentry Diff
7c14f70 1.73 MiB 2.32 MiB 612.36 KiB

@codecov-commenter
Copy link

Codecov Report

Base: 80.03% // Head: 79.87% // Decreases project coverage by -0.16% ⚠️

Coverage data is based on head (1cc35c7) compared to base (c787a0f).
Patch coverage: 0.00% of modified lines in pull request are covered.

Additional details and impacted files
@@                                 Coverage Diff                                  @@
##             feat/allow-turning-off-auto-init-in-otel-agent    #2418      +/-   ##
====================================================================================
- Coverage                                             80.03%   79.87%   -0.17%     
+ Complexity                                             3765     3759       -6     
====================================================================================
  Files                                                   301      302       +1     
  Lines                                                 14207    14222      +15     
  Branches                                               1884     1886       +2     
====================================================================================
- Hits                                                  11371    11360      -11     
- Misses                                                 2092     2119      +27     
+ Partials                                                744      743       -1     
Impacted Files Coverage Δ
...elemetry/OpenTelemetryLinkErrorEventProcessor.java 0.00% <0.00%> (ø)
...java/io/sentry/opentelemetry/SentryPropagator.java 10.25% <ø> (ø)
...a/io/sentry/opentelemetry/SentrySpanProcessor.java 76.69% <ø> (ø)
...try/src/main/java/io/sentry/SentrySpanStorage.java 0.00% <ø> (ø)

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

@adinauer adinauer mentioned this pull request Dec 6, 2022
21 tasks
Copy link
Member

@markushi markushi left a comment

Choose a reason for hiding this comment

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

LGTM!

parentSpanId,
null);

event.getContexts().setTrace(spanContext);
Copy link
Member

@markushi markushi Dec 6, 2022

Choose a reason for hiding this comment

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

l: I'm probably missing some context here, but is it fine to override the existing trace context?

Copy link
Member Author

Choose a reason for hiding this comment

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

I'd say yes because it only does so in case instrumenter is set to otel, there's an OTEL span with valid IDs and it can find the Sentry span in the shared storage. For now we don't allow starting transactions outside of OTEL if instrumenter is set to otel. So there shouldn't be anything worth keeping in there.

@adinauer adinauer merged commit 3814df6 into feat/allow-turning-off-auto-init-in-otel-agent Dec 6, 2022
@adinauer adinauer deleted the feat/link-errors-to-otel-created-traces branch December 6, 2022 14:46
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.

None yet

3 participants