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

Fix incorrect "all" and "resource" definition for Schema File #3777

Merged

Conversation

tigrannajaryan
Copy link
Member

The "all" and "resource" sections had incorrect definitions of "attribute_rename" transform. It was missing the subkey "attribute_map".

This is a bug fix and makes the implementation compliant with the spec: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/schemas/file_format_v1.1.0.md#resources-section

Related issue: open-telemetry/opentelemetry-specification#3245

@tigrannajaryan
Copy link
Member Author

I think this does not require a change log since it is a bug fix, but let me know if you think otherwise.

@MrAlias MrAlias added this to the Schema v0.0.4 milestone Feb 24, 2023
@MrAlias
Copy link
Contributor

MrAlias commented Feb 24, 2023

I think this does not require a change log since it is a bug fix, but let me know if you think otherwise.

Can you add an entry in the ### Fixed section?

@tigrannajaryan
Copy link
Member Author

I think this does not require a change log since it is a bug fix, but let me know if you think otherwise.

Can you add an entry in the ### Fixed section?

Done.

@codecov
Copy link

codecov bot commented Feb 24, 2023

Codecov Report

Merging #3777 (1ebbcc8) into main (17e5d0f) will decrease coverage by 0.1%.
The diff coverage is 100.0%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #3777     +/-   ##
=======================================
- Coverage   81.7%   81.7%   -0.1%     
=======================================
  Files        167     167             
  Lines      12462   12453      -9     
=======================================
- Hits       10184   10175      -9     
  Misses      2063    2063             
  Partials     215     215             
Impacted Files Coverage Δ
exporters/otlp/otlpmetric/internal/otest/client.go 97.1% <ø> (ø)
exporters/prometheus/exporter.go 83.5% <ø> (ø)
sdk/metric/metricdata/data.go 57.1% <ø> (ø)
sdk/metric/pipeline.go 90.6% <ø> (ø)
bridge/opencensus/internal/ocmetric/metric.go 93.3% <100.0%> (-0.5%) ⬇️
metric/instrument/asyncfloat64.go 100.0% <100.0%> (ø)
metric/instrument/asyncint64.go 100.0% <100.0%> (ø)
metric/instrument/instrument.go 100.0% <100.0%> (ø)
metric/instrument/syncfloat64.go 100.0% <100.0%> (ø)
metric/instrument/syncint64.go 100.0% <100.0%> (ø)
... and 4 more

@Aneurysm9 Aneurysm9 removed the Skip Changelog PRs that do not require a CHANGELOG.md entry label Feb 24, 2023
@tigrannajaryan
Copy link
Member Author

OK to merge this?

@MrAlias
Copy link
Contributor

MrAlias commented Feb 24, 2023

OK to merge this?

I think this meets the repo standard to wait a full 24 hours.

Is there a reason to expedite? Are we trying to push a build tools fix today?

The "all" and "resource" sections had incorrect definitions of "attribute_rename"
transform. It was missing the subkey "attribute_map".

This is a bug fix and makes the implementation compliant with the spec:
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/schemas/file_format_v1.1.0.md#resources-section

Related issue: open-telemetry/opentelemetry-specification#3245
@tigrannajaryan
Copy link
Member Author

Rebased and conflict resolved. I think this is ready to be merged.

@MrAlias MrAlias merged commit 071d317 into open-telemetry:main Feb 27, 2023
@tigrannajaryan tigrannajaryan deleted the feature/tigran/fixschemafile branch February 27, 2023 17:02
tigrannajaryan added a commit to tigrannajaryan/build-tools that referenced this pull request Feb 27, 2023
Pinned to specific commit 071d3173944553bed7d05057ecccb177d307978b of
https://github.com/open-telemetry/opentelemetry-go to update the schema check tool
to include the bug fix open-telemetry/opentelemetry-go#3777
(Fix incorrect "all" and "resource" definition for Schema File).
tigrannajaryan added a commit to open-telemetry/build-tools that referenced this pull request Feb 27, 2023
Pinned to specific commit 071d3173944553bed7d05057ecccb177d307978b of
https://github.com/open-telemetry/opentelemetry-go to update the schema check tool
to include the bug fix open-telemetry/opentelemetry-go#3777
(Fix incorrect "all" and "resource" definition for Schema File).
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.

None yet

4 participants