<div dir="ltr"><div>> I updated the <a href="http://github.com/robaho/scope_trace">github.com/robaho/scope_trace</a> code to demonstrate before/after tracing using ScopedValues. There are no ThreadLocals in the code. It works fine.</div><div><br></div><div>Thanks for doing this, I really appreciate it! </div><div><br></div><div>You create a SV at the beginning when the "request" is received. Try to create a "span" with a new SV for the JDBC statement. With how JDBC is being instrumented you actually don't have a handle on when and how the JDBC code is called. You only have access to BEFORE and AFTER. So in the BEFORE you would create a new span, put it in scope so that e.g. MDC get enriched with tracing data so that you will have subsequent logs with span and trace ids and then in the afterStatement you would stop the scope, clear MDC and close that new span. </div><div><br></div><div>I think that over the course of those 40 emails or so we're talking all the time about the same thing. You're trying to convince me that it's enough to create a SV at the beginning of code execution and wrap the actual execution in the runWhere which is not the case. When you do that you have in fact 1 span per request and then you just retrieve it in all your methods, there's no span nesting, no parent-child relationship. </div><div><br></div><div>In a normal application you would have 1 span for getting the request, at least 1 span for the JDBC call and 1 for each of the http calls. In your case you have 1 span for all 4.<br></div><div><br></div><div>> The problem with open/close semantics is that it is easy to close the scopes out of order - which corrupts the tracing information. <br></div><div><br></div><div>Yes, I mentioned that a couple of times already.</div><div><br></div><div>> Again referring to brave, look at <a href="https://github.com/openzipkin/brave/blob/69003dfc811418f0dbc42e9e17ff880ebe1f4b02/instrumentation/mysql/src/main/java/brave/mysql/TracingStatementInterceptor.java#L39">https://github.com/openzipkin/brave/blob/69003dfc811418f0dbc42e9e17ff880ebe1f4b02/instrumentation/mysql/src/main/java/brave/mysql/TracingStatementInterceptor.java#L39</a></div><div><br></div><div>This line creates a new span, whose parent was the current one in ThreadLocal and it also puts the newly created span in ThreadLocal. That's because the ThreadLocalSpan is being used. You don't have to use that API, you could create a span and put it in scope, that way the information about the underlying ThreadLocal storage mechanism wouldn't leak.<br></div><div><br></div><div>> This is an example of before/after tracing. It REQUIRES that the preStatement() and postStatment() will ALWAYS be executed in the correct sequence (with no intervening spans left open).</div><div><br></div><div>Looking at the above what happens is that in pre, a new span is created and put in thread local. It is then closed in the post and removed from ThreadLocal. That's not what you're doing in beforeStatement and afterStatement in your example.<br></div><div><br></div><div>> This code would need to changed regardless, most likely to something like Brave.currentTracer() which would be able to obtain the current tracer appropriately. I don’t see how adding open/close to ScopedValue values would alleviate this.</div><div><br></div><div>Typically this information gets passed via a constructor. Most likely in this case some JDBC SPI is being called and that class is instantiated without any dependencies.</div><div><br></div><div>> I am guessing most libraries have these hard-coded dependencies on ThreadLocal.</div><div><br></div><div>They do not. They use abstractions with scopes. Micrometer Observation gets the current observation from an ObservationRegistry. Underneath the default implementation does look into TL but it could look into SV.<br></div><div><br></div><div>> Thus, someone will write a VirtualThread/ScopedValue “native” tracing library, and that will be what people use - or developers will need to modify their libraries to use ScopedValues in an APPROPRIATE manner. <br></div><div><br></div><div>I'm trying to agree with you what APPROPRIATE means because I still don't understand because I think we're comparing apples to oranges at this point.<br></div><div><br></div><div>>  You seem to believe that a tracing library user will want to create a new span on every ScopedValue.runWhere() - I don’t think that is the case - as it will generate a lot of noise.</div><div><br></div><div>I know a few things about tracing libraries and instrumenting projects because I've been doing it for a decade. I never said that you should create a span on every runWhere. Assuming that tracing libraries would be using SV then I can imagine that when a Span would be put in scope then if it's not there in scope already then runWhere should be run.<br></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">pon., 17 cze 2024 o 14:14 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 style="overflow-wrap: break-word;">I updated the <a href="http://github.com/robaho/scope_trace" target="_blank">github.com/robaho/scope_trace</a> code to demonstrate before/after tracing using ScopedValues. There are no ThreadLocals in the code. It works fine.<div><br></div><div>The problem with open/close semantics is that it is easy to close the scopes out of order - which corrupts the tracing information. </div><div><br></div><div>Again referring to brave, look at <a href="https://github.com/openzipkin/brave/blob/69003dfc811418f0dbc42e9e17ff880ebe1f4b02/instrumentation/mysql/src/main/java/brave/mysql/TracingStatementInterceptor.java#L39" target="_blank">https://github.com/openzipkin/brave/blob/69003dfc811418f0dbc42e9e17ff880ebe1f4b02/instrumentation/mysql/src/main/java/brave/mysql/TracingStatementInterceptor.java#L39</a></div><div><br></div><div>This is an example of before/after tracing. It REQUIRES that the preStatement() and postStatment() will ALWAYS be executed in the correct sequence (with no intervening spans left open).</div><div><br></div><div>Notice also </div><div><br></div><div>Span span = ThreadLocalSpan.CURRENT_TRACER.next();</div><div><br></div><div>This code would need to changed regardless, most likely to something like Brave.currentTracer() which would be able to obtain the current tracer appropriately. <b>I don’t see how adding open/close to ScopedValue values would alleviate this.</b></div><div><br></div><div>I am guessing most libraries have these hard-coded dependencies on ThreadLocal.</div><div><br></div><div>Thus, someone will write a VirtualThread/ScopedValue “native” tracing library, and that will be what people use - or developers will need to modify their libraries to use ScopedValues in an APPROPRIATE manner. You seem to believe that a tracing library user will want to create a new span on every ScopedValue.runWhere() - I don’t think that is the case - as it will generate a lot of noise.</div><div><br></div><div>“scope values” are not only for tracing scopes.</div><div><br></div><div><br></div><div><div><blockquote type="cite"><div>On Jun 17, 2024, at 7:01 AM, robert engels <<a href="mailto:robaho@icloud.com" target="_blank">robaho@icloud.com</a>> wrote:</div><br><div><div dir="auto"><div dir="ltr"></div><div dir="ltr">Your statements like “a user puts a scope in a SV won’t be honored”. Of course not. I think you are trying to conflate scoped values with tracing scopes. They are not the same thing and can’t be treated as such due to the volume. </div><div dir="ltr"><br></div><div dir="ltr">The user will manage scopes for tracing the way they do now. It is an implementation detail on how the tracing library manages these scopes. They currently use thread locals. As I’ve pointed out several times the library can use a scoped value to carry the scope context - and mange the active scopes within in </div><div dir="ltr"><br></div><div dir="ltr">You are trying to make the scoped values be able to support tracing directly. I don’t believe that was ever a design goal. </div><div dir="ltr"><br><blockquote type="cite">On Jun 17, 2024, at 5:59 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>> Definitely, yes. If you need to create a context for every virtual thread, scoped values versus thread-local variables is the least of your problems.</div><div><br></div><div>Can you elaborate please?<br></div><div><br></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><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">czw., 13 cze 2024 o 19:23 Andrew Haley <<a href="mailto:aph-open@littlepinkcloud.com" target="_blank">aph-open@littlepinkcloud.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">On 6/13/24 16:03, Robert Engels wrote:<br>
> 3. More importantly, if you use VT effectively, you are talking about<br>
> orders of magnitude more “trace contexts” (in the default brave handler<br>
> they are shared by thread so they are highly limited since the only<br>
> effective Java threading implementation prior to VT required pooled<br>
> threads) which reinforces my point about the increase in the volume of<br>
> unique data is the major problem for existing tracing libraries, which<br>
> will probably require architectural changes anyway.<br>
<br>
Definitely, yes. If you need to create a context for every virtual thread,<br>
scoped values versus thread-local variables is the least of your problems.<br>
<br>
-- <br>
Andrew Haley  (he/him)<br>
Java Platform Lead Engineer<br>
Red Hat UK Ltd. <<a href="https://www.redhat.com/" rel="noreferrer" target="_blank">https://www.redhat.com</a>><br>
<a href="https://keybase.io/andrewhaley" rel="noreferrer" target="_blank">https://keybase.io/andrewhaley</a><br>
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671<br>
<br>
</blockquote></div>
</div></blockquote></div></div></blockquote></div><br></div></div></blockquote></div>