Skip to content

otelgin: add http.route metric attribute#7275

Merged
dmathieu merged 3 commits intoopen-telemetry:mainfrom
chenlujjj:gin-metric-route-label
Apr 29, 2025
Merged

otelgin: add http.route metric attribute#7275
dmathieu merged 3 commits intoopen-telemetry:mainfrom
chenlujjj:gin-metric-route-label

Conversation

@chenlujjj
Copy link
Copy Markdown
Contributor

@chenlujjj chenlujjj commented Apr 27, 2025

According to the Semantic conventions for HTTP metrics | OpenTelemetry, http.route is a stable label for http server metrics, I suggest to set it as a default label in the gin middleware.

@chenlujjj chenlujjj requested a review from a team as a code owner April 27, 2025 06:58
@github-actions github-actions Bot requested review from akats7 and flc1125 April 27, 2025 06:58
@flc1125
Copy link
Copy Markdown
Member

flc1125 commented Apr 27, 2025

Can you add some unit tests?

@flc1125
Copy link
Copy Markdown
Member

flc1125 commented Apr 27, 2025

@chenlujjj
Copy link
Copy Markdown
Contributor Author

sure, will do

@chenlujjj chenlujjj force-pushed the gin-metric-route-label branch 2 times, most recently from 5c09f8d to 4a426ee Compare April 27, 2025 14:47
@chenlujjj
Copy link
Copy Markdown
Contributor Author

updated unit test and changelog

Comment thread CHANGELOG.md Outdated
@chenlujjj chenlujjj force-pushed the gin-metric-route-label branch from 4a426ee to 758ef4c Compare April 27, 2025 14:52
Copy link
Copy Markdown
Member

@flc1125 flc1125 left a comment

Choose a reason for hiding this comment

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

cc @open-telemetry/go-approvers @akats7

Comment thread instrumentation/github.com/gin-gonic/gin/otelgin/gin.go Outdated
@chenlujjj chenlujjj force-pushed the gin-metric-route-label branch from 758ef4c to 8374c20 Compare April 27, 2025 15:53
@codecov
Copy link
Copy Markdown

codecov Bot commented Apr 28, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.1%. Comparing base (5f609fb) to head (af3f3e7).
Report is 1 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #7275     +/-   ##
=======================================
- Coverage   81.1%   81.1%   -0.1%     
=======================================
  Files        204     204             
  Lines      18148   18151      +3     
=======================================
  Hits       14728   14728             
- Misses      3001    3003      +2     
- Partials     419     420      +1     
Files with missing lines Coverage Δ
...umentation/github.com/gin-gonic/gin/otelgin/gin.go 93.0% <100.0%> (+0.1%) ⬆️

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@chenlujjj chenlujjj force-pushed the gin-metric-route-label branch from 8374c20 to 8736109 Compare April 28, 2025 15:29
Comment thread CHANGELOG.md Outdated
@chenlujjj chenlujjj force-pushed the gin-metric-route-label branch from 8736109 to 793a005 Compare April 29, 2025 08:07
@chenlujjj
Copy link
Copy Markdown
Contributor Author

Hi, @dmathieu, thanks for your comments, I've updated code and resolved them.

@dmathieu
Copy link
Copy Markdown
Member

Note: you should avoid force-pushing your branch. It makes reviews much easier.

@chenlujjj
Copy link
Copy Markdown
Contributor Author

@dmathieu Got it. I thought I should alway only keep one commit for the PR.

@dmathieu
Copy link
Copy Markdown
Member

No. GitHub will squash once we merge the PR.

@chenlujjj
Copy link
Copy Markdown
Contributor Author

That's cool. I'll keep in mind :-)

Comment thread CHANGELOG.md Outdated
Co-authored-by: Robert Pająk <pellared@hotmail.com>
@pellared pellared changed the title feat: set http.route metric label in gin middleware otelgin: add http.route metric attribute Apr 29, 2025
@dmathieu dmathieu merged commit b658ecf into open-telemetry:main Apr 29, 2025
27 checks passed
@chenlujjj chenlujjj deleted the gin-metric-route-label branch April 29, 2025 14:58
@MrAlias MrAlias added this to the v1.36.0 milestone May 22, 2025
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.

6 participants