AlexStocks commented on code in PR #1017:
URL: https://github.com/apache/dubbo-go-pixiu/pull/1017#discussion_r3829391107


##########
pkg/client/dubbo/dubbo.go:
##########
@@ -311,15 +364,14 @@ func (dc *Client) preparePayload(req 
*DubboOutboundRequest) ([]string, []hessian
                vals[i] = arg
        }
 
-       finalValues, err := json.Marshal(vals)
-       if err != nil {
-               return nil, nil, nil, errors.Wrap(err, "marshal dubbo 
arguments")
-       }
-
-       return types, vals, finalValues, nil
+       return types, vals, nil
 }
 
-func mergeOutboundAttachments(ctx context.Context, outbound map[string]any) 
map[string]any {
+func withAttachments(ctx context.Context, outbound map[string]any) 
context.Context {
+       // fast path: no attachments, no tracing -> reuse the context as-is
+       if !tracingEnabled.Load() && len(outbound) == 0 && 
ctx.Value(constant.AttachmentKey) == nil {

Review Comment:
   [P1] `SpanContext` 仍不足以判断全局 propagator 是否有内容要注入。OTel 的 `TextMapPropagator` 
可以独立传播 baggage;调用方使用 `TraceContext + Baggage` composite propagator、context 里只有 
`tenant=blue` baggage 而没有有效 span 时,这个 fast path 仍直接返回,Dubbo attachment 中不会出现 
`baggage`。Base 会无条件执行 `Inject`,所以同一探针在 Base 得到 `baggage=tenant=blue`,当前 Head 则连 
`AttachmentKey` 都没有;跨服务的租户、路由或关联元数据会因此静默丢失。
   
   请不要用 `SpanContextFromContext` 代替任意全局 propagator 的输出判断;可以先向临时 carrier 注入,再仅在 
carrier 和业务 attachment 都为空时复用原 context。请补充 tracing disabled、无 span、仅 baggage 
的回归测试。



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