<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=""><div class="">Did not mean to be short. I’d like to address your other points:</div><div class=""><br class=""></div><div class=""><blockquote type="cite" class=""><div dir="ltr" class=""><div class="">So Virtual Threads with Thread Locals will work as fast as Virtual Threads with Scope Values? I doubt that this is true…</div></div></blockquote><br class=""></div><div class="">If there is any performance gain, it is due to the property of them being immutable and shared - something your api changes are trying to discard - so this is somewhat circular reasoning.</div><div class=""><br class=""></div><blockquote type="cite" class=""><div dir="ltr" class=""><div class="">Yes but the framework might not know about it.</div></div></blockquote><div class=""><br class=""></div><div class="">Then the user level code does. Somewhere changes will need to be made. My reading of the Spring AOP docs tell me it could have SV support without changing a line of user code.</div><div class=""><br class=""></div><div class="">As I’ve mentioned many times, and others have concurred, VirtualThreads are not designed to “just turn them on and watch the performance improvement!” There is still architectural work that needs to be done to use them effectively. (e.g. in low concurrency they perform worse than platform threads).</div><div class=""><br class=""></div><div class=""><blockquote type="cite" class=""><div dir="ltr" class=""><div class="">In Micrometer we already have such issue...</div></div></blockquote><br class=""></div><div class="">That doesn’t answer the question of what does “our customers will ask us if we support SV” means. Does supporting SV means all tracing related context is in a SV? Does it mean that a library using SV performs better than one using TL.</div><div class=""><br class=""></div><div class=""><blockquote type="cite" class=""><div dir="ltr" class=""><div class="">believe me that we are not using proxies for observability in Spring</div></div></blockquote><br class=""></div><div class="">Maybe we are talking about different things, but the current docs for Spring AOP states they specifically use proxies and why: <a href="https://docs.spring.io/spring-framework/reference/core/aop/introduction-proxies.html" class="">https://docs.spring.io/spring-framework/reference/core/aop/introduction-proxies.html</a></div><div class=""><br class=""></div><div class="">Which makes perfect sense to me. Whenever you have existing layering (servlets, remote interfaces, etc.) you can put the AOP integration there - no need to inject. Same with libraries that support before/after callbacks - you just need to access the context to link the calls. Rather than setting this in a TL, you can just as easily use thread id + method name, etc to create the key to link the calls. There are requirements about call order scoping anyway - or you will have leaks.</div><div class=""><br class=""></div><div class=""><blockquote type="cite" class=""><div dir="ltr" class=""><div class="">it's not possible without doing massive changes in the API</div></div></blockquote><br class=""></div><div class="">The entire brave codebase is only 27k lines and 181 files. I suspect the entire change set would be much less than 2k lines of code to have a true SV based distributed tracing library. This is not a substantial amount of work to do things properly.</div><div class=""><br class=""></div><div class="">Looking through the ‘instrumentation’ directory of nearly 50k LOC - all(?) calls defer to the library - so I would expect zero changes there.</div><div class=""><br class=""></div><div class="">Anyway, you are welcome to continue to hang your hat on an SV api change, but to me it would be a bad change outside of the purpose of SV, so my guess it will be a difficult effort to convince others smarter than me to do so.</div><div class=""><br class=""></div><div><br class=""><blockquote type="cite" class=""><div class="">On Jun 20, 2024, at 7:42 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="">> I don’t think this is the case, as long as you are not leaking the TL, the performance overhead vs SV will be negligible for a tracing library. It is one extra indirection (I think).</div><div class=""><br class=""></div><div class="">So Virtual Threads with Thread Locals will work as fast as Virtual Threads with Scope Values? I doubt that this is true especially after reading the JEP: "Introduce scoped values, which enable a method to share immutable data both with its callees within a thread, and with child threads. Scoped values are easier to reason about than thread-local variables. They also have lower space and time costs, especially when used together with virtual threads (JEP 444) and structured concurrency (JEP 480). This is a preview API."</div><div class=""><br class="">> It is not the only purpose, but I think it is the primary one. Also, by making them non-writable / i.e. not shared up, they are safer - no one down the stack can change the value a caller sees.<br class=""></div><div class=""><br class=""></div><div class="">I still would like to get the answer from the JDK maintainers if my arguments (which I think I presented a lot) would allow it to consider opening the API to the idea of giving the library maintainers more control. I doubt it that the final users (people writing actual business code) would ever use the scoping API.</div><div class=""><br class=""></div><div class="">> “somewhere” is is occurring. It must. Something needs to assign/create the “request id” is in order to have the trace. Wherever that is, must change, or maybe not - if you are using a framework it probably only changes in the framework.</div><div class=""><br class=""></div><div class="">Yes but the framework might not know about it. Library maintainers know that they will be instrumenting libraries, they don't always know if they are at the beginning of the call stack or not.</div><div class=""><br class=""></div><div class="">> You still have never fully stated what “our customers will ask us if we support SV” means. This thread has a lot of conversation, and I don’t think you’ve answered this. It can mean several different things.</div><div class=""><br class=""></div><div class="">In Micrometer we already have such issue [1] and I'm pretty sure in Spring there are similar ones.</div><div class=""><br class=""></div><div class="">> Even for a framework like Spring to “support virtual threads” - work needs to be done. You can’t just “turn it on”. Many frameworks use async IO which should be removed, or thread pooling which should be removed. Configuring a tracing library designed for virtual threads is just another part of this support of VT.</div><div class=""><br class=""></div><div class="">It's not about configuring it. Tracing libraries would need to be completely rewritten. That's what I'm trying to say from the very beginning.</div><div class=""><br class=""></div><div class="">> Look at the tracing support for Spring AOP. It is a different story. See <a href="https://docs.spring.io/spring-framework/docs/1.2.3/reference/aop.html" class="">https://docs.spring.io/spring-framework/docs/1.2.3/reference/aop.html</a> and section 5.1.3. Proxies are certainly used for Spring AOP (at least at the time of that writing but I see no reason why they would change it).</div><div class=""><br class=""></div><div class="">Like I said it the last time. I've co-authored / reviewed instrumentation of Spring components with Micrometer Observation so I do know what I'm talking about. You're pointing to a Spring Framework version 1.2.3 and we have 6.x now so you're pointing to an outdated doc. Also please believe me that we are not using proxies for observability in Spring. I know that because like I say again, I co-authored / reviewed most of these instrumentations.</div><div class=""><br class=""></div><div class="">> If you review the code, the TracingContext uses a ConcurrentLinkedQueue for the events for this exact reason. I would not shared the Span between threads, and if you are using the TaskScope it will create a new child context linked to the parent context.</div><div class=""><br class=""></div><div class="">I understand. <br class=""></div><div class=""><br class=""></div><div class="">> What I provided is a POC, but it is seems to me with some clean-up it would work fine in an SV native tracing library. Even SV uses a small TL behind the scenes - a local TL in the TraceContext may be necessary but you would get the automatic clean-up.</div><div class=""><br class=""></div><div class="">I understand that you think this would be the case. I actually started implementing this in Micrometer Observation and still there's Micrometer Tracing and OpenTelemetry and Brave. If you think this is feasible you can look at Brave and try to prototype the solution. I will look into doing that too but IMO it's not possible without doing massive changes in the API.<br class=""></div><div class=""><br class=""></div><div class="">[1] - <a href="https://github.com/micrometer-metrics/context-propagation/issues/108" class="">https://github.com/micrometer-metrics/context-propagation/issues/108</a></div><div class=""><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">śr., 19 cze 2024 o 20:03 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=""><blockquote type="cite" class="">Now, correct me if I'm wrong, but in order to maintain that performance gain behind the scenes Scoped Values would be used to retrieve the information about e.g. the current span (for tracing).</blockquote><div class=""><br class=""></div>I don’t think this is the case, as long as you are not leaking the TL, the performance overhead vs SV will be negligible for a tracing library. It is one extra indirection (I think).<div class=""><br class=""><div class=""><blockquote type="cite" class=""><div dir="ltr" class=""><div dir="ltr" class=""><div class="">If the only purpose of Scoped Values is to allow a value to be there for the life of a runnable / callable then indeed this whole discussion is pointless.</div></div></div></blockquote><div class=""><br class=""></div>It is not the only purpose, but I think it is the primary one. Also, by making them non-writable / i.e. not shared up, they are safer - no one down the stack can change the value a caller sees.<br class=""><div class=""><br class=""></div><div class=""><blockquote type="cite" class=""><div dir="ltr" class=""><div dir="ltr" class=""><div class="">so I would just configure my framework (e.g. Spring Boot) to use Virtual Threads. </div></div></div></blockquote><br class=""></div><div class=""><blockquote type="cite" class=""><div dir="ltr" class=""><div dir="ltr" class=""><div class="">You assume that you know what the entry port to your application is (a "request" comes in). You can't assume that in a production application. You might have an MVC app but maybe you have a command line application. We would have to try to guess all those entry points so that we wrap these calls in `where(...)`</div></div></div></blockquote><div class=""><br class=""></div><div class="">“somewhere” is is occurring. It must. Something needs to assign/create the “request id” is in order to have the trace. Wherever that is, must change, or maybe not - if you are using a framework it probably only changes in the framework.</div><div class=""><br class=""></div><div class="">You still have never fully stated what “our customers will ask us if we support SV” means. This thread has a lot of conversation, and I don’t think you’ve answered this. It can mean several different things.</div><div class=""><br class=""></div><div class="">Even for a framework like Spring to “support virtual threads” - work needs to be done. You can’t just “turn it on”. Many frameworks use async IO which should be removed, or thread pooling which should be removed. Configuring a tracing library designed for virtual threads is just another part of this support of VT.</div><div class=""><br class=""></div><div class=""><blockquote type="cite" class=""><div dir="ltr" class=""><div dir="ltr" class=""><div class="">No proxies are being involved.</div></div></div></blockquote><br class=""></div><div class="">Of course not in this case, you are referring to HTTP requests through servlets/filters - there is no need to use proxies.</div><div class=""><br class=""></div><div class="">Look at the tracing support for Spring AOP. It is a different story. See <a href="https://docs.spring.io/spring-framework/docs/1.2.3/reference/aop.html" target="_blank" class="">https://docs.spring.io/spring-framework/docs/1.2.3/reference/aop.html</a> and section 5.1.3. Proxies are certainly used for Spring AOP (at least at the time of that writing but I see no reason why they would change it).</div><div class=""><br class=""></div><div class=""><blockquote type="cite" class=""><div dir="ltr" class=""><div dir="ltr" class=""><div class="">The span from TraceContext can be shared between threads. How will you guard that span against concurrent writes?</div></div></div></blockquote><br class=""></div><div class="">If you review the code, the TracingContext uses a ConcurrentLinkedQueue for the events for this exact reason. I would not shared the Span between threads, and if you are using the TaskScope it will create a new child context linked to the parent context.</div><div class=""><br class=""></div><div class="">What I provided is a POC, but it is seems to me with some clean-up it would work fine in an SV native tracing library. Even SV uses a small TL behind the scenes - a local TL in the TraceContext may be necessary but you would get the automatic clean-up.</div><div class=""><br class=""></div><div class=""><div class=""><div class=""><br class=""><blockquote type="cite" class=""><div class="">On Jun 19, 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 dir="ltr" class=""><div 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><div class=""><br class=""></div><div class="">I'm really happy about that. We're talking about a complicated problem and I'm sure we'll reach some compromise whatever that will be ;)<br class=""></div><br class=""><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. Your requested changes eliminate a (the?) primary purpose of the api change! So, if you don’t want this, continue with ThreadLocal.</div><div class=""><br class=""></div><div class="">That's a very interesting point. </div><div class=""><br class=""></div><div class="">As a developer I shouldn't care whether ThreadLocal or ScopedVariable is being used. Let's say that I have an MVC application and would like to have better performance so I would just configure my framework (e.g. Spring Boot) to use Virtual Threads. Hopefully we are in agreement here that this is the intended use case of Virtual Threads - improve performance. Now, correct me if I'm wrong, but in order to maintain that performance gain behind the scenes Scoped Values would be used to retrieve the information about e.g. the current span (for tracing). As a user I don't care what is happening under the hood. From the point of view of the library creator I very much do.</div><div class=""><br class=""></div><div class="">If the only purpose of Scoped Values is to allow a value to be there for the life of a runnable / callable then indeed this whole discussion is pointless. However we are already asked by our users whether we will support Scoped Values and I'm pretty sure that their assumption is that it will work in a transparent way (i.e. again they won't care about whether TL or SV are used under the hood - they want performance gains).</div><div class=""><br class=""></div><div class="">Assumption that everyone has to do some migration to use this feature is acceptable, but still if only the scope would be returned by this API, no changes would be required. I think we constantly go in circles with this and I still do not understand why is it such a big problem (I understood the topic of security, but I'm personally not convinced that this is such a big deal; it's similar to a case if someone didn't close a stream or something like that. If they don't they will have problems too)<br class=""></div><br class=""><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 “rewrite all of our code” without providing any evidence of why that would be the case. <br class=""></div><div class=""><br class=""></div><div class="">For Micrometer Observation I need to introduce new API that instead of allowing to create a Scope and close it I need to create new API that works more or less like FilterChain and Filters for HTTP (we have an analogous approach of ObservationHandlers that would need to work with HandlerChain). In the Micrometer repo only I need to do 60+ changes of the API usage. Of course I can't break compatibility so I need to leave the old API. Plus I need to add new ObservationHandler methods and not break compatibility. That's only for Micrometer Observation. I will need to have similar changes in Micrometer Tracing plus Brave and OpenTelemetry. Only after all of these projects are changed will we have to go to all the projects that actually use us to change their usage to the new one. </div><div class=""><br class=""></div><div class="">I don't have to mention that for all of these changes we need to ensure that compatibility will not be broken and that things will work with and without Scoped Values. There also must be a TL and SV interop because not everyone will use SV only. For me it sounds like a lot of work.<br class=""></div><br class=""><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="">I'm sorry but in the vast majority of cases it was me doing or co-authoring the Spring instrumentation (plus dozens of other projects) and what you wrote is not entirely correct. Micrometer Observation API is being used to do the instrumentation. You can check an example here [1]. No proxies are being involved.<br class=""></div><br class=""><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="">So do I understand correctly that you should either use TL or SV ? No library will migrate directly to use only this or that. There must be some bridging or an option to use gradual migration.<br class=""></div><br class="">> I updated the my code to demonstrate the fallback to ThreadLocal when the top-level ScopedValue context is not available.<br class=""><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 looked at your sample and have a couple of comments:</div><div class="">- You assume that you know what the entry port to your application is (a "request" comes in). You can't assume that in a production application. You might have an MVC app but maybe you have a command line application. We would have to try to guess all those entry points so that we wrap these calls in `where(...)`</div><div class="">- Your approach is equivalent to creation of a TL entry that is mutable and all the scopes are put there inside that entry. I'm not saying this is wrong but tracing libraries do not work like this. Maybe they need to be rewritten completely to work like that? I will need to think about this but this would require major rewrite of Micrometer Tracing, Brave and Opentelemetry which I highly doubt would happen.</div><div class="">- I understand that the SV will contain the TraceContext. The span from TraceContext can be shared between threads. How will you guard that span against concurrent writes?<br class=""></div><br class=""><div class="">> I still think you are stuck and looking at the problem the wrong way.</div><div class=""><br class=""></div><div class="">That is possible. The problem I have is that as I see it when SVs become GA, users will ask us, library creators to have tracing work with SV and it won't work out of the box unless we tell them to use TL. That might be the proper answer but I'm trying to find a solution that would be beneficial for the community.<br class=""></div><div class=""><br class=""></div><div class="">[1] - <a href="https://github.com/spring-projects/spring-framework/blob/v6.1.9/spring-web/src/main/java/org/springframework/web/filter/ServerHttpObservationFilter.java#L102-L128" target="_blank" class="">https://github.com/spring-projects/spring-framework/blob/v6.1.9/spring-web/src/main/java/org/springframework/web/filter/ServerHttpObservationFilter.java#L102-L128</a></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">pon., 17 cze 2024 o 16:08 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="">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 class=""><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" target="_blank" class="">marcin.grzejszczak@gmail.com</a>> wrote:</div><br class=""><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"><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" 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="">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></div></blockquote></div>
</div>
</div></blockquote></div><br class=""></div></div></div></div></div></div></blockquote></div>
</div></blockquote></div><br class=""></body></html>