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

feat(instrumentation/http/otelhttp): move client metrics creation into internal semconv package #6002

Merged
merged 15 commits into from
Oct 10, 2024

Conversation

VinozzZ
Copy link
Contributor

@VinozzZ VinozzZ commented Aug 8, 2024

In preparation for implementing OTEL_SEMCONV_STABILITY_OPT_IN for HTTP client metrics, this is the first PR to emit only old client metrics. This PR will enable us to add the ability for otelhttp to emit both old and new client metrics.

Address issue #5973

@VinozzZ VinozzZ requested a review from dmathieu as a code owner August 8, 2024 23:36
@VinozzZ VinozzZ requested a review from a team August 8, 2024 23:36
Copy link

codecov bot commented Aug 8, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 67.2%. Comparing base (900fc4b) to head (cc38490).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #6002   +/-   ##
=====================================
  Coverage   67.1%   67.2%           
=====================================
  Files        194     194           
  Lines      12709   12766   +57     
=====================================
+ Hits        8533    8584   +51     
- Misses      3901    3904    +3     
- Partials     275     278    +3     
Files with missing lines Coverage Δ
instrumentation/net/http/otelhttp/common.go 100.0% <ø> (ø)
instrumentation/net/http/otelhttp/handler.go 91.9% <100.0%> (+1.8%) ⬆️
...entation/net/http/otelhttp/internal/semconv/env.go 92.0% <100.0%> (+3.3%) ⬆️
...tion/net/http/otelhttp/internal/semconv/v1.20.0.go 94.4% <100.0%> (+3.2%) ⬆️
instrumentation/net/http/otelhttp/transport.go 94.3% <100.0%> (-0.9%) ⬇️

... and 2 files with indirect coverage changes

@MrAlias
Copy link
Contributor

MrAlias commented Aug 9, 2024

It looks like the lint CI is failing. Please run make precommit, it should fix the issue.

@dashpole dashpole added the Skip Changelog Allow PR to succeed without requiring an addition to the CHANGELOG label Aug 15, 2024
@dashpole
Copy link
Contributor

This is now ready for review.

@VinozzZ VinozzZ requested a review from dashpole August 20, 2024 14:38
@XSAM
Copy link
Member

XSAM commented Aug 29, 2024

@open-telemetry/go-approvers PTAL

@XSAM
Copy link
Member

XSAM commented Aug 30, 2024

Overall, it looks good to me. I only have this question to address. #6002 (comment)

@VinozzZ VinozzZ requested a review from a team as a code owner October 2, 2024 21:23
@pellared pellared requested review from MrAlias and XSAM October 3, 2024 16:53
@dmathieu dmathieu merged commit 87d0229 into open-telemetry:main Oct 10, 2024
23 of 24 checks passed
@MrAlias MrAlias added this to the v1.31.0 milestone Oct 10, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog Allow PR to succeed without requiring an addition to the CHANGELOG
Projects
Development

Successfully merging this pull request may close these issues.

5 participants