Avoid Complex objects on Span tags #249
Conversation
ghost
assigned
drodriguezhdez
left a comment
drodriguezhdez
left a comment
There was a problem hiding this comment.
I have some doubts about when span is checked if it is instance of our span.
| coverage.RestoreCoverageCounters() | ||
| } else if cov := coverage.EndCoverage(); cov != nil { | ||
| test.span.SetTag(tags.Coverage, *cov) | ||
| if sp, ok := test.span.(tracer.Span); ok { |
There was a problem hiding this comment.
Here it is checked if the span is our span, tag is gonna be set using unsafe, which means tag is not gonna be escaped?
There was a problem hiding this comment.
That's right the idea is that we know the coverage object is "safe" so if we detect our span, we ensure that we write the object without any modification. But if we are using other tracer we call the normal opentracing Api, that should be followed by the tracer.
There was a problem hiding this comment.
Is there any case where we should use the "escaped" mode in our spans? Or only it is needed when the user is adding custom spans?
There was a problem hiding this comment.
When the user is adding custom spans, but we fallback to the Unescape mode if we don't detect a complex object, so its safe call the "escaped" version. Also there are some "helper" methods inside the api, that call directly to the api version of the SetTag (in our case the "escaped" version) so we need to play nice with the api.
This PR is for avoiding Complex objects on Span tags and enables a new api for that
UnsafeSetTag.The complex objects are transformed by the new
GetValidValuefunc.