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 transition from instrumentationlibrary -> scope #5085

Merged
merged 6 commits into from
Mar 29, 2022

Conversation

codeboten
Copy link
Contributor

Follow up to #5076.

This is the last piece of upgrading to v0.15.0. This PR adds the tests and code to ensure the transition plan is implemented in the collector.

Fixes #5074

model/otlpgrpc/logs.go Outdated Show resolved Hide resolved
model/otlpgrpc/logs_test.go Outdated Show resolved Hide resolved
model/otlpgrpc/metrics.go Outdated Show resolved Hide resolved
This is the last piece of upgrading to v0.15.0. This PR adds the tests and code to ensure the transition plan is implemented in the collector.

Fixes open-telemetry#5074
@codeboten codeboten force-pushed the codeboten/transition-scope branch from 9f21939 to 0e8ed3a Compare March 28, 2022 15:23
@codecov
Copy link

codecov bot commented Mar 28, 2022

Codecov Report

Merging #5085 (7a61880) into main (1d5fdd1) will decrease coverage by 0.09%.
The diff coverage is 73.97%.

@@            Coverage Diff             @@
##             main    #5085      +/-   ##
==========================================
- Coverage   90.04%   89.94%   -0.10%     
==========================================
  Files         183      183              
  Lines       11035    11103      +68     
==========================================
+ Hits         9936     9987      +51     
- Misses        877      891      +14     
- Partials      222      225       +3     
Impacted Files Coverage Δ
model/otlpgrpc/logs.go 50.58% <68.00%> (+7.03%) ⬆️
model/otlpgrpc/traces.go 50.58% <68.00%> (+7.03%) ⬆️
model/otlpgrpc/metrics.go 53.08% <85.00%> (+9.53%) ⬆️
model/otlp/json_unmarshaler.go 79.31% <100.00%> (+2.38%) ⬆️
service/collector.go 77.10% <0.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 1d5fdd1...7a61880. Read the comment docs.

@codeboten codeboten marked this pull request as ready for review March 28, 2022 20:48
@codeboten codeboten requested review from a team and bogdandrutu March 28, 2022 20:48
@@ -133,7 +138,11 @@ func (lr LogsRequest) MarshalJSON() ([]byte, error) {

// UnmarshalJSON unmarshalls LogsRequest from JSON bytes.
func (lr LogsRequest) UnmarshalJSON(data []byte) error {
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated to your change: looks like for some reason we now have 2 places in the codebase to unmarshal OTLP JSON. I am not sure if this duplication is intentional.

Copy link
Member

Choose a reason for hiding this comment

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

It is intentional because we have logically 2 "encodings":

  1. For Request/Response for grpc/http
  2. For persistent storage like Kafka/File etc. See proto Request message vs TracesData.

Co-authored-by: Tigran Najaryan <[email protected]>
@codeboten codeboten force-pushed the codeboten/transition-scope branch from 7085979 to 7a61880 Compare March 28, 2022 22:23
@@ -133,7 +138,11 @@ func (lr LogsRequest) MarshalJSON() ([]byte, error) {

// UnmarshalJSON unmarshalls LogsRequest from JSON bytes.
func (lr LogsRequest) UnmarshalJSON(data []byte) error {
Copy link
Member

Choose a reason for hiding this comment

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

It is intentional because we have logically 2 "encodings":

  1. For Request/Response for grpc/http
  2. For persistent storage like Kafka/File etc. See proto Request message vs TracesData.

@bogdandrutu bogdandrutu merged commit 831373a into open-telemetry:main Mar 29, 2022
Nicholaswang pushed a commit to Nicholaswang/opentelemetry-collector that referenced this pull request Jun 7, 2022
)

* add transition from instrumentationlibrary -> scope

This is the last piece of upgrading to v0.15.0. This PR adds the tests and code to ensure the transition plan is implemented in the collector.

Fixes open-telemetry#5074

* fix spacing, add link

* update unmarshaler, adding tests

* update grpc tests

* update changelog

* Update model/otlpgrpc/logs.go

Co-authored-by: Tigran Najaryan <[email protected]>

Co-authored-by: Tigran Najaryan <[email protected]>
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.

support transition phase of translating old proto to new proto
4 participants