-
Notifications
You must be signed in to change notification settings - Fork 782
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
[Instrumentation.GrpcNetClient] Support only stable HTTP semantic convention #5259
[Instrumentation.GrpcNetClient] Support only stable HTTP semantic convention #5259
Conversation
388310c
to
01d168b
Compare
01d168b
to
9ca3aaf
Compare
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #5259 +/- ##
==========================================
- Coverage 83.38% 83.01% -0.37%
==========================================
Files 297 272 -25
Lines 12531 11965 -566
==========================================
- Hits 10449 9933 -516
+ Misses 2082 2032 -50
Flags with carried forward coverage won't be shown. Click here to find out more.
|
Co-authored-by: Reiley Yang <reyang@microsoft.com>
@reyang, @cijothomas, all comments for changeling handled by Reiley suggestion. |
@@ -9,6 +9,11 @@ | |||
[issue](https://github.com/open-telemetry/opentelemetry-dotnet/issues/5092) | |||
for details and workaround. | |||
([#5077](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5077)) | |||
* Removed `OTEL_SEMCONV_STABILITY_OPT_IN` environment variable support. The |
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.
One question .. The grpc conventions are still not stable and this instrumentaion is still emitting them. Should we explicitly call it out, to avoid giving the feel that the entire conventions emitted by this is 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.
I think it's fine, folks who want to spend time reading changelog should already know semver2.
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.
Approving. The changelog was probably clear enough before, but just adding a suggestion making it a bit more precise.
Co-authored-by: Alan West <3676547+alanwest@users.noreply.github.com>
Fixes # N/A
Design discussion issue # N/A
Changes
Drop support for old semantic convention in GrpcNetClient instrumenation
Merge requirement checklist
CHANGELOG.md
files updated for non-trivial changes[ ] Changes in public API reviewed (if applicable)