-
Notifications
You must be signed in to change notification settings - Fork 316
Pass options along when creating a span with StartSpanFromContext #114
Conversation
Do you mind adding unit tests? |
here you go. |
@@ -21,6 +21,7 @@ func nextFakeID() int { | |||
type testSpanContext struct { | |||
HasParent bool | |||
FakeID int | |||
Options *StartSpanOptions |
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.
I'd have a slight preference to just add tags to testSpan
and test that they get passed through correctly... if you don't feel like implementing this, I can do it :)
@@ -28,6 +29,35 @@ func (n testSpanContext) ForeachBaggageItem(handler func(k, v string) bool) {} | |||
type testSpan struct { | |||
spanContext testSpanContext | |||
OperationName string | |||
StartTime time.Time | |||
Tags map[string]interface{} |
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.
struct equality won't work with a map member, so I implemented an Equal method.
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.
maybe adding an id and compare it would make more sense.
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.
LGTM, thanks.
Thanks @gwik... the test LGTM but I will give @yurishkuro a chance to provide feedback if he so desires... |
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.
please fix lint errors
testtracer_test.go:64:1: receiver name n should be consistent with previous receiver name s for testSpan
Options were ignored when creating a span.