-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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 gogo/protobuf to OpenTelemetry OTLP data model #5067
Conversation
Signed-off-by: Yuri Shkuro <github@ysh.us>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #5067 +/- ##
==========================================
- Coverage 95.62% 95.54% -0.08%
==========================================
Files 314 320 +6
Lines 18296 18461 +165
==========================================
+ Hits 17495 17639 +144
- Misses 642 660 +18
- Partials 159 162 +3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
if len(s.ParentSpanId) == 0 { | ||
// If ParentSpanId is empty array then gogo/jsonpb renders it as empty string. | ||
// To match the output with grpc-gateway we set it to nil and it won't be included. | ||
s.ParentSpanId = nil |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
field is not nullable so this fix is no longer possible
"status": {}, | ||
"status": { | ||
"code": "STATUS_CODE_ERROR" | ||
}, |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Added this and span kind as examples of enums. This matches the previous output when using grpc-gateway marshaling (and is different from OTLP-JSON encoding).
"name": "foobar", | ||
"parentSpanId": "", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could not find a way to suppress this empty field. In practice not a big deal all but one span in a trace will have non-empty parent. Also, verified that OTLP-JSON also includes empty parent ID.
@@ -0,0 +1,45 @@ | |||
// Copyright (c) 2023 The Jaeger Authors. | |||
// Copyright The OpenTelemetry Authors |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Copied from OTEL, but had to change to marshal IDs as base64, to keep with the original grpc-gateway version.
Which problem is this PR solving?
Description of the changes
How was this change tested?