fix zipkinv2 translation error tag handling #2253
Conversation
|
hm, oddly seeing this error Is that expected? I know some work around |
| @@ -164,6 +164,11 @@ func populateSpanStatus(tags map[string]string, status pdata.SpanStatus) { | |||
| delete(tags, tracetranslator.TagStatusMsg) | |||
| } | |||
| } | |||
|
|
|||
| if _, ok := tags[tracetranslator.TagError]; ok { | |||
There was a problem hiding this comment.
does the value matter? true or false?
There was a problem hiding this comment.
hmm, It looks like only lower case true is accepted, so i will update
ref:
|
Ignore the issue in the contrib tests, I sent open-telemetry/opentelemetry-collector-contrib#1757 to fix contrib |
Codecov Report
@@ Coverage Diff @@
## master #2253 +/- ##
=======================================
Coverage 92.05% 92.05%
=======================================
Files 272 272
Lines 15311 15326 +15
=======================================
+ Hits 14094 14109 +15
Misses 837 837
Partials 380 380
Continue to review full report at Codecov.
|
|
@bogdandrutu failing test looks unrelated to this PR fwiw |
|
@bogdandrutu would've been nice to see this get iincluded in v0.17 ...anything i can help with here? |
|
@ericmustin v0.17 was released yesterday. @bogdandrutu OK to merge this? |
|
@bogdandrutu lmk if anything else needed here to merge, i believe this is relevant to some end users onboarding with otel so would love to be able to fit into next release 🙇 |
|
This PR was marked stale due to lack of activity. It will be closed in 7 days. |
|
@bogdandrutu it's been 9 days so a bit concerned this may fall thru the cracks, just a friendly ping here, this is blocking an end user so would love to be able to see this get in for the next release to help with adoption. |
|
@ericmustin Most of the OTel community is on break, so I would expect a review from Monday, January 4th (around that date people will start to come back). That also means no releases during the next days. Other than that, let's definitely get an early review by then. cc @tigrannajaryan |
| @@ -128,3 +143,23 @@ func generateTraceSingleSpanMinmalResource() pdata.Traces { | |||
| rsc.Attributes().UpsertString(conventions.AttributeServiceName, "SoleAttr") | |||
| return td | |||
| } | |||
|
|
|||
| func generateTraceSingleSpanErrorStatus() pdata.Traces { | |||
There was a problem hiding this comment.
It's a nit but perhaps generateTraceOneSpanOneTraceID (or just testdata.GenerateTraceDataOneSpan) could be reused to make this even simpler?
There was a problem hiding this comment.
LGTM, just waiting for @bogdandrutu to make sure he does not have any further comments.
Description:
This PR addresses #2214 and, by extension, open-telemetry/opentelemetry-collector-contrib#1644. ZipkinV2 internal translation was not taking into account the
errortag. As mentioned in this Specification PR open-telemetry/opentelemetry-specification#1257, and confirmed by @owais , any presence of theerrortag should be used to indicate a span status ofErrorand is also overriding on any other tag that denotes status for zipkinLink to tracking Issue:
#2214
Testing:
Added a unit test to current zipkin translation tests