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

Update opentelemetry/proto spec #294

Closed

Conversation

trevor-dialpad
Copy link
Contributor

A couple things done here:

  1. Updated the proto based on the latest spec
  2. Updated the metrics adapter to support the new spec

This should solve #293

Ran protoc --swift_out=. opentelemetry/proto/*/v1/*.proto and made all of the definitions public
@trevor-dialpad
Copy link
Contributor Author

This is a draft because the metrics I'm posting from my client aren't being successfully picked up my the collector - I'm trying to figure out why that is so I can address it but I haven't found much to go off of so far. If anyone has any clue as to why these metrics won't be picked up by the opentelemetry-collector with OTLP/GRPC I'd appreciate any ideas.

@codecov
Copy link

codecov bot commented Mar 15, 2022

Codecov Report

Merging #294 (ac8d3d9) into main (1c648c1) will decrease coverage by 0.89%.
The diff coverage is 30.56%.

❗ Current head ac8d3d9 differs from pull request most recent head 24d718d. Consider uploading reports for the commit 24d718d to get more accurate results

@@            Coverage Diff             @@
##             main     #294      +/-   ##
==========================================
- Coverage   75.35%   74.45%   -0.90%     
==========================================
  Files         361      361              
  Lines       22569    22789     +220     
==========================================
- Hits        17006    16968      -38     
- Misses       5563     5821     +258     
Impacted Files Coverage Δ
...xporters/OpenTelemetryProtocol/proto/logs.pb.swift 0.00% <0.00%> (ø)
...rters/OpenTelemetryProtocol/proto/metrics.pb.swift 22.78% <ø> (+3.08%) ⬆️
...ters/OpenTelemetryProtocol/proto/resource.pb.swift 90.00% <ø> (ø)
.../OpenTelemetryProtocol/proto/trace_config.pb.swift 0.00% <0.00%> (ø)
.../OpenTelemetryProtocol/metric/MetricsAdapter.swift 32.17% <12.50%> (-0.82%) ⬇️
...orters/OpenTelemetryProtocol/proto/common.pb.swift 38.56% <36.36%> (-11.82%) ⬇️
...porters/OpenTelemetryProtocol/proto/trace.pb.swift 57.99% <53.80%> (-7.95%) ⬇️
...penTelemetryProtocol/OtlpMetricExporterTests.swift 86.60% <100.00%> (ø)
...ces/Exporters/Persistence/Storage/FileReader.swift 74.19% <0.00%> (-9.68%) ⬇️
... and 8 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 1c648c1...24d718d. Read the comment docs.

@trevor-dialpad
Copy link
Contributor Author

A bit more details I have since posting this draft:

  1. This is the commit that caused metrics to stop working with the Otel collector as of v0.44.0. This removed the backwards compatibility to support metrics being sent based on the older spec (using "labels", "int_histogram", etc.)

  2. This exactly what I did to generate the new spec. Then I went and manually updated some of the generated proto swift functions and variables to be public in order to compile.

protoc --swift_out=. opentelemetry/proto/*/*/v1/*.proto
protoc --grpc-swift_out=. opentelemetry/proto/*/*/v1/*.proto

@trevor-dialpad
Copy link
Contributor Author

Fixed with: #300

@trevor-dialpad trevor-dialpad deleted the update_otlp_proto branch May 13, 2022 20:42
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.

1 participant