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

Add semconvutil and use it instead of httpconv and netconv #3817

Merged
merged 26 commits into from
May 25, 2023

Conversation

pellared
Copy link
Member

@pellared pellared commented May 16, 2023

Why

Towards open-telemetry/opentelemetry-go#4081 (point 1)

Closes open-telemetry/opentelemetry-go#3744

It will make updating semconv in https://github.com/open-telemetry/opentelemetry-go a lot easier. This repo will only contain the "currently used" semconv code. Updating to newer will also be easier as we will change existing code instead of generating new code.

What

  1. Add semconvutil as a internal shared package which is not to be published (Go templates).
  2. Copy semconvutil package (via go generate + gotmpl) and use it instead of httpconv and netconv packages which are planned to be deprecated.

The "copy package" via go generate approach is used in order to not introduce dependencies between modules. More: open-telemetry/opentelemetry-go#3846

@codecov
Copy link

codecov bot commented May 16, 2023

Codecov Report

Merging #3817 (5b8181c) into main (849072e) will increase coverage by 9.2%.
The diff coverage is 100.0%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #3817     +/-   ##
=======================================
+ Coverage   70.1%   79.3%   +9.2%     
=======================================
  Files        149     163     +14     
  Lines       7133   10325   +3192     
=======================================
+ Hits        5004    8196   +3192     
  Misses      1998    1998             
  Partials     131     131             
Impacted Files Coverage Δ
...stful/otelrestful/internal/semconvutil/httpconv.go 100.0% <100.0%> (ø)
...estful/otelrestful/internal/semconvutil/netconv.go 100.0% <100.0%> (ø)
...hub.com/emicklei/go-restful/otelrestful/restful.go 100.0% <100.0%> (ø)
...ation/github.com/gin-gonic/gin/otelgin/gintrace.go 83.5% <100.0%> (ø)
...gonic/gin/otelgin/internal/semconvutil/httpconv.go 100.0% <100.0%> (ø)
...-gonic/gin/otelgin/internal/semconvutil/netconv.go 100.0% <100.0%> (ø)
...rilla/mux/otelmux/internal/semconvutil/httpconv.go 100.0% <100.0%> (ø)
...orilla/mux/otelmux/internal/semconvutil/netconv.go 100.0% <100.0%> (ø)
...trumentation/github.com/gorilla/mux/otelmux/mux.go 88.4% <100.0%> (ø)
...entation/github.com/labstack/echo/otelecho/echo.go 100.0% <100.0%> (ø)
... and 12 more

@pellared pellared added the Skip Changelog Allow PR to succeed without requiring an addition to the CHANGELOG label May 16, 2023
@pellared pellared changed the title Add semconvutil based on semconv/v1.18.0 Add semconvutil and use it instead of httpconv and netconv May 16, 2023
Makefile Outdated Show resolved Hide resolved
@pellared pellared marked this pull request as ready for review May 16, 2023 14:30
@pellared pellared requested a review from a team May 16, 2023 14:30
@pellared pellared marked this pull request as draft May 17, 2023 09:24
@pellared pellared marked this pull request as ready for review May 17, 2023 09:36
@pellared pellared marked this pull request as draft May 18, 2023 18:01
@pellared pellared marked this pull request as ready for review May 18, 2023 18:58
@pellared pellared requested a review from MrAlias May 18, 2023 18:58
internal/shared/semconvutil/doc.go.tmpl Outdated Show resolved Hide resolved
versions.yaml Outdated Show resolved Hide resolved
@MrAlias MrAlias merged commit e9956ff into open-telemetry:main May 25, 2023
@pellared pellared deleted the move-httpnetconv branch May 25, 2023 16:51
@MrAlias MrAlias added this to the v0.43.0 milestone Aug 28, 2023
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
None yet
Development

Successfully merging this pull request may close these issues.

Split httpconv and netconv by signal
4 participants