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 to OTLP proto 0.12.0 #4724

Merged
merged 1 commit into from
Jan 26, 2022

Conversation

tigrannajaryan
Copy link
Member

@tigrannajaryan tigrannajaryan commented Jan 21, 2022

Resolves #4717

  • Removed all deprecated metric messages and corresponding conversion code and tests.
  • Removed deprecated_code from span status and corresponding conversions and tests.
  • Renamed logs field to log_records in InstrumentationLibraryLogs message.

@codecov
Copy link

codecov bot commented Jan 21, 2022

Codecov Report

Merging #4724 (165ecee) into main (105f75b) will decrease coverage by 0.05%.
The diff coverage is 81.48%.

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #4724      +/-   ##
==========================================
- Coverage   90.73%   90.68%   -0.06%     
==========================================
  Files         179      179              
  Lines       10696    10541     -155     
==========================================
- Hits         9705     9559     -146     
+ Misses        770      763       -7     
+ Partials      221      219       -2     
Impacted Files Coverage Δ
model/internal/otlp_wrapper.go 100.00% <ø> (+7.97%) ⬆️
model/pdata/traces.go 91.66% <ø> (-2.46%) ⬇️
model/pdata/logs.go 82.14% <20.00%> (-13.70%) ⬇️
model/pdata/generated_log.go 96.65% <93.33%> (ø)
internal/otlptext/logs.go 100.00% <100.00%> (ø)
model/pdata/generated_trace.go 96.89% <100.00%> (+0.01%) ⬆️
processor/batchprocessor/splitlogs.go 100.00% <100.00%> (ø)

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 105f75b...165ecee. Read the comment docs.

@tigrannajaryan tigrannajaryan force-pushed the update-proto branch 3 times, most recently from 3eb0aad to d9107d2 Compare January 21, 2022 16:29
@tigrannajaryan tigrannajaryan marked this pull request as ready for review January 21, 2022 16:29
@tigrannajaryan tigrannajaryan requested review from a team and Aneurysm9 January 21, 2022 16:29
@tigrannajaryan
Copy link
Member Author

@open-telemetry/collector-approvers please review the deletions of metrics code carefully. I want to make sure I didn't break anything and didn't delete any code that was supposed to stay.

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.

It gets a bit hard to follow the changes in the generated *.pb.go files, but it looked like the metrics changes were what I'd expect. The deprecated types were removed and from there it looked like noise from shuffling things around.

Copy link
Contributor

@codeboten codeboten left a comment

Choose a reason for hiding this comment

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

I think the changes all look good, I'd like to merge this after this next release though

model/pdata/logs.go Show resolved Hide resolved
model/pdata/generated_log.go Outdated Show resolved Hide resolved
@tigrannajaryan
Copy link
Member Author

I think the changes all look good, I'd like to merge this after this next release though

Yes, let's hold this until the release is done.

@tigrannajaryan tigrannajaryan marked this pull request as draft January 24, 2022 13:50
@tigrannajaryan tigrannajaryan marked this pull request as ready for review January 26, 2022 17:58
- Removed all deprecated metric messages and corresponding conversion code and tests.
- Removed deprecated_code from span status and corresponding conversions and tests.
- Renamed logs field to log_records in InstrumentationLibraryLogs message.
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.

Upgrade to proto v0.12.0 to remove lots of deprecated code
4 participants