You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@dubbo.apache.org by "cvictory (via GitHub)" <gi...@apache.org> on 2026/04/29 11:30:52 UTC
[PR] fix(triple): prevent traceparent corruption in triple_invoker [dubbo-go]
cvictory opened a new pull request, #3315:
URL: https://github.com/apache/dubbo-go/pull/3315
## Bug
- **Symptom**: When using `otelClientTrace` filter + Triple protocol, the client-side `traceparent` header gets corrupted, causing downstream spans to reference wrong parents in tracing systems (Jaeger, etc.)
- **Root Cause 1**: `parseAttachments()` copies stale upstream `traceparent` from `ctx.Value(constant.AttachmentKey)` into the invocation, overwriting the correct value injected by the OTEL client filter
- **Root Cause 2**: `mergeAttachmentToOutgoing()` uses `AppendToOutgoingContext()` which mutates a shared `http.Header` pointer. Serial RPCs from the same parent context accumulate `traceparent` values, and `http.Header.Get()` returns the stale first entry
## Fix
1. **`parseAttachments()`**: Skip `traceparent` and `tracestate` keys when copying ctx attachments to invocation, preserving OTEL-injected values
2. **`mergeAttachmentToOutgoing()`**: Reset outgoing headers with `NewOutgoingContext(ctx, http.Header{})` before appending, giving each RPC its own isolated header
## Verification
- 2 new regression tests added:
- `TestParseAttachments_TraceparentNotOverwritten` — verifies OTEL traceparent is preserved, upstream stale value rejected, normal user keys still copied
- `TestMergeAttachmentToOutgoing_HeaderIsolation` — verifies serial RPCs get independent headers with no traceparent accumulation
- All existing tests pass unchanged (`go test ./protocol/triple/ -v -count=1` — PASS, 4.533s)
- Validation ladder: stopping point at full-package test (change is internal, no exported API changes)
- Residual risk: untested with non-W3C propagators (B3, Jaeger); `tracePropagationKeys` may need extension
Closes #3240
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(triple): prevent traceparent corruption in triple_invoker [dubbo-go]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3315:
URL: https://github.com/apache/dubbo-go/pull/3315#discussion_r3161600524
##########
protocol/triple/triple_invoker.go:
##########
@@ -161,6 +162,11 @@ func mergeAttachmentToOutgoing(ctx context.Context, inv base.Invocation) (contex
if timeout, ok := inv.GetAttachment(constant.TimeoutKey); ok {
ctx = context.WithValue(ctx, tri.TimeoutKey{}, timeout)
}
+ // Reset outgoing headers with a fresh empty header before appending.
+ // Without this, AppendToOutgoingContext mutates a shared http.Header pointer
+ // from the parent context, causing traceparent accumulation when the same
+ // parent ctx is reused across serial RPCs (e.g. Service A → B then A → C).
+ ctx = tri.NewOutgoingContext(ctx, http.Header{})
Review Comment:
[P1] 这里用 `tri.NewOutgoingContext(ctx, http.Header{})` 直接重建了整个 outgoing header。`protocol/triple/triple_protocol/header.go` 的实现会替换掉 ctx 里已有的 header，因此调用方原本通过 `tri.NewOutgoingContext`/`AppendToOutgoingContext` 放进去的鉴权头、自定义 metadata 等都会被静默丢掉，只剩 invocation attachments。这个补丁修掉了 traceparent 串写，但同时引入了“其他 header 全丢”的回归。建议先克隆已有 outgoing header，再只覆盖/去重 trace 相关 key。
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(triple): prevent traceparent corruption in triple_invoker [dubbo-go]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks commented on code in PR #3315:
URL: https://github.com/apache/dubbo-go/pull/3315#discussion_r3161590636
##########
protocol/triple/triple_invoker.go:
##########
@@ -161,6 +162,11 @@ func mergeAttachmentToOutgoing(ctx context.Context, inv base.Invocation) (contex
if timeout, ok := inv.GetAttachment(constant.TimeoutKey); ok {
ctx = context.WithValue(ctx, tri.TimeoutKey{}, timeout)
}
+ // Reset outgoing headers with a fresh empty header before appending.
+ // Without this, AppendToOutgoingContext mutates a shared http.Header pointer
+ // from the parent context, causing traceparent accumulation when the same
+ // parent ctx is reused across serial RPCs (e.g. Service A → B then A → C).
+ ctx = tri.NewOutgoingContext(ctx, http.Header{})
Review Comment:
[P1] 这里用 `tri.NewOutgoingContext(ctx, http.Header{})` 直接重建了整个 outgoing header。`protocol/triple/triple_protocol/header.go` 的实现会替换掉 ctx 里已有的 header，因此调用方原本通过 `tri.NewOutgoingContext`/`AppendToOutgoingContext` 放进去的鉴权头、自定义 metadata 等都会被静默丢掉，只剩 invocation attachments。这个补丁修掉了 traceparent 串写，但同时引入了“其他 header 全丢”的回归。建议先克隆已有 outgoing header，再只覆盖/去重 trace 相关 key。
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(triple): prevent traceparent corruption in triple_invoker [dubbo-go]
Posted by "zouyx (via GitHub)" <gi...@apache.org>.
zouyx commented on PR #3315:
URL: https://github.com/apache/dubbo-go/pull/3315#issuecomment-4348536432
@claude review
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(triple): prevent traceparent corruption in triple_invoker [dubbo-go]
Posted by "AlexStocks (via GitHub)" <gi...@apache.org>.
AlexStocks closed pull request #3315: fix(triple): prevent traceparent corruption in triple_invoker
URL: https://github.com/apache/dubbo-go/pull/3315
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(triple): prevent traceparent corruption in triple_invoker [dubbo-go]
Posted by "Alanxtl (via GitHub)" <gi...@apache.org>.
Alanxtl commented on PR #3315:
URL: https://github.com/apache/dubbo-go/pull/3315#issuecomment-4385554198
this issue have been done in #3285
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(triple): prevent traceparent corruption in triple_invoker [dubbo-go]
Posted by "codecov-commenter (via GitHub)" <gi...@apache.org>.
codecov-commenter commented on PR #3315:
URL: https://github.com/apache/dubbo-go/pull/3315#issuecomment-4343319207
## [Codecov](https://app.codecov.io/gh/apache/dubbo-go/pull/3315?dropdown=coverage&src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache) Report
:white_check_mark: All modified and coverable lines are covered by tests.
:white_check_mark: Project coverage is 51.19%. Comparing base ([`ea93b8a`](https://app.codecov.io/gh/apache/dubbo-go/commit/ea93b8a9e1c0a9b8b05e786d955ab4f8aeaa221d?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)) to head ([`01a63b9`](https://app.codecov.io/gh/apache/dubbo-go/commit/01a63b95ae223c6278e0e5b658b7be4cdf3d8f82?dropdown=coverage&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache)).
<details><summary>Additional details and impacted files</summary>
```diff
@@ Coverage Diff @@
## main #3315 +/- ##
==========================================
+ Coverage 50.79% 51.19% +0.40%
==========================================
Files 488 488
Lines 44203 36567 -7636
==========================================
- Hits 22451 18720 -3731
+ Misses 20242 16336 -3906
- Partials 1510 1511 +1
```
</details>
[:umbrella: View full report in Codecov by Sentry](https://app.codecov.io/gh/apache/dubbo-go/pull/3315?dropdown=coverage&src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
:loudspeaker: Have feedback on the report? [Share it here](https://about.codecov.io/codecov-pr-comment-feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=apache).
<details><summary> :rocket: New features to boost your workflow: </summary>
- :snowflake: [Test Analytics](https://docs.codecov.com/docs/test-analytics): Detect flaky tests, report on failures, and find test suite problems.
</details>
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(triple): prevent traceparent corruption in triple_invoker [dubbo-go]
Posted by "sonarqubecloud[bot] (via GitHub)" <gi...@apache.org>.
sonarqubecloud[bot] commented on PR #3315:
URL: https://github.com/apache/dubbo-go/pull/3315#issuecomment-4343282451
## [](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3315) **Quality Gate passed**
Issues
 [0 New issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3315&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0 Accepted issues](https://sonarcloud.io/project/issues?id=apache_dubbo-go&pullRequest=3315&issueStatuses=ACCEPTED)
Measures
 [0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_dubbo-go&pullRequest=3315&issueStatuses=OPEN,CONFIRMED&sinceLeakPeriod=true)
 [0.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3315&metric=new_coverage&view=list)
 [0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_dubbo-go&pullRequest=3315&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarQube Cloud](https://sonarcloud.io/dashboard?id=apache_dubbo-go&pullRequest=3315)
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org
Re: [PR] fix(triple): prevent traceparent corruption in triple_invoker [dubbo-go]
Posted by "Copilot (via GitHub)" <gi...@apache.org>.
Copilot commented on code in PR #3315:
URL: https://github.com/apache/dubbo-go/pull/3315#discussion_r3161173605
##########
protocol/triple/triple_invoker.go:
##########
@@ -161,6 +162,11 @@ func mergeAttachmentToOutgoing(ctx context.Context, inv base.Invocation) (contex
if timeout, ok := inv.GetAttachment(constant.TimeoutKey); ok {
ctx = context.WithValue(ctx, tri.TimeoutKey{}, timeout)
}
+ // Reset outgoing headers with a fresh empty header before appending.
+ // Without this, AppendToOutgoingContext mutates a shared http.Header pointer
+ // from the parent context, causing traceparent accumulation when the same
+ // parent ctx is reused across serial RPCs (e.g. Service A → B then A → C).
+ ctx = tri.NewOutgoingContext(ctx, http.Header{})
Review Comment:
mergeAttachmentToOutgoing unconditionally resets the outgoing metadata to an empty header. This prevents traceparent accumulation, but it also drops any outgoing headers already present on ctx (e.g., set earlier via tri.NewOutgoingContext/tri.AppendToOutgoingContext by user code or other filters). Consider cloning existing outgoing headers into a new map (to avoid shared mutations) and optionally deleting only trace propagation keys before appending invocation attachments, so non-trace metadata is preserved while still isolating per-RPC headers.
```suggestion
// Rebuild outgoing headers from a cloned copy so appends for this RPC do not
// mutate headers stored on a reused parent context. Preserve existing
// non-trace metadata, but drop trace propagation headers to avoid
// traceparent/tracestate accumulation across serial RPCs.
outgoingHeader := http.Header{}
if header, ok := tri.FromOutgoingContext(ctx); ok && header != nil {
outgoingHeader = header.Clone()
outgoingHeader.Del("traceparent")
outgoingHeader.Del("tracestate")
outgoingHeader.Del("Traceparent")
outgoingHeader.Del("Tracestate")
}
ctx = tri.NewOutgoingContext(ctx, outgoingHeader)
```
##########
protocol/triple/triple_invoker_test.go:
##########
@@ -503,3 +503,84 @@ func Test_mergeAttachmentToOutgoing(t *testing.T) {
})
}
}
+
+// TestParseAttachments_TraceparentNotOverwritten verifies that parseAttachments does NOT
+// copy traceparent/tracestate from ctx attachments into the invocation, preserving the
+// values injected by the OTEL filter. (Regression test for issue #3240, Root Cause 1)
+func TestParseAttachments_TraceparentNotOverwritten(t *testing.T) {
+ // Simulate: OTEL filter has already injected the correct traceparent into invocation
+ otelTraceparent := "00-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa-bbbbbbbbbbbbbbbb-01"
+ otelTracestate := "vendor=opaque"
+
+ inv := invocation.NewRPCInvocationWithOptions(
+ invocation.WithAttachment("traceparent", otelTraceparent),
+ invocation.WithAttachment("tracestate", otelTracestate),
+ )
+
+ // Simulate: upstream request ctx carries a DIFFERENT (stale) traceparent
+ upstreamTraceparent := "00-cccccccccccccccccccccccccccccccc-dddddddddddddddd-01"
+ upstreamTracestate := "vendor=stale"
+ userAtta := map[string]any{
+ "traceparent": upstreamTraceparent,
+ "tracestate": upstreamTracestate,
+ "user-key": "user-val", // normal user key should still be copied
+ }
+ ctx := context.WithValue(context.Background(), constant.AttachmentKey, userAtta)
+
+ url := common.NewURLWithOptions()
+ parseAttachments(ctx, url, inv)
+
+ // traceparent and tracestate must remain as the OTEL filter set them
+ tp, _ := inv.GetAttachment("traceparent")
+ assert.Equal(t, otelTraceparent, tp, "traceparent must not be overwritten by ctx attachment")
+
+ ts, _ := inv.GetAttachment("tracestate")
+ assert.Equal(t, otelTracestate, ts, "tracestate must not be overwritten by ctx attachment")
+
+ // normal user key must still be copied
+ uk, _ := inv.GetAttachment("user-key")
+ assert.Equal(t, "user-val", uk, "non-trace user attachment must still be copied")
+}
+
+// TestMergeAttachmentToOutgoing_HeaderIsolation verifies that serial calls to
+// mergeAttachmentToOutgoing from the same parent context produce independent outgoing
+// headers. (Regression test for issue #3240, Root Cause 2)
+func TestMergeAttachmentToOutgoing_HeaderIsolation(t *testing.T) {
+ parentCtx := context.Background()
Review Comment:
TestMergeAttachmentToOutgoing_HeaderIsolation seeds parentCtx with context.Background(), which does not reproduce the original accumulation bug: the old implementation only accumulates when the parent ctx already contains Triple metadata (extraDataKey/headerOutgoingKey) so subsequent calls mutate the same http.Header. To make this a real regression test, initialize parentCtx with an existing outgoing header (e.g., tri.NewOutgoingContext(context.Background(), http.Header{})) before calling mergeAttachmentToOutgoing twice.
```suggestion
parentCtx := tri.NewOutgoingContext(context.Background(), http.Header{})
```
##########
protocol/triple/triple_invoker.go:
##########
@@ -202,13 +208,26 @@ func parseInvocation(ctx context.Context, url *common.URL, invocation base.Invoc
return callType, inRaw, method, nil
}
+// tracePropagationKeys lists header keys managed by trace propagators (e.g. W3C TraceContext).
+// These must not be copied from user-supplied ctx attachments into the invocation because the
+// OTEL filter has already injected the correct values for the current span. Allowing the copy
+// would silently overwrite them with stale upstream values.
+var tracePropagationKeys = map[string]struct{}{
+ "traceparent": {},
+ "tracestate": {},
+}
+
// parseAttachments retrieves attachments from users passed-in and URL, then injects them into ctx
func parseAttachments(ctx context.Context, url *common.URL, invocation base.Invocation) {
// retrieve users passed-in attachment
attaRaw := ctx.Value(constant.AttachmentKey)
if attaRaw != nil {
if userAtta, ok := attaRaw.(map[string]any); ok {
for key, val := range userAtta {
+ // Skip trace-propagation keys to preserve values injected by the OTEL filter.
+ if _, skip := tracePropagationKeys[key]; skip {
+ continue
+ }
Review Comment:
parseAttachments now skips traceparent/tracestate from ctx attachments unconditionally. That preserves OTEL-injected values, but it also prevents propagating trace context at all when the OTEL client filter is disabled (i.e., when invocation doesn't already contain these keys). A safer approach is to skip only when the invocation already has a value for these keys (so ctx can't overwrite), and to match keys case-insensitively (ctx attachments may not always be lowercase).
--
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: notifications-unsubscribe@dubbo.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: notifications-unsubscribe@dubbo.apache.org
For additional commands, e-mail: notifications-help@dubbo.apache.org