Removes the need to set OR on OTLA #3986
Conversation
without this change we are required to set OR on OTLA so that when NullObservation gets created we will call the proper handlers to e.g. clear tracing values. with this change we no longer require to set OR on OTLA because we will reuse the OR that was set on the Observation before clearing happens in OTLA via `setValue()`. When there was no Observation before calling `setValue()` we just skip creating any scopes. That way we are always reusing the ObservationRegistry that is used between Observations.
Sonatype Lift is retiringSonatype Lift will be retiring on Sep 12, 2023, with its analysis stopping on Aug 12, 2023. We understand that this news may come as a disappointment, and Sonatype is committed to helping you transition off it seamlessly. If you’d like to retain your data, please export your issues from the web console. |
| * @return corresponding observation registry | ||
| * @since 1.10.10 | ||
| */ | ||
| default ObservationRegistry getObservationRegistry() { |
There was a problem hiding this comment.
Are we able to operate on the ObservationRegistry.ObservationConfig level instead? All the calls to the registry to obtain the current Observation/Scope can go to the static, global instance.
There was a problem hiding this comment.
They cannot because we need an ObservationRegistry to create an Observation, not an ObservationConfig
| child.stop(); | ||
| parent.stop(); | ||
|
|
||
| then(child.getEnclosingScope()).isNull(); | ||
| then(parent.getEnclosingScope()).isNull(); | ||
| } | ||
|
|
||
| @Test | ||
| void uses2DifferentObservationRegistries() { | ||
| AtomicReference<String> reference = new AtomicReference<>(""); |
There was a problem hiding this comment.
rename to lastCalledHandler perhaps?
| ContextRegistry.getInstance().removeContextAccessor(mapContextAccessor); | ||
| ContextRegistry.getInstance().registerContextAccessor(mapContextAccessor); |
There was a problem hiding this comment.
Please consider not coupling the ObservationView with the registry even further in the public API - perhaps we can evolve the usage in the future to have ObservationRegistry be simply global and static for ThreadLocal access, while the ObservationConfig is something that can vary from Observation to Observation.
We can create a separate issue for that. I don't see real value of doing such a change at this point |
Right. The fact |
without this change we are required to set OR on OTLA so that when NullObservation gets created we will call the proper handlers to e.g. clear tracing values.
with this change we no longer require to set OR on OTLA because we will reuse the OR that was set on the Observation before clearing happens in OTLA via
setValue(). When there was no Observation before callingsetValue()we just skip creating any scopes. That way we are always reusing the ObservationRegistry that is used between Observations.