<html><head><meta http-equiv="Content-Type" content="text/html; charset=utf-8"></head><body style="word-wrap: break-word; -webkit-nbsp-mode: space; line-break: after-white-space;" class="">To be clear, I have no control whatsoever over what does or does not happen with Java. I am only stating my opinion - and trying to help!<div class=""><br class=""></div><div class=""><blockquote type="cite" class=""><div dir="ltr" class=""><div class="">From the beginning I want to implement everything using Scoped Values but I don't want to need to rewrite all my code. Neither as a library implementor nor in the instrumented projects.</div></div></blockquote><div class=""><br class=""></div><div class="">I do think it is strange that you want to take advantage of a new api, but you don’t want to do any work to do so??? This wouldn’t be an api change then, it would be a performance optimization behind the scenes. A primary purpose of the ScopedValue api addition is the automatic closing of the ScopeValue for better resource management in a VirtualThread system. <b class="">Your requested changes eliminate a (the?) primary purpose of the api change!</b> So, if you don’t want this, continue with ThreadLocal.</div><div class=""><br class=""></div><div class="">Still, if the tracing library code is properly structured I would doubt it would be more than a global search and replace in the library to switch from ThreadLocal to ScopedValues. You keep stating “<b class="">rewrite all of our code</b>” without providing any evidence of why that would be the case. </div><div class=""><br class=""></div><div class="">It is highly doubtful any instrumented projects would need to change. I looked at how the tracing support is done with Spring and the proxies and that is certainly wouldn’t be required there.</div><div class=""><br class=""></div><div class="">If you wanted no changes to instrumented projects whatsoever, you would need to use the “check if ScopedValue context exists, if not use context from ThreadLocal” design as was suggested - which is meaningless overhead compared to the overhead of tracing itself. And if the instrumented project is being changed to use VirtualThreads it SHOULD be changed at the same time to update the tracing library setup code.</div><div class=""><br class=""></div><div class="">I updated the my code to demonstrate the fallback to ThreadLocal when the top-level ScopedValue context is not available.</div><div class=""><br class=""></div><div class="">I also updated the code to work when the ScopedValue setup is not performed. I am pretty sure it works the way most tracing libraries are interfaced with.</div><div class=""><br class=""></div><div class="">I still think you are stuck and looking at the problem the wrong way.</div><div class=""><br class=""></div><div class="">The code I shared emits all of the data to performed nested traces and analysis.</div><div class=""><br class=""></div><div class=""><div class="">event Event[start=1718639998618, end=1718639998807, description=close span handleRequest, span=SpanID 1985, tags [request 943],parent SpanId -1], duration 189ms</div><div class="">event Event[start=1718639998709, end=1718639998709, description=open span callServer2, span=SpanID 6021, tags [],parent SpanId 1943], duration 0ms</div><div class="">event Event[start=1718639998709, end=1718639998807, description=close span callServer2, span=SpanID 6021, tags [],parent SpanId 1943], duration 98ms</div><div class="">event Event[start=1718639998807, end=1718639998807, description=open span making jdbc call, span=SpanID 7920, tags [jdbc],parent SpanId 1943], duration 0ms</div><div class="">event Event[start=1718639998807, end=1718639998807, description=call jdbc, span=SpanID 7920, tags [jdbc],parent SpanId 1943], duration 0ms</div><div class="">event Event[start=1718639998617, end=1718639998722, description=close span handleRequest, span=SpanID 1872, tags [request 885],parent SpanId -1], duration 105ms</div><div class="">event Event[start=1718639998808, end=1718639998808, description=open span making jdbc call, span=SpanID 7921, tags [jdbc],parent SpanId 1715], duration 0ms</div><div class="">event Event[start=1718639998808, end=1718639998808, description=call jdbc, span=SpanID 7921, tags [jdbc],parent SpanId 1715], duration 0ms</div><div class="">event Event[start=1718639998808, end=1718639998808, description=close span making jdbc call, span=SpanID 7921, tags [jdbc],parent SpanId 1715], duration 0ms</div><div class="">event Event[start=1718639998597, end=1718639998722, description=close span handleRequest, span=SpanID 71, tags [request 35],parent SpanId -1], duration 125ms</div></div><div class=""><br class=""></div><div class=""><br class=""></div><div class=""><div><br class=""><blockquote type="cite" class=""><div class="">On Jun 17, 2024, at 10:19 AM, Marcin Grzejszczak <<a href="mailto:marcin.grzejszczak@gmail.com" class="">marcin.grzejszczak@gmail.com</a>> wrote:</div><br class="Apple-interchange-newline"><div class=""><div dir="ltr" class=""><div class="">> Right, because we don’t want to use thread locals!!!. The code I posted supports nested spans. If you are using a ThreadLocal.CURRENT_TRACER - you only have a single value per thread - so either the spans are chained, or there is a stack.</div><div class=""><br class=""></div><div class="">I never said that I want you to use thread locals!!! From the beginning I want to implement everything using Scoped Values but I don't want to need to rewrite all my code. Neither as a library implementor nor in the instrumented projects.<br class=""></div><div class=""><br class=""></div><div class="">> I guarantee I can implement a full tracing library without using ThreadLocal and have it support nested spans, etc. How can I guarantee this? Because, you can implement tracing by passing “context” as a parameter to every method. If you read the ScopedValue documentation it states that the purpose of ScopedValue is to avoid needing to pass these method parameters. So given that, you can replace callMethod(context,parm1…) with callMethod(parm1,…). A tracing library is trivially implemented with a Context parameter.</div><div class=""><br class=""></div><div class="">I fully believe you but I don't want to need to rewrite everything!</div><div class=""><br class=""></div><div class="">> There is absolutely no need to add open/close semantics to ScopedValue to support tracing libraries.</div><div class=""><br class=""></div><div class="">Maybe someone else can do better at explaining this, I give up. I understand that this CAN be done. I just doubt that forcing us to rewrite all of our code even in a trivial fashion is OK, but I understood the message.<br class=""></div><div class=""><br class=""></div><div class=""><div dir="ltr" class="gmail_signature" data-smartmail="gmail_signature"><div dir="ltr" class=""><div class=""><div dir="ltr" class=""><div class="">Pozdrawiam / Best regards,</div><div class="">Marcin Grzejszczak</div><div class=""><br class=""><div class=""><a href="https://marcin.grzejszczak.pl/" target="_blank" class="">https://marcin.grzejszczak.pl</a></div><div class=""><a href="https://toomuchcoding.com/" target="_blank" class="">https://toomuchcoding.com</a></div></div></div></div></div></div></div><br class=""></div><br class=""><div class="gmail_quote"><div dir="ltr" class="gmail_attr">pon., 17 cze 2024 o 15:14 Robert Engels <<a href="mailto:robaho@icloud.com" class="">robaho@icloud.com</a>> napisał(a):<br class=""></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;" class="">That is not correct. There is a “trace context” per request at the top. The “trace context” supports nested spans (which I call scopes). I have changed the name to Span to make this clearer, and I also added Span tag support. If the parentScopeId ==-1, then you are at the top-level (request). As an example:<div class=""><br class=""><div class=""><div class="">request 409, event Event[start=1718636994650, end=1718636994650, description=open span making jdbc call, spanId=3824, parentSpanId=-1], duration 0ms</div><div class="">request 409, event Event[start=1718636994650, end=1718636994650, description=call jdbc, spanId=3824, <b class="">parentSpanId=825</b>], duration 0ms</div><div class="">request 409, event Event[start=1718636994650, end=1718636994650, description=close span making jdbc call, spanId=3824, <b class="">parentSpanId=825</b>], duration 0ms</div><div class="">request 409, event Event[start=1718636994494, end=1718636994650, description=close span handleRequest, spanId=825, parentSpanId=-1], duration 156ms</div><div class="">request 148, event Event[start=1718636994487, end=1718636994598, description=close span handleRequest, spanId=299, parentSpanId=-1], duration 111ms</div><div class="">request 409, total duration 166ms</div><div class="">request 786, total duration 167ms</div><div class="">request 75, event Event[start=1718636994485, end=1718636994655, description=close span handleRequest, spanId=149, parentSpanId=-1], duration 170ms</div><div class="">request 148, total duration 119ms</div><div class="">request 452, total duration 171ms</div><div class="">request 75, total duration 177ms</div><div class="">request 496, event Event[start=1718636994602, end=1718636994602, description=call jdbc, spanId=3367, parentSpanId=1000], duration 0ms</div><div class="">request 496, event Event[start=1718636994602, end=1718636994602, description=close span making jdbc call, spanId=3367, parentSpanId=1000], duration 0ms</div><div class=""><b class="">request 496</b>, event Event[start=1718636994495, end=1718636994602, description=close span handleRequest, spanId=1000, <b class="">parentSpanId=-1</b>], duration 107ms</div><div class="">request 496, total duration 117ms</div><div class="">request 635, event Event[start=1718636994497, end=1718636994658, description=close span handleRequest, spanId=1301, parentSpanId=-1], duration 161ms</div><div class="">request 308, total duration 176ms</div><div class="">request 576, event Event[start=1718636994658, end=1718636994658, description=close span making jdbc call, spanId=3901, parentSpanId=1163], duration 0ms</div><div class="">request 635, total duration 171ms</div><div class="">request 576, event Event[start=1718636994497, end=1718636994658, description=close span handleRequest, spanId=1163, parentSpanId=-1], duration 161ms</div><div class="">request 267, event Event[start=1718636994658, end=1718636994658, description=call jdbc, spanId=3899, parentSpanId=538], duration 0ms</div><div class=""><br class=""></div><div class=""><br class=""></div><div class=""><blockquote type="cite" class=""><div dir="ltr" class=""><div class="">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.</div></div></blockquote><br class=""></div><div class="">Right, because we don’t want to use thread locals!!!. The code I posted supports nested spans. If you are using a ThreadLocal.CURRENT_TRACER - you only have a single value per thread - so either the spans are chained, or there is a stack.</div><div class=""><br class=""></div><div class="">I guarantee I can implement a full tracing library without using ThreadLocal and have it support nested spans, etc. How can I guarantee this? Because, you can implement tracing by passing “context” as a parameter to every method. If you read the ScopedValue documentation it states that the purpose of ScopedValue is to avoid needing to pass these method parameters. So given that, you can replace callMethod(context,parm1…) with callMethod(parm1,…). A tracing library is trivially implemented with a Context parameter.</div><div class=""><br class=""></div><div class="">There is absolutely no need to add open/close semantics to ScopedValue to support tracing libraries.</div><div class=""><br class=""></div><div class=""><div class=""><br class=""><blockquote type="cite" class=""><div class="">On Jun 17, 2024, at 9:43 AM, Marcin Grzejszczak <<a href="mailto:marcin.grzejszczak@gmail.com" target="_blank" class="">marcin.grzejszczak@gmail.com</a>> wrote:</div><br class=""><div class=""><div dir="ltr" class=""><div class="">> I updated the <a href="http://github.com/robaho/scope_trace" target="_blank" class="">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 class=""><br class=""></div><div class="">Thanks for doing this, I really appreciate it! </div><div class=""><br class=""></div><div class="">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 class=""><br class=""></div><div class="">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 class=""><br class=""></div><div class="">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 class=""></div><div class=""><br class=""></div><div class="">> The problem with open/close semantics is that it is easy to close the scopes out of order - which corrupts the tracing information. <br class=""></div><div class=""><br class=""></div><div class="">Yes, I mentioned that a couple of times already.</div><div class=""><br class=""></div><div class="">> 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" class="">https://github.com/openzipkin/brave/blob/69003dfc811418f0dbc42e9e17ff880ebe1f4b02/instrumentation/mysql/src/main/java/brave/mysql/TracingStatementInterceptor.java#L39</a></div><div class=""><br class=""></div><div class="">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 class=""></div><div class=""><br class=""></div><div class="">> 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 class=""><br class=""></div><div class="">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 class=""></div><div class=""><br class=""></div><div class="">> 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 class=""><br class=""></div><div class="">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 class=""><br class=""></div><div class="">> I am guessing most libraries have these hard-coded dependencies on ThreadLocal.</div><div class=""><br class=""></div><div class="">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 class=""></div><div class=""><br class=""></div><div class="">> 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 class=""></div><div class=""><br class=""></div><div class="">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 class=""></div><div class=""><br class=""></div><div class="">> 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 class=""><br class=""></div><div class="">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 class=""></div><div class=""><br class=""></div><div class=""><div class=""><div dir="ltr" class="gmail_signature"><div dir="ltr" class=""><div class=""><div dir="ltr" class=""><div class="">Pozdrawiam / Best regards,</div><div class="">Marcin Grzejszczak</div><div class=""><br class=""><div class=""><a href="https://marcin.grzejszczak.pl/" target="_blank" class="">https://marcin.grzejszczak.pl</a></div><div class=""><a href="https://toomuchcoding.com/" target="_blank" class="">https://toomuchcoding.com</a></div></div></div></div></div></div></div><br class=""></div></div><br class=""><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" target="_blank" class="">robaho@icloud.com</a>> napisał(a):<br class=""></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex"><div class="">I updated the <a href="http://github.com/robaho/scope_trace" target="_blank" class="">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 class=""><br class=""></div><div class="">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 class=""><br class=""></div><div class="">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" class="">https://github.com/openzipkin/brave/blob/69003dfc811418f0dbc42e9e17ff880ebe1f4b02/instrumentation/mysql/src/main/java/brave/mysql/TracingStatementInterceptor.java#L39</a></div><div class=""><br class=""></div><div class="">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 class=""><br class=""></div><div class="">Notice also </div><div class=""><br class=""></div><div class="">Span span = ThreadLocalSpan.CURRENT_TRACER.next();</div><div class=""><br class=""></div><div class="">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 class="">I don’t see how adding open/close to ScopedValue values would alleviate this.</b></div><div class=""><br class=""></div><div class="">I am guessing most libraries have these hard-coded dependencies on ThreadLocal.</div><div class=""><br class=""></div><div class="">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 class=""><br class=""></div><div class="">“scope values” are not only for tracing scopes.</div><div class=""><br class=""></div><div class=""><br class=""></div><div class=""><div class=""><blockquote type="cite" class=""><div class="">On Jun 17, 2024, at 7:01 AM, robert engels <<a href="mailto:robaho@icloud.com" target="_blank" class="">robaho@icloud.com</a>> wrote:</div><br class=""><div class=""><div dir="auto" class=""><div dir="ltr" class=""></div><div dir="ltr" class="">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" class=""><br class=""></div><div dir="ltr" class="">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" class=""><br class=""></div><div dir="ltr" class="">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" class=""><br class=""><blockquote type="cite" class="">On Jun 17, 2024, at 5:59 AM, Marcin Grzejszczak <<a href="mailto:marcin.grzejszczak@gmail.com" target="_blank" class="">marcin.grzejszczak@gmail.com</a>> wrote:<br class=""><br class=""></blockquote></div><blockquote type="cite" class=""><div dir="ltr" class=""><div dir="ltr" class=""><div class="">> 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 class=""><br class=""></div><div class="">Can you elaborate please?<br class=""></div><div class=""><br class=""></div><div class=""><div dir="ltr" class="gmail_signature"><div dir="ltr" class=""><div class=""><div dir="ltr" class=""><div class="">Pozdrawiam / Best regards,</div><div class="">Marcin Grzejszczak</div><div class=""><br class=""><div class=""><a href="https://marcin.grzejszczak.pl/" target="_blank" class="">https://marcin.grzejszczak.pl</a></div><div class=""><a href="https://toomuchcoding.com/" target="_blank" class="">https://toomuchcoding.com</a></div></div></div></div></div></div></div><br class=""></div><br class=""><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" class="">aph-open@littlepinkcloud.com</a>> napisał(a):<br class=""></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 class="">
> 3. More importantly, if you use VT effectively, you are talking about<br class="">
> orders of magnitude more “trace contexts” (in the default brave handler<br class="">
> they are shared by thread so they are highly limited since the only<br class="">
> effective Java threading implementation prior to VT required pooled<br class="">
> threads) which reinforces my point about the increase in the volume of<br class="">
> unique data is the major problem for existing tracing libraries, which<br class="">
> will probably require architectural changes anyway.<br class="">
<br class="">
Definitely, yes. If you need to create a context for every virtual thread,<br class="">
scoped values versus thread-local variables is the least of your problems.<br class="">
<br class="">
-- <br class="">
Andrew Haley (he/him)<br class="">
Java Platform Lead Engineer<br class="">
Red Hat UK Ltd. <<a href="https://www.redhat.com/" rel="noreferrer" target="_blank" class="">https://www.redhat.com</a>><br class="">
<a href="https://keybase.io/andrewhaley" rel="noreferrer" target="_blank" class="">https://keybase.io/andrewhaley</a><br class="">
EAC8 43EB D3EF DB98 CC77 2FAD A5CD 6035 332F A671<br class="">
<br class="">
</blockquote></div>
</div></blockquote></div></div></blockquote></div><br class=""></div></div></blockquote></div>
</div></blockquote></div><br class=""></div></div></div></div></blockquote></div>
</div></blockquote></div><br class=""></div></div></body></html>