Tsukikage7 commented on issue #3338:
URL: https://github.com/apache/dubbo-go/issues/3338#issuecomment-5114413545

   Thanks for asking. Yes, I think a single upper logical `CLIENT` span is the 
right default model.
   
   The OpenTelemetry RPC semantic conventions describe the client span as 
covering the whole logical RPC, including internal retries: 
https://opentelemetry.io/docs/specs/semconv/rpc/rpc-spans/. In dubbo-go, this 
means the span should start before directory/router/load-balance/retry 
processing and end after the final result.
   
   I suggest the following boundaries:
   
   1. Keep the provider/server filter span unchanged.
   2. Create one consumer logical `CLIENT` span above the cluster invocation.
   3. Do not emit another Dubbo `CLIENT` span for every retry attempt by 
default.
   4. Propagate the logical span context to every actual provider attempt, so 
each reached provider creates its normal `SERVER` span as a child of the 
logical client span.
   5. Record retry, router, load-balance and provider-selection diagnostics as 
bounded events/attributes on the logical span.
   6. Set the logical span's final status from the final RPC outcome. 
Intermediate failed attempts should be events, especially when a later retry 
succeeds.
   
   I agree that this is larger than the current scope of #3551. To keep review 
and rollback manageable, I suggest preserving the span-name/semantic-attribute 
work in #3551 and handling the cluster-level logical span migration as a 
separate implementation slice or, if it must stay in one PR, as clearly 
separated commits.
   
   Before removing the per-attempt client filter, we should preserve the 
current tracing enable/disable configuration and context injection behavior, 
and add tests for:
   
   - one attempt -> one logical client span;
   - multiple retries -> still one logical client span covering the total 
duration;
   - retry failure followed by success;
   - all attempts failed;
   - timeout and cancellation;
   - propagation to every reached provider;
   - direct invocation / special cluster paths;
   - no duplicate client spans;
   - tracing-disabled behavior.
   
   For peer information, the logical span can describe the logical service 
target and the final contacted peer where appropriate; per-attempt provider and 
error details can stay in span events rather than becoming high-cardinality 
metric labels.
   
   So the architecture direction is OK, but I recommend implementing it 
incrementally instead of turning the current naming/attribute change into one 
large rewrite.
   


-- 
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]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to