Bump OpenTracing to 0.32.0 #14
Conversation
Signed-off-by: Pavol Loffay <ploffay@redhat.com>
|
Once merged I will release it as 0.3.0 |
objectiser
left a comment
objectiser
left a comment
There was a problem hiding this comment.
LGTM - just the one issue.
| new TracedRunnable(runnable, tracer)); | ||
| Span toActivate = span != null ? span : tracer.activeSpan(); | ||
| return delegate.submit(toActivate == null ? runnable : | ||
| new TracedRunnable(runnable, tracer), toActivate); |
There was a problem hiding this comment.
The toActivate should be in the TracedRunnable parameters I believe?
sjoerdtalsma
left a comment
sjoerdtalsma
left a comment
There was a problem hiding this comment.
Aside from the naming comment, functionally this looks good!
| } | ||
|
|
||
| Scope createScope(String operationName) { | ||
| Span createSpan(String operationName) { |
There was a problem hiding this comment.
Nit: The method name does not indicate that there may not be a span created (which in cases with active spans would actually be the norm).
The name should make it feel natural that null gets returned and in which situations. Something like createNewSpanWhenNoActiveSpanExists() may be a bit too verbose, but I hope you get my point.
This rename may be a good time to think of a better name.
There was a problem hiding this comment.
thanks @sjoerdtalsma I will leave it as it was. The method also takes into account an internal parameter whether to create a span or not.
3 participants
Signed-off-by: Pavol Loffay ploffay@redhat.com