search

opentracing / opentracing-java

OpenTracing API for Java. 🛑 This library is DEPRECATED! https://github.com/opentracing/specification/issues/163
http://opentracing.io
Apache License 2.0
1.68k stars 344 forks source link

Add generic typed setTag/withTag #311

Closed tylerbenson closed 5 years ago

tylerbenson commented 5 years ago

Reduces the need to call Tags.SOMETAG.getKey() and adds type safety for setTag if done from the span.

Fixes #271

coveralls commented 5 years ago

Coverage Status

Coverage decreased (-0.8%) to 76.645% when pulling abb8375adda3c374fe38d279e80305eec382ce3d on tylerbenson:tyler/withtag into 92023843474cc76f49af4660c2e19b9a13a46aeb on opentracing:v0.32.0.

yurishkuro commented 5 years ago

+1

felixbarny commented 5 years ago

Nice one. Why is this considered a breaking change?

yurishkuro commented 5 years ago

If someone implements the Span interface (e.g. as a decorator), then any change to the interface is a breaking change. But we typically allowed additions.

yurishkuro commented 5 years ago

We have 2 approvals. @tedsuo @austinlparker do you want to review?

carlosalberto commented 5 years ago

+1

tylerbenson commented 5 years ago

One thing to note, I didn't follow @cwe1ss's suggestion to split the method declaration out by type. I don't have anything against that. If y'all prefer the non-generic option I can do that instead. Thoughts?

tedsuo commented 5 years ago

LGTM

carlosalberto commented 5 years ago

@yurishkuro Approved - is it ok if I go ahead and merge?