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

otlpmetrichttp: Use go.opentelemetry.io/proto/slim/otlp #5222

Merged
merged 5 commits into from Apr 22, 2024

Conversation

pellared
Copy link
Member

@pellared pellared commented Apr 17, 2024

Fixes #2579

I tried to do it with as minimal changes as possible while also doing some cleanups (code removal). Here is how I done it:

  1. For simple files (without logic) generated via gotmpl , I added protoImportPrefix template variable to control the imports for the proto packages.
  2. For complex files (options and test collector) I inlined the source code, removed the unnecessary code for given signal, simplified to have only one option type, and updated to use go.opentelemetry.io/proto/slim/otlp for otlpmetrichttp. I did not want to go to far with the refactoring.
  3. In the rest of files in otlpmetrichttp package, I simply replaced go.opentelemetry.io/proto/otlp with go.opentelemetry.io/proto/slim/otlp.

Copy link

codecov bot commented Apr 17, 2024

Codecov Report

Attention: Patch coverage is 93.84615% with 4 lines in your changes are missing coverage. Please review.

Project coverage is 85.5%. Comparing base (f885333) to head (d6fd030).

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #5222     +/-   ##
=======================================
+ Coverage   84.5%   85.5%   +0.9%     
=======================================
  Files        258     258             
  Lines      17348   17030    -318     
=======================================
- Hits       14675   14570    -105     
+ Misses      2362    2152    -210     
+ Partials     311     308      -3     
Files Coverage Δ
...pmetric/otlpmetricgrpc/internal/otest/collector.go 95.7% <ø> (+73.0%) ⬆️
exporters/otlp/otlpmetric/otlpmetrichttp/client.go 82.4% <ø> (ø)
...porters/otlp/otlpmetric/otlpmetrichttp/exporter.go 87.5% <ø> (ø)
...tlpmetric/otlpmetrichttp/internal/oconf/options.go 98.9% <100.0%> (+6.1%) ⬆️
...otlpmetric/otlpmetrichttp/internal/otest/client.go 97.7% <ø> (ø)
...pmetric/otlpmetrichttp/internal/otest/collector.go 72.3% <ø> (+11.7%) ⬆️
...ric/otlpmetrichttp/internal/transform/attribute.go 100.0% <ø> (ø)
...pmetric/otlpmetrichttp/internal/transform/error.go 100.0% <ø> (ø)
...ic/otlpmetrichttp/internal/transform/metricdata.go 100.0% <ø> (ø)
...pmetric/otlpmetricgrpc/internal/oconf/envconfig.go 92.0% <83.3%> (-1.0%) ⬇️
... and 2 more

@pellared pellared added the area:metrics Part of OpenTelemetry Metrics label Apr 17, 2024
@pellared pellared added this to the v1.26.0 milestone Apr 17, 2024
@pellared pellared added pkg:exporter:otlp Related to the OTLP exporter package enhancement New feature or request labels Apr 17, 2024
Copy link
Contributor

@MrAlias MrAlias left a comment

Choose a reason for hiding this comment

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

🚀

@pellared pellared merged commit 6e92163 into open-telemetry:main Apr 22, 2024
26 checks passed
@pellared pellared deleted the otlpmetrichttp-use-slim branch April 22, 2024 11:07
XSAM added a commit to XSAM/opentelemetry-go that referenced this pull request Apr 22, 2024
pellared pushed a commit that referenced this pull request Apr 23, 2024
* Revert "otlpmetrichttp: Use go.opentelemetry.io/proto/slim/otlp (#5222)"

This reverts commit 6e92163.

* Revert "otlploghttp: Use go.opentelemetry.io/proto/slim/otlp (#5216)"

This reverts commit fe3de70.

* Remove slim dep

* Fix CI
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:metrics Part of OpenTelemetry Metrics enhancement New feature or request pkg:exporter:otlp Related to the OTLP exporter package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

otlp*http modules import grpc
3 participants