xuzhiguang commented on PR #816: URL: https://github.com/apache/skywalking-java/pull/816#issuecomment-5122578022
@wu-sheng Thanks for the detailed review. My plan is to handle only the outermost enhanced method by checking: ``` java objInst.getClass() == method.getDeclaringClass() ``` For a ThreadPoolExecutor subclass, this is true in the subclass-generated override, but false when its super call enters the generated ThreadPoolExecutor override. I will also use a ThreadLocal<Boolean> to record whether the outermost invocation actually created a span. afterMethod and handleMethodException will use this recorded state rather than recomputing from the mutated arguments, and only the invocation that created the span will stop or log it. Regarding the caller-side ThreadPoolExecutor/invokeAll and invokeAny local span, I believe it is useful and intentional. Unlike execute and submit, these methods block the caller: invokeAll waits for all tasks, while invokeAny waits until one task completes successfully (or times out/fails). The local span therefore represents the batch invocation's wall-clock waiting time. It also provides a clear parent for the context snapshots captured for the submitted callables, so the worker-side segments are associated with the specific invokeAll / invokeAny operation rather than directly with the broader caller span. I understand that this adds one caller-side span per invocation. If maintainers prefer this plugin to provide context propagation only, without an invocation span, I can adjust the design accordingly; otherwise I will keep the span with the duplicate-span protection described above. -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
