-
-
Notifications
You must be signed in to change notification settings - Fork 444
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
POTEL 37b - No longer selectively copy OTel span attributes #3663
POTEL 37b - No longer selectively copy OTel span attributes #3663
Conversation
# Conflicts: # sentry-opentelemetry/sentry-opentelemetry-core/src/main/java/io/sentry/opentelemetry/SpanDescriptionExtractor.java
…noring sentry specific ones
…PROCESS_COMMAND_ARGS
|
Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
bfc2f88 | 456.92 ms | 530.96 ms | 74.04 ms |
9294b06 | 438.15 ms | 502.29 ms | 64.15 ms |
33d3c1b | 349.24 ms | 358.60 ms | 9.36 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
bfc2f88 | 1.70 MiB | 2.29 MiB | 599.45 KiB |
9294b06 | 1.70 MiB | 2.29 MiB | 599.31 KiB |
33d3c1b | 1.70 MiB | 2.29 MiB | 599.45 KiB |
@@ -21,35 +19,6 @@ public final class SpanDescriptionExtractor { | |||
@SuppressWarnings("deprecation") | |||
public @NotNull OtelSpanInfo extractSpanInfo( |
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.
Is the TODO above this method still relevant?
CHANGELOG.md
Outdated
@@ -13,6 +13,7 @@ | |||
- Use OpenTelemetry span name as fallback for transaction name ([#3557](https://github.com/getsentry/sentry-java/pull/3557)) | |||
- In certain cases we were sending transactions as "<unlabeled transaction>" when using OpenTelemetry | |||
- Add OpenTelemetry span data to Sentry span ([#3593](https://github.com/getsentry/sentry-java/pull/3593)) | |||
- No longer selectively copy OpenTelemetry attributes to Sentry spans `data` ([#3663](https://github.com/getsentry/sentry-java/pull/3663)) |
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.
Should we also include the fact, that we no longer set the span data on the transaction data?
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.
LGTM
📜 Description
We're copying all of attribues as of (#3593). This means we no longer need to selectively copy some of them.
💡 Motivation and Context
Remove unnecessary code.
💚 How did you test it?
📝 Checklist
sendDefaultPII
is enabled.🔮 Next steps