fix SpanKind type assertion in opentracing bridge #2898
Conversation
Prajithp
requested review from
jmacd,
MrAlias,
Aneurysm9,
evantorrie,
XSAM,
dashpole,
MadVikingGod,
pellared and
hanyuancheung
as code owners
|
Can you fix the tests? |
MrAlias
added
blocked:CLA
|
@dmathieu Done. I have already signed the CLA document. |
Codecov Report
@@ Coverage Diff @@
## main #2898 +/- ##
=======================================
- Coverage 76.2% 76.2% -0.1%
=======================================
Files 179 179
Lines 11939 11940 +1
=======================================
- Hits 9103 9102 -1
- Misses 2596 2598 +2
Partials 240 240
|
|
Seems like the CLA is not signed yet, could you pls check again and fix it? @Prajithp |
|
@hanyuancheung Tried multiple times by signing the Doc. After clicking the Finish button, page is redirecting back to Github without any messages |
| @@ -497,8 +498,9 @@ func otTagsToOTelAttributesKindAndError(tags map[string]interface{}) ([]attribut | |||
| for k, v := range tags { | |||
| switch k { | |||
| case string(otext.SpanKind): | |||
| if s, ok := v.(string); ok { | |||
| switch strings.ToLower(s) { | |||
| refValue := reflect.ValueOf(v) | |||
There was a problem hiding this comment.
In order to avoid using the reflect package, how about taking the opposite approach, of turning strings into SpanKindEnum?
case string(otext.SpanKind):
if s, ok := v.(string); ok {
v = otext.SpanKindEnum(strings.ToLower(s))
}
if s, ok := v.(otext.SpanKindEnum); ok {
switch s {
case otext.SpanKindRPCClientEnum:
kind = trace.SpanKindClient
case otext.SpanKindRPCServerEnum:
kind = trace.SpanKindServer
case otext.SpanKindProducerEnum:
kind = trace.SpanKindProducer
case otext.SpanKindConsumerEnum:
kind = trace.SpanKindConsumer
}
}There was a problem hiding this comment.
Yes, this looks good to me if reflect package is not the ideal solution for this. Should I update the commit?
There was a problem hiding this comment.
Let's wait for another opinion.
There was a problem hiding this comment.
I agree that reflect seems out of place. I would maybe suggest a map[otext.SpanKindEnum]trace.SpanKind with a default so we don't leave a kind = "".
MrAlias
removed
the
blocked:CLA
We have system which supports multiple distributed tracing systems. When using opentracing bridge, all the spankinds are created as INTERNAL due to type assertion issue in bridge module.