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

Log a warning whenever GlobalOpenTelemetry.set() is called #5264

Conversation

mateuszrzeszutek
Copy link
Member

I decided to implement this while I still remember about it 😅

CC @jack-berg

@mateuszrzeszutek mateuszrzeszutek requested a review from a team as a code owner January 28, 2022 11:17
@Advice.OnMethodEnter(suppress = Throwable.class)
public static void onEnter() {
LoggerFactory.getLogger(application.io.opentelemetry.api.GlobalOpenTelemetry.class)
.warn(
Copy link
Member

Choose a reason for hiding this comment

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

maybe info? (which is the javaagent's default log level, so will still get logged)

Copy link
Member Author

Choose a reason for hiding this comment

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

I think that warn makes slightly more sense in this case (since the log warns you about incorrect API usage), but I don't really have a strong opinion on this. I'll change that if we decide so.

Copy link
Member

Choose a reason for hiding this comment

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

my only concern is about users adding the javaagent later to an existing app that was previously manually instrumented, and then they're stuck with this warning message which is not really actionable

let's keep at warn and we can always revisit later

Copy link
Contributor

Choose a reason for hiding this comment

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

my only concern is about users adding the javaagent later to an existing app that was previously manually instrumented, and then they're stuck with this warning message which is not really actionable

let's keep at warn and we can always revisit later

Why not actionable? They can remove their manual usages of GlobalOpenTelemetry.set?

Copy link
Member

Choose a reason for hiding this comment

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

sorry, I should have been more specific, I was thinking about ops users

@trask trask merged commit cf6f9b7 into open-telemetry:main Jan 31, 2022
@mateuszrzeszutek mateuszrzeszutek deleted the log-error-on-GlobalOpenTelemetry-set branch February 1, 2022 06:26
RashmiRam pushed a commit to RashmiRam/opentelemetry-auto-instr-java that referenced this pull request May 23, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants