ScopedValue.runWhere not returning scope

Thu Jun 13 09:48:27 UTC 2024

Robert, in that exact class that you're showing (the
`ThreadLocalCurrentTraceContext` [1] ) you have the method

@Override public Scope newScope(@Nullable TraceContext currentSpan) {
    final TraceContext previous = local.get();
    local.set(currentSpan);
    Scope result = previous != null ? new RevertToPreviousScope(local,
previous) : revertToNull;
    return decorateScope(currentSpan, result);
  }

That very class extends the CurrentTraceContext that requires to define how
you create a new Scope. How will that work with Scoped Values?

[1] -
https://github.com/openzipkin/brave/blob/6.0.3/brave/src/main/java/brave/propagation/ThreadLocalCurrentTraceContext.java

Pozdrawiam / Best regards,
Marcin Grzejszczak

https://marcin.grzejszczak.pl
https://toomuchcoding.com

śr., 12 cze 2024 o 12:55 robert engels <robaho at icloud.com> napisał(a):

> Also, looking at brave, they seem to understand the various ways of
> managing the current context. See
> https://github.com/openzipkin/brave/tree/master/brave/src/main/java/brave/propagation
>
> and so plugging into this a context based on a ScopedValue seems trivial
> (see notes in TraceContextThreadLocal)
>
> I would expect other libraries have similar features.
>
> On Jun 12, 2024, at 7:17 AM, robert engels <robaho at icloud.com> wrote:
>
> 
> 
> 
> 
> 
> The code I provided will also work with before/after only tracers - the
> around is a simplification of this.
>
> Wrapping of the Runnable would happen the same way all instrumentation is
> done: manually, byte code manipulation, or dynamic proxies.
>
> The SV vs TL is only how the current context is obtained. It is trivial to
> change between the two.
>
> I think it would be helpful if you created a project that uses the aspects
> that demonstrates your perceived problem rather than the abstract
> solutions. This feels like an XY problem.
>
> Or do nothing, TLs are not going away.
>
>
>
>
> On Jun 12, 2024, at 4:07 AM, Marcin Grzejszczak <
> marcin.grzejszczak at gmail.com> wrote:
>
> 
> > I posted a minimalist “tracing library” using ScopedValue here
> https://github.com/robaho/scope_trace
>
> Thanks for doing the example.
>
> >I think it is a fairly trivial change for tracing libraries to support
> ScopeValue instead of ThreadLocal if they wished to.
>
> As I presented by showing AFAIR 6 projects, not every single time the
> change would be trivial. Not all libraries give you the option to
> instrument the actual runnable (like an around aspect). Sometimes you have
> only before and after. Again, let me emphasise, I'm not saying that
> changing these libraries would not be possible. I'm saying that it wouldn't
> always be trivial and would require changes in various different projects.
>
> Pozdrawiam / Best regards,
> Marcin Grzejszczak
>
> https://marcin.grzejszczak.pl
> https://toomuchcoding.com
>
>
> wt., 11 cze 2024 o 15:25 Robert Engels <robaho at icloud.com> napisał(a):
>
>> I posted a minimalist “tracing library” using ScopedValue here
>> https://github.com/robaho/scope_trace
>>
>> I think it is a fairly trivial change for tracing libraries to support
>> ScopeValue instead of ThreadLocal if they wished to.
>>
>>
>> On Jun 11, 2024, at 7:35 AM, Andrew Haley <aph-open at littlepinkcloud.com>
>> wrote:
>>
>> On 6/11/24 11:32, Marcin Grzejszczak wrote:
>>
>> >> So why not produce an example of how it would be used? As far as I can
>> >> see, your proposal fails to ensure the property in the case I posted/
>> >
>> > I've created samples at the very beginning with the before and after
>> > interceptor. I obviously know that I could rewrite the interceptor
>> > to become an around one but my point is that it's not an ideal
>> > solution to require instrumentors to rewrite everything. Also I
>> > still see this whole discussion as having two ways of doing the same
>> > thing. Current approach:
>> >
>> > ScopedValue.runWhere(scopedValue, someValue, () -> {
>> >        codeToRun();
>> >      });
>> >
>> > Proposed approach:
>> >
>> > try(Scope scope = ScopedValue.openScope(scopedValue, someValue)) {
>> >    codeToRun();
>>
>> This API fails the test I provided:
>>
>>   final ScopedValue aScopedValue = ...;
>>   final var x = aScopedValue.get();
>>   ScopedValue.openScope(aScopedValue, someValue);
>>   final var y = aScopedValue.get();
>>
>>   assert(x == y); // Always succeeds
>>
>> In other words, scoped values would no longer be strictly scoped.
>> That's what the "scoped" means in the name "scoped values". Strict
>> scoping is a fundamental property.
>>
>> > }
>> >
>> > I understand that in the proposed approach one can mess things up, I
>> > really do. But library instrumentors and tracing library creators
>> > would benefit from that sort of API.
>>
>> If we were planning to deprecate thread-local variables I think this
>> argument might have some merit. But we're not. They're not going away.
>>
>> > Also I've asked a different question... What if the users want to
>> > leverage the ScopedValues to get the current span information (or
>> > current principal from security point of view) but also they are
>> > using TL based libraries? How would the interop look like?
>>
>> As the JEP says, scoped values do not replace all uses of thread-local
>> variables. There are still some cases where thread-local variables
>> will be used, and this looks like one of them.
>>
>> > Let's assume that they put into the current scope a span that they
>> > want to have within that scope, we would need all the tracer
>> > libraries (and security components and others that use the same
>> > mechanism) to be ScopeValue aware. Would they first try to read SV
>> > and then if that's not there a TL?
>>
>> Yes, I think so.
>>
>> > They would need to most likely read SV and populate TL if the SV
>> > value is present so that other components that still rely on TL
>> > would work. That would have to also work the other way round, if a
>> > TL based library sets a value (e.g. span) then maybe the ScopedValue
>> > should be created with a scope for that new value?
>>
>> I'm not sure how that'd work in general, but there's nothing to stop
>> entry points for a library from doing so, and it might be a good idea.
>> Something like:
>>
>>    if (someThreadLocal.get() != NOTHING) {
>>        where(someScopedValue, someThreadLocal.get()).run(realEntry);
>>    } else {
>>        realEntry.run();
>>    }
>>
>> --
>> Andrew Haley  (he/him)
>> Java Platform Lead Engineer
>> Red Hat UK Ltd. <https://www.redhat.com>
>> https://keybase.io/andrewhaley
>> EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671
>>
>>
>>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://mail.openjdk.org/pipermail/loom-dev/attachments/20240613/1585ee27/attachment-0001.htm>