<div dir="ltr"><div>Robert, in that exact class that you're showing (the `ThreadLocalCurrentTraceContext` [1] ) you have the method</div><div><br></div><div>@Override public Scope newScope(@Nullable TraceContext currentSpan) {<br> final TraceContext previous = local.get();<br> local.set(currentSpan);<br> Scope result = previous != null ? new RevertToPreviousScope(local, previous) : revertToNull;<br> return decorateScope(currentSpan, result);<br> }</div><div><br></div><div>That very class extends the CurrentTraceContext that requires to define how you create a new Scope. How will that work with Scoped Values?</div><div><br></div><div>[1] - <a href="https://github.com/openzipkin/brave/blob/6.0.3/brave/src/main/java/brave/propagation/ThreadLocalCurrentTraceContext.java">https://github.com/openzipkin/brave/blob/6.0.3/brave/src/main/java/brave/propagation/ThreadLocalCurrentTraceContext.java</a></div><div><br></div><div><div><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div>Pozdrawiam / Best regards,</div><div>Marcin Grzejszczak</div><div><br><div><a href="https://marcin.grzejszczak.pl" target="_blank">https://marcin.grzejszczak.pl</a></div><div><a href="https://toomuchcoding.com" target="_blank">https://toomuchcoding.com</a></div></div></div></div></div></div></div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">śr., 12 cze 2024 o 12:55 robert engels <<a href="mailto:robaho@icloud.com">robaho@icloud.com</a>> napisał(a):<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div dir="auto"><div dir="ltr"></div><div dir="ltr">Also, looking at brave, they seem to understand the various ways of managing the current context. See <a href="https://github.com/openzipkin/brave/tree/master/brave/src/main/java/brave/propagation" target="_blank">https://github.com/openzipkin/brave/tree/master/brave/src/main/java/brave/propagation</a></div><div dir="ltr"><br></div><div dir="ltr">and so plugging into this a context based on a ScopedValue seems trivial (see notes in TraceContextThreadLocal)</div><div dir="ltr"><br></div><div dir="ltr">I would expect other libraries have similar features. </div><div dir="ltr"><br><blockquote type="cite">On Jun 12, 2024, at 7:17 AM, robert engels <<a href="mailto:robaho@icloud.com" target="_blank">robaho@icloud.com</a>> wrote:<br><br></blockquote></div><blockquote type="cite"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"><div dir="ltr"></div><div dir="ltr">The code I provided will also work with before/after only tracers - the around is a simplification of this. </div><div dir="ltr"><br></div><div dir="ltr">Wrapping of the Runnable would happen the same way all instrumentation is done: manually, byte code manipulation, or dynamic proxies. </div><div dir="ltr"><br></div><div dir="ltr">The SV vs TL is only how the current context is obtained. It is trivial to change between the two. </div><div dir="ltr"><br></div><div dir="ltr">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. </div><div dir="ltr"><br></div><div dir="ltr">Or do nothing, TLs are not going away. </div><div dir="ltr"><br></div><div dir="ltr"><br></div><div dir="ltr"><br></div><div dir="ltr"><br><blockquote type="cite">On Jun 12, 2024, at 4:07 AM, Marcin Grzejszczak <<a href="mailto:marcin.grzejszczak@gmail.com" target="_blank">marcin.grzejszczak@gmail.com</a>> wrote:<br><br></blockquote></div><blockquote type="cite"><div dir="ltr"><div dir="ltr"><div>> I posted a minimalist “tracing library” using ScopedValue here <a href="https://github.com/robaho/scope_trace" target="_blank">https://github.com/robaho/scope_trace</a></div><div><br></div><div>Thanks for doing the example. </div><div><br></div><div>>I think it is a fairly trivial change for tracing libraries to support ScopeValue instead of ThreadLocal if they wished to.</div><div><br></div><div>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.<br></div><div><br></div><div><div><div dir="ltr" class="gmail_signature"><div dir="ltr"><div><div dir="ltr"><div>Pozdrawiam / Best regards,</div><div>Marcin Grzejszczak</div><div><br><div><a href="https://marcin.grzejszczak.pl" target="_blank">https://marcin.grzejszczak.pl</a></div><div><a href="https://toomuchcoding.com" target="_blank">https://toomuchcoding.com</a></div></div></div></div></div></div></div><br></div></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">wt., 11 cze 2024 o 15:25 Robert Engels <<a href="mailto:robaho@icloud.com" target="_blank">robaho@icloud.com</a>> napisał(a):<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div>I posted a minimalist “tracing library” using ScopedValue here <a href="https://github.com/robaho/scope_trace" target="_blank">https://github.com/robaho/scope_trace</a><div><br></div><div>I think it is a fairly trivial change for tracing libraries to support ScopeValue instead of ThreadLocal if they wished to.</div><div><div> <br><div><br><blockquote type="cite"><div>On Jun 11, 2024, at 7:35 AM, Andrew Haley <<a href="mailto:aph-open@littlepinkcloud.com" target="_blank">aph-open@littlepinkcloud.com</a>> wrote:</div><br><div><div>On 6/11/24 11:32, Marcin Grzejszczak wrote:<br><br>>> So why not produce an example of how it would be used? As far as I can<br>>> see, your proposal fails to ensure the property in the case I posted/<br>><br>> I've created samples at the very beginning with the before and after<br>> interceptor. I obviously know that I could rewrite the interceptor<br>> to become an around one but my point is that it's not an ideal<br>> solution to require instrumentors to rewrite everything. Also I<br>> still see this whole discussion as having two ways of doing the same<br>> thing. Current approach:<br>><br>> ScopedValue.runWhere(scopedValue, someValue, () -> {<br>> codeToRun();<br>> });<br>><br>> Proposed approach:<br>><br>> try(Scope scope = ScopedValue.openScope(scopedValue, someValue)) {<br>> codeToRun();<br><br>This API fails the test I provided:<br><br> final ScopedValue aScopedValue = ...;<br> final var x = aScopedValue.get();<br> ScopedValue.openScope(aScopedValue, someValue);<br> final var y = aScopedValue.get();<br><br> assert(x == y); // Always succeeds<br><br>In other words, scoped values would no longer be strictly scoped.<br>That's what the "scoped" means in the name "scoped values". Strict<br>scoping is a fundamental property.<br><br>> }<br>><br>> I understand that in the proposed approach one can mess things up, I<br>> really do. But library instrumentors and tracing library creators<br>> would benefit from that sort of API.<br><br>If we were planning to deprecate thread-local variables I think this<br>argument might have some merit. But we're not. They're not going away.<br><br>> Also I've asked a different question... What if the users want to<br>> leverage the ScopedValues to get the current span information (or<br>> current principal from security point of view) but also they are<br>> using TL based libraries? How would the interop look like?<br><br>As the JEP says, scoped values do not replace all uses of thread-local<br>variables. There are still some cases where thread-local variables<br>will be used, and this looks like one of them.<br><br>> Let's assume that they put into the current scope a span that they<br>> want to have within that scope, we would need all the tracer<br>> libraries (and security components and others that use the same<br>> mechanism) to be ScopeValue aware. Would they first try to read SV<br>> and then if that's not there a TL?<br><br>Yes, I think so.<br><br>> They would need to most likely read SV and populate TL if the SV<br>> value is present so that other components that still rely on TL<br>> would work. That would have to also work the other way round, if a<br>> TL based library sets a value (e.g. span) then maybe the ScopedValue<br>> should be created with a scope for that new value?<br><br>I'm not sure how that'd work in general, but there's nothing to stop<br>entry points for a library from doing so, and it might be a good idea.<br>Something like:<br><br> if (someThreadLocal.get() != NOTHING) {<br> where(someScopedValue, someThreadLocal.get()).run(realEntry);<br> } else {<br> realEntry.run();<br> }<br><br>-- <br>Andrew Haley (he/him)<br>Java Platform Lead Engineer<br>Red Hat UK Ltd. <<a href="https://www.redhat.com" target="_blank">https://www.redhat.com</a>><br><a href="https://keybase.io/andrewhaley" target="_blank">https://keybase.io/andrewhaley</a><br>EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671<br><br></div></div></blockquote></div><br></div></div></div></blockquote></div>
</div></blockquote></div></div></div></div></div></blockquote></div></blockquote></div>