POTEL 26 - Customize OpenTelemetry Scope.close behaviour
#3517
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
#skip-changelog
📜 Description
Replace the default
ThreadLocalContextStoragewithSentryOtelThreadLocalStoragewhich hands back a differentScopeimplementation that on.close()does not check whether the current activeContextis the one it expects.💡 Motivation and Context
By default
ScopeImpl.close()in OpenTelemetry checks whether the current activeContextis the one from back whenContextwas forked. This means if during lifecycle of thisScopeanything does not clean up properly, we're stuck with an outdatedContextthat holds aSpanreference. ThisSpanhas already ended but will be used as parent for other requests coming in and breaks tracing for follow up requests.With the customized behaviour in this PR, we're now ignoring whether an inner
Scopewas not closed properly and clean up anyways.The problem with improper cleanup is not an easy one to fix. Sentry static API makes use of
getCurrentScopes()which, forks scopes and sets it on theIScopesStorageif there's no validScopesfor the current Thread available. This creates a lifecycle token that is ignored, since we don't know where to actually restore the previousContext.We tested this using
server.tomcat.threads.max=1inapplication.propertiesso there is only one thread handling requests. Before this change, the second request would fail to properly create transactions/spans and does not send anything to Sentry.More details:
OTel changed this behaviour to support things like Camel, where
Scope.close()might be called multiple times. See open-telemetry/opentelemetry-java#5055Risks:
This could mean when
closecalls go out of order, we end up with a different state than before. However this scenario probably means random result anyways.💚 How did you test it?
📝 Checklist
sendDefaultPIIis enabled.🔮 Next steps