otelgin: update default span name to align with semantic conventions

[flc1125]

fixes #7088

SpanNameFormatter has undergone a breaking change, which can be considered when reviewing whether the project necessitates it.

From my previous recollection, I believe this part was adjusted according to this logic. Moreover, otelgin is still in an unstable version, meaning it permits some breaking changes.

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 81.0%. Comparing base (da1ea48) to head (7e89019).
Report is 1 commits behind head on main.

Additional details and impacted files

@@          Coverage Diff          @@
##            main   #6381   +/-   ##
=====================================
  Coverage   80.9%   81.0%           
=====================================
  Files        204     204           
  Lines      18065   18079   +14     
=====================================
+ Hits       14630   14650   +20     
+ Misses      3007    3003    -4     
+ Partials     428     426    -2     

Files with missing lines	Coverage Δ
...ntation/github.com/gin-gonic/gin/otelgin/config.go	100.0% <100.0%> (ø)
...umentation/github.com/gin-gonic/gin/otelgin/gin.go	92.8% <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.

[dmathieu]

Based on the ongoing discussions with the need or not for SpanNameFormatter, could you update this PR to only include the default span name?

[flc1125]

Based on the ongoing discussions with the need or not for SpanNameFormatter, could you update this PR to only include the default span name?

OK, I will add another draft version of the PR (which may not have passed unit testing and CI).

[flc1125]

@dmathieu A draft version: #6396

[flc1125]

Temporarily closed for now; will be reprocessed once there are further developments.

[akats7]

@flc1125 I missed this issue the first time around, but what was the blocker here?

[flc1125]

@flc1125 I missed this issue the first time around, but what was the blocker here?

It's been quite some time, and I recall it was because there were still some content or semantic agreements that needed confirmation. Moreover, discussions were still ongoing. So, this matter was temporarily put on hold.

Due to the prolonged lack of progress, I have decided to close it for now. Until there are new developments, I will consider reopening it.

If there are any new changes, I will continue to address this issue.

[flc1125]

#7088 These two issues may have some correlation.

[akats7]

Yep this seems to resolve that issue. I don’t see any blockers regarding only updating the default span names. Unless the goal was to have the logic sit in the semconv package and avoid duplication.

[flc1125]

Yep this seems to resolve that issue. I don’t see any blockers regarding updating only updating the default span names. Unless the goal was to have the logic sit in the semconv package and avoid duplication.

OK, if there are no problems hindered for now, I will continue to track this issue.

[akats7]

Yep this seems to resolve that issue. I don’t see any blockers regarding updating only updating the default span names. Unless the goal was to have the logic sit in the semconv package and avoid duplication.

OK, if there are no problems hindered for now, I will continue to track this issue.

Do you mind either updating the PR title or opening a new PR to make it clear that we’re not deprecating SpanNameFormatter

[flc1125]

Do you mind either updating the PR title or opening a new PR to make it clear that we’re not deprecating SpanNameFormatter

It has been changed. If there is any mistake in the description, can you help me adjust it. (PS: My English is not very good)

[flc1125]

Ready for review. @akats7 @dmathieu

[dmathieu]

FullPath() is a high cardinality value. We should make this the HTTP pattern/route, not the path.

If the path is /blog/1234, and the route is defined as /blog/{id} then, the latter should be used.

[dmathieu]

Left minor comments. It overall looks good.

[dmathieu]

Unless someone objects, I will merge this tomorrow.