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

[4.x] Guard against NPE during early invocation of Span.current() #8257

Merged
merged 2 commits into from
Jan 18, 2024

Conversation

tjquinno
Copy link
Member

Description

Resolves #8254

See description for details and related 3.x PR #8256 for explanation.

Signed-off-by: Tim Quinn <tim.quinn@oracle.com>
@tjquinno tjquinno self-assigned this Jan 17, 2024
@oracle-contributor-agreement oracle-contributor-agreement bot added the OCA Verified All contributors have signed the Oracle Contributor Agreement. label Jan 17, 2024
@tjquinno tjquinno changed the title Guard against NPE during early invocation of Span.current() [4.x] Guard against NPE during early invocation of Span.current() Jan 18, 2024
@tjquinno tjquinno added this to the 4.0.4 milestone Jan 18, 2024
logging that adds the current span to each message), then this method can be invoked before the static initializer has
completed and, therefore, before TRACER_PROVIDER is assigned.
*/
return (TRACER_PROVIDER == null) ? Optional.empty() : TRACER_PROVIDER.currentSpan();
Copy link
Member

Choose a reason for hiding this comment

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

I think that this is creating a bug. We should never reach this method within initalization.
If we do, then we should change the way we initialize, rather than checking if a constant field is null

Copy link
Member Author

Choose a reason for hiding this comment

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

As I mentioned in our DMs, we could add a static boolean that indicates if the static initializer is in progress or not. Then currentSpan could check that boolean and, if true, return Optional.empty() right away rather than trying to use TRACER_PROVIDER. That might look slightly less odd than checking a supposedly final field for null, but it amounts to the same thing: checking whether the static initializer is in-progress or not.

As we also discussed, I will also add code to throw exceptions from other methods that rely on TRACER_PROVIDER but are void or have non-Optional return types.

@tjquinno tjquinno requested a review from tomas-langer January 18, 2024 16:20
@tjquinno tjquinno merged commit b78bc01 into helidon-io:main Jan 18, 2024
12 checks passed
@tjquinno tjquinno deleted the 4.x-tracer-provider-fix branch January 18, 2024 17:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
OCA Verified All contributors have signed the Oracle Contributor Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

TracerProviderHelper#currentSpan() throws NPE in some situations
2 participants