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
ThreadLocalContextStorage
withSentryOtelThreadLocalStorage
which hands back a differentScope
implementation that on.close()
does not check whether the current activeContext
is the one it expects.💡 Motivation and Context
By default
ScopeImpl.close()
in OpenTelemetry checks whether the current activeContext
is the one from back whenContext
was forked. This means if during lifecycle of thisScope
anything does not clean up properly, we're stuck with an outdatedContext
that holds aSpan
reference. ThisSpan
has 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
Scope
was 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 theIScopesStorage
if there's no validScopes
for 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=1
inapplication.properties
so 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
close
calls 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
sendDefaultPII
is enabled.🔮 Next steps