Deprecate AutoFinishScopeManager #335
Conversation
There was a problem hiding this comment.
I am in favour of deprecating this, but please explain the reason a bit more.
@yurishkuro ?
| @@ -18,6 +18,13 @@ | |||
|
|
|||
| import java.util.concurrent.atomic.AtomicInteger; | |||
|
|
|||
| /** | |||
| * @deprecated use {@link ThreadLocalScopeManager} instead. | |||
There was a problem hiding this comment.
Does this explain you you can still 'automatically' finish spans when scope is closed?
Or do we not want that anymore?
i.e. Please describe the reason for deprecating this functionality
There was a problem hiding this comment.
Added a (very) small note. Let me know if you think we need further clarification/details ;)
|
The operating mode this classes proposes is in contrast with the main philosophy of 0.32 where we deprecated autoFinish flag in activate (), because it too easily leads to programming mistakes of using try-with-resource on the scope and not having the right span to handle errors. I think we should stick with one recommended instrumentation pattern, shown in 0.32 readme, instead of having the instrumentation be dependent on which scope manager is in use. |
|
Also, the historical context is that we wanted to preserve this for people migrating from 0.30. That being said, I will add this and @yurishkuro 's note and then merge. |
| /** | ||
| * @deprecated use {@link ThreadLocalScope} instead. | ||
| * The operation mode of this class contrasts with the 0.32 | ||
| * deprecation of auto finishing {@link Span}s upon {@link Scope#close()}. |
There was a problem hiding this comment.
Could also add a link to this issue for more background.
|
Hey hey guys - if you are ok with this, I will merge later today, and issue the 0.32 release right after ;) Let me know. |
* Deprecate the StringTag.set() overload taking a StringTag. (#262) * Implement Trace Identifiers. (#280) * Bump JaCoCo to use a Java 10 friendly version (#306) * Remove finish span on close (#301) * Deprecate finishSpanOnClose on activation. * Add ScopeManager.activeSpan() and Tracer.activateSpan(). * Clarify the API changes and deprecations. * Add an error reporting sample to opentracing-testbed. * Simple layer on top of ByteBuffer for BINARY format. (#276) * Add generic typed setTag/withTag (#311) * Allow injecting into maps of type Map<String,Object> (#310) * Add simple registerIfAbsent to global tracer (#289) * Split Inject and Extract Builtin interfaces (#316) * Deprecate ScopeManager.active() (#326) * Make Tracer extends Closable. (#329) * Do not make isRegistered() synchronized. (#333) * Deprecate AutoFinishScopeManager (#335)
The last fix before releasing 0.32 ;)
@yurishkuro