-
Notifications
You must be signed in to change notification settings - Fork 182
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
[chore] Mention JavaAgent v2.0.0 default behaviour #1533
Conversation
@open-telemetry/java-instrumentation-maintainers FYI |
@devurandom a related discussion has been taking place in #534, can you review that issue? thanks |
Co-authored-by: Armin Ruech <[email protected]>
Thanks for the hint. I was not aware of that issue. My PR is (to a limited degree) addressing his point in #534 (comment): I point out that, if you're using Java agent v2.0.0 or later, the migration does not apply to you. I do not feel qualified to address the wider-scope suggestions he points out, like #534 (comment) to create a table for all supported languages, that fully address the issue and could close it. |
if you are upgrading from Java agent 1.x to 2.x, I think it does apply to you |
@@ -29,6 +29,9 @@ updated to the stable HTTP semantic conventions, they: | |||
- May drop the environment variable in their next major version and emit only | |||
the stable HTTP and networking conventions. | |||
|
|||
The OpenTelemetry Java Agent [v2.0.0](https://github.com/open-telemetry/opentelemetry-java-instrumentation/releases/tag/v2.0.0) and later already defaults to the stable |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for the clarification!
should this text belong in the javaagent docs? I believe this doc is intended for instrumentation authors and telemetry backends rather than the end-users of specific instrumentation.
I don't mind having it here either, but then I think we need a section for instrumentation-specific behavior that would initially contain Java instrumentation agent.
Curious what @trask thinks.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
should this text belong in the javaagent docs?
I think that would probably be a better place, until we solve the more general questions in #534
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Similar to @TylerHelmuth in #534, I would probably not have stumbled upon this info in the Java agent docs, since I was (via web search and general reading about metrics and their attributes) coming from the semconv docs.
@@ -29,6 +29,9 @@ updated to the stable HTTP semantic conventions, they: | |||
- May drop the environment variable in their next major version and emit only | |||
the stable HTTP and networking conventions. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I wonder if this whole section above is adding to the confusion. It was a good part of the blog post, but I wonder if the document here should just focus on the summary of changes?
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
Closed as inactive. Feel free to reopen if this PR is still being worked on. |
Changes
HTTP semantic convention stability migration docs 1 do not mention that the JavaAgent v2.0.0 already defaults to the new, stable behaviour 2, which could lead to people believing they have to set
OTEL_SEMCONV_STABILITY_OPT_IN=http
when they don't.This does not change an actual convention, thus I believe it does not need a changelog entry.
Note: if the PR is touching an area that is not listed in the existing areas, or the area does not have sufficient domain experts coverage, the PR might be tagged as experts needed and move slowly until experts are identified.
Merge requirement checklist
[chore]
Footnotes
https://opentelemetry.io/docs/specs/semconv/non-normative/http-migration/ ↩
https://github.com/open-telemetry/opentelemetry-java-instrumentation/releases/tag/v2.0.0 ↩