Conversation
|
| if (Sentry.isGlobalHubMode()) { | ||
| if (result == null | ||
| || (result instanceof Span && !((Span) result).getSpanContext().isValid())) { | ||
| if (isOpentelemetrySpan(contextKey)) { |
There was a problem hiding this comment.
Played around with this too much. It should check whether the key refers to a Span earlier.
Performance metrics 🚀
|
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| e3d2723 | 425.58 ms | 481.35 ms | 55.76 ms |
| fef532b | 444.25 ms | 466.31 ms | 22.06 ms |
| d0dccd7 | 399.63 ms | 437.92 ms | 38.29 ms |
| bdc74cc | 531.79 ms | 677.23 ms | 145.44 ms |
| 0eb29b3 | 431.80 ms | 455.50 ms | 23.70 ms |
| 0d57111 | 387.49 ms | 414.13 ms | 26.64 ms |
| 0f2f4fd | 395.67 ms | 444.42 ms | 48.75 ms |
| 9ab9244 | 449.06 ms | 501.39 ms | 52.33 ms |
| 53db701 | 399.88 ms | 423.54 ms | 23.66 ms |
App size
| Revision | Plain | With Sentry | Diff |
|---|---|---|---|
| e3d2723 | 1.58 MiB | 2.08 MiB | 507.28 KiB |
| fef532b | 1.58 MiB | 2.08 MiB | 507.45 KiB |
| d0dccd7 | 1.58 MiB | 2.08 MiB | 507.67 KiB |
| bdc74cc | 1.58 MiB | 2.08 MiB | 507.34 KiB |
| 0eb29b3 | 1.58 MiB | 2.08 MiB | 507.44 KiB |
| 0d57111 | 1.58 MiB | 2.08 MiB | 507.45 KiB |
| 0f2f4fd | 1.58 MiB | 2.08 MiB | 507.44 KiB |
| 9ab9244 | 1.58 MiB | 2.08 MiB | 507.92 KiB |
| 53db701 | 1.58 MiB | 2.08 MiB | 507.28 KiB |
lbloder
left a comment
There was a problem hiding this comment.
Minor comment about param naming.
Otherwise LGTM 👍
| } | ||
|
|
||
| private <V> boolean shouldReturnRootSpanInstead( | ||
| final @NotNull ContextKey<V> contextKey, final @Nullable V result) { |
There was a problem hiding this comment.
(l) not sure about the param naming. Would something other than result be more fitting?
| } | ||
|
|
||
| @SuppressWarnings("unchecked") | ||
| private <V> @Nullable V returnUnfinishedRootSpanIfAvailable(final @Nullable V result) { |
There was a problem hiding this comment.
(l) not sure about the param naming. Would something other than result be more fitting?
…ry API as fallback root
|
|
||
| ### Features | ||
|
|
||
| - Support `globalHubMode` for OpenTelemetry ([#4349](https://github.com/getsentry/sentry-java/pull/4349)) |
There was a problem hiding this comment.
- 🚫 The changelog entry seems to be part of an already released section
## 8.11.0.
Consider moving the entry to the## Unreleasedsection, please.
|
Closing this for now since it didn't help the customer. If we need this again, we can reopen. |
|
In case this needs Sentry scopes added to OpenTelemetry |
We're planning to release an alpha version from this branch so this PR shouldn't be merged to
mainyet.📜 Description
Contextfrom OpenTelemetryContextStorageifglobalHubModeis enabledContext.getif there is no (valid) span stored on the context💡 Motivation and Context
Using Sentry with OpenTelemetry for Java Desktop apps where customers have no control over their scheduling requires
globalHubModeto ensure e.g. outgoing HTTP client spans are attached to the root span / transaction that is currently running.Without this fix, HTTP client spans are recorded as separate Sentry transactions due to OpenTelemetry
Contextnot being propagated e.g. when usingnew Thread(new Runnable() { ... }).start().To fix this it's also possible to manually transfer the OpenTelemetry
Contextbut not every customer has control over their scheduling:💚 How did you test it?
📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps