Update OTLP to v0.4.0 [Breaking change] #1142

tigrannajaryan · 2020-06-18T19:24:11Z

This is a breaking OTLP change.

Use AnyValue introduced in recent change to OTLP.
Changes are encapsulated in AttributeValue and most of the codebase is
unaffected, which proves the wrappers are very useful.
Rename AttributeKeyValue to KeyValue (the change comes from OTLP).
Use local protoc to compile ProtoBufs. Previously used znly/protoc
docker image is outdated and results in incorrect code for gRPC-Gateway.

TODO:

Need to add support for ARRAY value type. This is not urgent since
there are no known data sources that use the ARRAY type yet.
Use Gogoproto (gogoproto.nullable) = false annotation for AnyValue
to possibly improve performance further.

codecov · 2020-06-18T19:32:04Z

Codecov Report

Merging #1142 into master will decrease coverage by 0.01%.
The diff coverage is 93.46%.

@@            Coverage Diff             @@
##           master    #1142      +/-   ##
==========================================
- Coverage   87.49%   87.48%   -0.02%     
==========================================
  Files         202      202              
  Lines       14525    14641     +116     
==========================================
+ Hits        12709    12808      +99     
- Misses       1376     1388      +12     
- Partials      440      445       +5

Impacted Files	Coverage Δ
consumer/pdata/common.go	`95.07% <90.90%> (-3.51%)`	⬇️
internal/data/testdata/common.go	`100.00% <100.00%> (ø)`
internal/data/testdata/log.go	`100.00% <100.00%> (ø)`
internal/goldendataset/generator_commons.go	`88.46% <100.00%> (-0.83%)`	⬇️
internal/goldendataset/span_generator.go	`98.85% <100.00%> (ø)`
translator/internaldata/resource_to_oc.go	`81.39% <100.00%> (+4.92%)`	⬆️
exporter/fileexporter/file_exporter.go	`70.08% <0.00%> (-1.71%)`	⬇️

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 3253955...f75fb2f. Read the comment docs.

tigrannajaryan · 2020-06-23T17:54:35Z

@bogdandrutu please review especially the AttributeValue/AttributeMap changes.

tigrannajaryan · 2020-06-23T17:56:15Z

@keitwb Body of logs proto can be now updated to AnyValue in a future PR.

tigrannajaryan · 2020-06-23T17:56:51Z

@kbrockhoff there are some minor changes to golden data generator in this PR. You may want to review.

nilebox · 2020-06-24T00:12:08Z

translator/internaldata/resource_to_oc.go

+		m.ForEach(func(k string, v pdata.AttributeValue) {
+			kvs = append(kvs, fmt.Sprintf("%q:%s", k, attributeValueToString(v, true)))
+		})
+		return "{" + strings.Join(kvs, ",") + "}"


https://golang.org/pkg/strings/#Builder is more efficient

Also is there any need to worry about escaping , in the key or value or are these "JSON-like strings" not meant to be strictly parsed?

Changed to strings.Builder.

Escaping is handled correctly by %q formatter for the key and by recursive call to attributeValueToString for the values. Updated the tests to include backslash and double quote in the key and in the value.

This is a breaking OTLP change. - Use AnyValue introduced in recent change to OTLP. Changes are encapsulated in AttributeValue and most of the codebase is unaffected, which proves the wrappers are very useful. - Rename AttributeKeyValue to KeyValue (the change comes from OTLP). - Use local protoc to compile ProtoBufs. Previously used znly/protoc docker image is outdated and results in incorrect code for gRPC-Gateway. TODO: - Need to add support for ARRAY value type. This is not urgent since there are no known data sources that use the ARRAY type yet. - Use Gogoproto `(gogoproto.nullable) = false` annotation for AnyValue to possibly improve performance further.

owais · 2020-06-25T15:01:26Z

translator/internaldata/resource_to_oc.go

 }

-func attributeValueToString(attr pdata.AttributeValue) string {
+func attributeValueToString(attr pdata.AttributeValue, jsonLike bool) string {


do we even translate this back from string?

I am not sure I understand the question.

owais · 2020-06-25T15:02:49Z

translator/internaldata/resource_to_oc.go

+
+	case pdata.AttributeValueMAP:
+		// OpenCensus attributes cannot represent maps natively. Convert the
+		// map to a JSON-like string.


Why not JSON? Do we want this to be just human-readable representation or do we expect someone to attempt to translate it back?

It is a stringified JSON. I think it is a good compromise since it preserves the full information and can be translated back, yet it is also more or less human readable.
I am open to other suggestions on how else we can translate this.

This is a breaking OTLP change. - Use AnyValue introduced in recent change to OTLP. Changes are encapsulated in AttributeValue and most of the codebase is unaffected, which proves the wrappers are very useful. - Rename AttributeKeyValue to KeyValue (the change comes from OTLP). - Use local protoc to compile ProtoBufs. Previously used znly/protoc docker image is outdated and results in incorrect code for gRPC-Gateway. TODO: - Need to add support for ARRAY value type. This is not urgent since there are no known data sources that use the ARRAY type yet. - Use Gogoproto `(gogoproto.nullable) = false` annotation for AnyValue to possibly improve performance further.

* Rename 'correlation' to 'baggage' * Rename CorrelationContext progator to Baggage * Update CHANGELOG

tigrannajaryan added the WIP label Jun 18, 2020

tigrannajaryan requested review from bogdandrutu, dmitryax, flands, owais, pjanotti, rghetia and songy23 as code owners June 18, 2020 19:24

tigrannajaryan self-assigned this Jun 18, 2020

tigrannajaryan force-pushed the feature/tigran/anyvalue branch 4 times, most recently from e50951d to 211b5c9 Compare June 22, 2020 21:27

tigrannajaryan mentioned this pull request Jun 23, 2020

Release v0.4.0 to match Collector v0.4.0 open-telemetry/opentelemetry-proto#161

Closed

tigrannajaryan force-pushed the feature/tigran/anyvalue branch 5 times, most recently from f84fbe7 to b746b29 Compare June 23, 2020 17:11

tigrannajaryan changed the title ~~[WIP] [DON'T REVIEW] Use AnyValue introduced in recent change to OTLP~~ Update OTLP to v0.4.0 Jun 23, 2020

tigrannajaryan removed the WIP label Jun 23, 2020

tigrannajaryan changed the title ~~Update OTLP to v0.4.0~~ Update OTLP to v0.4.0 [Breaking change] Jun 23, 2020

tigrannajaryan force-pushed the feature/tigran/anyvalue branch from b746b29 to 2a20003 Compare June 23, 2020 17:15

tigrannajaryan assigned keitwb and bogdandrutu Jun 23, 2020

tigrannajaryan assigned owais Jun 23, 2020

nilebox approved these changes Jun 24, 2020

View reviewed changes

tigrannajaryan force-pushed the feature/tigran/anyvalue branch from 2a20003 to a39bb23 Compare June 24, 2020 18:20

tigrannajaryan requested a review from james-bebbington as a code owner June 24, 2020 18:20

tigrannajaryan force-pushed the feature/tigran/anyvalue branch from a39bb23 to f75fb2f Compare June 24, 2020 18:21

nilebox approved these changes Jun 25, 2020

View reviewed changes

owais approved these changes Jun 25, 2020

View reviewed changes

bogdandrutu merged commit 4777e91 into open-telemetry:master Jun 25, 2020

tigrannajaryan deleted the feature/tigran/anyvalue branch June 25, 2020 21:46

flands added this to the Beta 0.5 milestone Jun 28, 2020

rmfitzpatrick mentioned this pull request Jul 27, 2020

Span Attributes are Null #1385

Closed

MovieStoreGuy pushed a commit to atlassian-forks/opentelemetry-collector that referenced this pull request Nov 11, 2021

Rename CorrelationContext to Baggage (open-telemetry#1142)

06689a0

* Rename 'correlation' to 'baggage' * Rename CorrelationContext progator to Baggage * Update CHANGELOG

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Update OTLP to v0.4.0 [Breaking change] #1142

Update OTLP to v0.4.0 [Breaking change] #1142

Uh oh!

tigrannajaryan commented Jun 18, 2020 •

edited

Loading

Uh oh!

codecov bot commented Jun 18, 2020 •

edited

Loading

Uh oh!

tigrannajaryan commented Jun 23, 2020

Uh oh!

tigrannajaryan commented Jun 23, 2020

Uh oh!

tigrannajaryan commented Jun 23, 2020

Uh oh!

nilebox Jun 24, 2020

Uh oh!

keitwb Jun 24, 2020

Uh oh!

tigrannajaryan Jun 24, 2020

Uh oh!

owais Jun 25, 2020

Uh oh!

tigrannajaryan Jun 25, 2020

Uh oh!

owais Jun 25, 2020

Uh oh!

tigrannajaryan Jun 25, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Update OTLP to v0.4.0 [Breaking change] #1142

Update OTLP to v0.4.0 [Breaking change] #1142

Uh oh!

Conversation

tigrannajaryan commented Jun 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

codecov bot commented Jun 18, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

tigrannajaryan commented Jun 23, 2020

Uh oh!

tigrannajaryan commented Jun 23, 2020

Uh oh!

tigrannajaryan commented Jun 23, 2020

Uh oh!

nilebox Jun 24, 2020

Choose a reason for hiding this comment

Uh oh!

keitwb Jun 24, 2020

Choose a reason for hiding this comment

Uh oh!

tigrannajaryan Jun 24, 2020

Choose a reason for hiding this comment

Uh oh!

owais Jun 25, 2020

Choose a reason for hiding this comment

Uh oh!

tigrannajaryan Jun 25, 2020

Choose a reason for hiding this comment

Uh oh!

owais Jun 25, 2020

Choose a reason for hiding this comment

Uh oh!

tigrannajaryan Jun 25, 2020

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

tigrannajaryan commented Jun 18, 2020 •

edited

Loading

codecov bot commented Jun 18, 2020 •

edited

Loading