[draft] Migrate to semconv for otelmux #6638

[TanishqPorwar]

tried using otelhttp.Handler in otelmux to address #6638

[TanishqPorwar]

@dmathieu would it make sense to use otelhttp since it already adheres to semconv? I'll tidy up this PR if this approach aligns with the right direction.

[codecov[bot]]

Codecov Report

Attention: Patch coverage is 89.13043% with 5 lines in your changes missing coverage. Please review.

Project coverage is 68.7%. Comparing base (9a53a30) to head (92636cd). Report is 9 commits behind head on main.

Files with missing lines	Patch %	Lines
...mentation/github.com/gorilla/mux/otelmux/config.go	0.0%	5 Missing :warning:

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #6653     +/-   ##
=======================================
- Coverage   68.7%   68.7%   -0.1%     
=======================================
  Files        200     200             
  Lines      16872   16864      -8     
=======================================
- Hits       11601   11590     -11     
- Misses      4927    4932      +5     
+ Partials     344     342      -2     

Files with missing lines	Coverage Δ
...trumentation/github.com/gorilla/mux/otelmux/mux.go	93.5% <100.0%> (+4.3%)	:arrow_up:
...mentation/github.com/gorilla/mux/otelmux/config.go	84.8% <0.0%> (-15.2%)	:arrow_down:

... and 5 files with indirect coverage changes

[dmathieu]

Yes, if we can get HTTP instrumentations to use otelhttp rather than reimplement things themselves, that's definitely better.

[akats7]

Yes, if we can get HTTP instrumentations to use otelhttp rather than reimplement things themselves, that's definitely better.

@dmathieu wasn't there talk of refactoring otelhttp, not sure to what extent the intended changes would be breaking

[dmathieu]

Yes. But if/when that happens, we will make the appropriate changes to avoid breaking other instrumentations. Reusing otelhttp also makes it much easier to keep. things working the same way.

We won't be able to make breaking changes in otelhttp that easily though, because many folks rely on it as a stable dependency despite it not being one.

[dmathieu]

@TanishqPorwar did you see you're working on this alongside @martinyonatann (https://github.com/open-telemetry/opentelemetry-go-contrib/pull/6648)?

[TanishqPorwar]

@dmathieu thanks for pointing this out. My goal with this PR was to propose an alternate solution by reusing otelhttp.

This approach minimizes redundancy, adheres to semconv, and simplifies future maintenance. That said, I’m open to discussing how we can align our efforts. @martinyonatann, let me know your thoughts, and we can figure out the best way forward.

[martinyonatann]

from my personal perspective, to avoid unintended changes to otelhtttp, i prefer to reimplement the semconv and request packages because that aligns with the purpose of creating the package template.

Reusing otelhttp is definitely is nice-to-have, and i agree that it's a goal worth pursuing. However, we'll need to carefully pan how we can reuse it across frameworks that are built on net/http, such as mux, echo, gin, etc.

Each framework has its nuances, so ensuring a consistent implementation while accommodating these differences is key. Let's discuss how we can create a roadmap or strategy for this, so we can make the transition as smooth and efficient as possible.

[akats7]

I think for this current iteration and to first complete the semconv migration let’s go with the reimplementation, especially since a good amount of work has been completed. We can continue to evaluate the merits of reusing otelhttp but I agree there are additional considerations we need to make if we’re going to start using otelhttp as a base.

[dmathieu]

Let's keep the discussion in a single place, #6638.