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 Version func to otel/sdk #3949

Merged
merged 5 commits into from
Mar 30, 2023
Merged

Add Version func to otel/sdk #3949

merged 5 commits into from
Mar 30, 2023

Conversation

MrAlias
Copy link
Contributor

@MrAlias MrAlias commented Mar 29, 2023

Towards #3946

Currently the SDK resources report the SDK version as the API version. If a project uses a different version of the SDK than the API this information will be incorrect. Add a version function to the SDK and use it.

@MrAlias MrAlias added the pkg:SDK Related to an SDK package label Mar 29, 2023
@codecov
Copy link

codecov bot commented Mar 29, 2023

Codecov Report

Merging #3949 (b3a6da5) into main (63a0f51) will increase coverage by 0.0%.
The diff coverage is 100.0%.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #3949   +/-   ##
=====================================
  Coverage   81.7%   81.7%           
=====================================
  Files        170     171    +1     
  Lines      12949   12950    +1     
=====================================
+ Hits       10588   10591    +3     
+ Misses      2138    2136    -2     
  Partials     223     223           
Impacted Files Coverage Δ
sdk/internal/internal.go 100.0% <ø> (ø)
...s/otlp/otlpmetric/internal/transform/metricdata.go 100.0% <100.0%> (ø)
exporters/prometheus/exporter.go 83.7% <100.0%> (ø)
sdk/metric/internal/histogram.go 100.0% <100.0%> (ø)
sdk/metric/metricdata/data.go 57.1% <100.0%> (ø)
sdk/metric/metricdata/metricdatatest/assertion.go 87.3% <100.0%> (+0.2%) ⬆️
...dk/metric/metricdata/metricdatatest/comparisons.go 88.2% <100.0%> (ø)
sdk/resource/builtin.go 90.3% <100.0%> (ø)
sdk/version.go 100.0% <100.0%> (ø)

... and 1 file with indirect coverage changes

Copy link
Member

@Aneurysm9 Aneurysm9 left a comment

Choose a reason for hiding this comment

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

Should there be a distinct version.go for each module? I believe that's what we've done in contrib and rely on multimod to manage them.

@MrAlias
Copy link
Contributor Author

MrAlias commented Mar 29, 2023

Should there be a distinct version.go for each module? I believe that's what we've done in contrib and rely on multimod to manage them.

SGTM. The only other place the otel.Version is used, as far as I can tell, is in the OTLP exporters:

return "OTel OTLP Exporter Go/" + otel.Version()

I could see that being split up and being put in the appropriate modules and each of the modules getting a version.go to track their versions.

I can also see going the same route as contrib with version.go for all modules. Just not sure that is necessary. 🤷

Sounds like good tasks for follow up PRs.

CHANGELOG.md Outdated Show resolved Hide resolved
@pellared
Copy link
Member

Because of #3949 (comment)

I changed the "Fixed" to "Towards" in the PR description.

I also added a comment to the issue about usage of otel.Version() in OTLP exporters.

Co-authored-by: Robert Pająk <pellared@hotmail.com>
@MrAlias MrAlias merged commit 271df1d into open-telemetry:main Mar 30, 2023
@MrAlias MrAlias deleted the sdk-ver branch March 30, 2023 19:49
@MrAlias MrAlias mentioned this pull request Apr 27, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pkg:SDK Related to an SDK package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants