Implement scope.close method

openzipkin-contrib / brave-opentracing

Bridge between OpenTracing and Brave

http://opentracing.io

Apache License 2.0

64 stars 39 forks source link

Implement scope.close method #83

Closed pavolloffay closed 6 years ago

pavolloffay commented 6 years ago

Signed-off-by: Pavol Loffay ploffay@redhat.com

pavolloffay commented 6 years ago

this programming pattern can cause misalignment bugs as it encourages people to close a scope they didn't open. I left a comment here opentracing/opentracing-java#267 (comment)

I agree, however this should be changed in the OT API and not by not implementing the method correctly. The issues is that the current implementation does not work with many OT instrumentations:

codefromthecrypt commented 6 years ago

sorry, the javadoc says this is only used to get the active span.

https://github.com/opentracing/opentracing-java/blob/master/opentracing-api/src/main/java/io/opentracing/ScopeManager.java#L39

that part is implemented and you guys have no tests. This issue will be left open to encourage you to fix the api.

codefromthecrypt commented 6 years ago

by the way. you personally wrote or copied most of the instrumentations that don't work. You can fix those.