feat(gRPC): add support for http header propagation to gRPC sources #2450
Conversation
maxbol
requested review from
Noroth,
StarpTech,
devsergiy,
endigma and
jensneuse
as code owners
WalkthroughAdds HTTP header forwarding to gRPC subgraph calls: a client interceptor forwards selected HTTP headers (and OTEL trace context) as gRPC metadata, provider config now accepts headers-to-forward, and runtime injects incoming HTTP headers into the context for propagation. Changes
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~25 minutes 🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
router/pkg/grpcconnector/grpcremote/grpc_remote.go (1)
69-83: Potential race condition:Start()readsg.ccwithout holding the mutex.The
Start()method checksg.cc == nilon line 70 without acquiring the lock, whileGetClient()usesRLock. IfStart()is called concurrently (e.g., during retry or reconnection scenarios), this could lead to a data race or double initialization.🔒 Proposed fix to add mutex protection
func (g *RemoteGRPCProvider) Start(ctx context.Context) error { + g.mu.Lock() + defer g.mu.Unlock() + if g.cc == nil { // Create gRPC client with header forwarding interceptor opts := []grpc.DialOption{ grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithUnaryInterceptor(HeaderForwardingInterceptor(g.headersToForward)), } clientConn, err := grpc.NewClient(g.endpoint, opts...) if err != nil { return fmt.Errorf("failed to create client connection: %w", err) } g.cc = clientConn } return nil }
🤖 Fix all issues with AI agents
In @router/pkg/grpcconnector/grpcremote/header_interceptor.go:
- Around line 61-65: The loop that appends HTTP headers to gRPC metadata uses
headerName directly (see headersToForward, httpHeaders.Values and md.Append) but
gRPC metadata keys must be lowercase; update the code to lowercase the header
name (e.g., via strings.ToLower) before calling md.Append so keys like
"X-Request-ID" become "x-request-id" when added to metadata.
🧹 Nitpick comments (1)
router/pkg/grpcconnector/grpcremote/header_interceptor.go (1)
53-70: Consider preserving existing outgoing metadata from the context.The current implementation creates a fresh
metadata.MDand overwrites any existing outgoing metadata in the context. If upstream code has already set metadata (e.g., for authentication), it would be lost.♻️ Proposed fix to merge with existing metadata
- md := make(metadata.MD) + // Get existing outgoing metadata or create new + md, ok := metadata.FromOutgoingContext(ctx) + if !ok { + md = make(metadata.MD) + } else { + md = md.Copy() // Don't mutate the original + } // Inject OTEL trace context otel.GetTextMapPropagator().Inject(ctx, metadataCarrier{md})
📜 Review details
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
router/core/engine_loader_hooks.gorouter/core/graph_server.gorouter/pkg/grpcconnector/grpccommon/grpc_plugin_client.gorouter/pkg/grpcconnector/grpcremote/grpc_remote.gorouter/pkg/grpcconnector/grpcremote/header_interceptor.go
🧰 Additional context used
🧠 Learnings (10)
📓 Common learnings
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2172
File: router/core/graph_server.go:0-0
Timestamp: 2025-09-17T20:55:39.456Z
Learning: The Initialize method in router/internal/retrytransport/manager.go has been updated to properly handle feature-flag-only subgraphs by collecting subgraphs from both routerConfig.GetSubgraphs() and routerConfig.FeatureFlagConfigs.ConfigByFeatureFlagName, ensuring all subgraphs receive retry configuration.
📚 Learning: 2025-09-17T20:55:39.456Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2172
File: router/core/graph_server.go:0-0
Timestamp: 2025-09-17T20:55:39.456Z
Learning: The Initialize method in router/internal/retrytransport/manager.go has been updated to properly handle feature-flag-only subgraphs by collecting subgraphs from both routerConfig.GetSubgraphs() and routerConfig.FeatureFlagConfigs.ConfigByFeatureFlagName, ensuring all subgraphs receive retry configuration.
Applied to files:
router/core/graph_server.go
📚 Learning: 2025-08-20T10:08:17.857Z
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2155
File: router/core/router.go:1857-1866
Timestamp: 2025-08-20T10:08:17.857Z
Learning: router/pkg/config/config.schema.json forbids null values for traffic_shaping.subgraphs: additionalProperties references $defs.traffic_shaping_subgraph_request_rule with type "object". Therefore, in core.NewSubgraphTransportOptions, dereferencing each subgraph rule pointer is safe under schema-validated configs, and a nil-check is unnecessary.
Applied to files:
router/core/graph_server.go
📚 Learning: 2025-08-20T10:08:17.857Z
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2155
File: router/core/router.go:1857-1866
Timestamp: 2025-08-20T10:08:17.857Z
Learning: In the Cosmo router codebase, JSON schema validation prevents null values in TrafficShapingRules subgraph configurations, making nil checks unnecessary when dereferencing subgraph rule pointers in NewSubgraphTransportOptions.
Applied to files:
router/core/graph_server.go
📚 Learning: 2026-01-06T12:37:21.521Z
Learnt from: asoorm
Repo: wundergraph/cosmo PR: 2379
File: router/pkg/connectrpc/operation_registry_test.go:381-399
Timestamp: 2026-01-06T12:37:21.521Z
Learning: In Go code (Go 1.25+), prefer using sync.WaitGroup.Go(func()) to run a function in a new goroutine, letting the WaitGroup manage Add/Done automatically. Avoid manual wg.Add(1) followed by go func() { defer wg.Done(); ... }() patterns. Apply this guidance across all Go files in the wundergraph/cosmo repository where concurrency is used.
Applied to files:
router/core/graph_server.gorouter/pkg/grpcconnector/grpcremote/grpc_remote.gorouter/pkg/grpcconnector/grpccommon/grpc_plugin_client.gorouter/core/engine_loader_hooks.gorouter/pkg/grpcconnector/grpcremote/header_interceptor.go
📚 Learning: 2025-08-20T21:05:58.731Z
Learnt from: SkArchon
Repo: wundergraph/cosmo PR: 2158
File: router/pkg/grpcconnector/grpccommon/grpc_plugin_client.go:149-153
Timestamp: 2025-08-20T21:05:58.731Z
Learning: In the Cosmo codebase, the GRPCPluginClient struct fields `tracer` and `getTraceAttributes` are always properly initialized during construction and will never be nil, so defensive nil checks are not needed.
Applied to files:
router/pkg/grpcconnector/grpccommon/grpc_plugin_client.go
📚 Learning: 2025-11-24T13:44:40.478Z
Learnt from: endigma
Repo: wundergraph/cosmo PR: 2334
File: router/cmd/flightrecorder/module/module.go:83-97
Timestamp: 2025-11-24T13:44:40.478Z
Learning: In the FlightRecorder module (router/cmd/flightrecorder/module/module.go), concurrent WriteTo() errors are acceptable and intentional. The module doesn't need multiple traces for events happening close together, so no synchronization is needed for concurrent writes.
Applied to files:
router/pkg/grpcconnector/grpccommon/grpc_plugin_client.go
📚 Learning: 2025-11-19T15:13:57.821Z
Learnt from: dkorittki
Repo: wundergraph/cosmo PR: 2273
File: router/core/graphql_handler.go:0-0
Timestamp: 2025-11-19T15:13:57.821Z
Learning: In the Cosmo router (wundergraph/cosmo), error handling follows a two-phase pattern: (1) Prehandler phase handles request parsing, validation, and setup errors using `httpGraphqlError` and `writeOperationError` (in files like graphql_prehandler.go, operation_processor.go, parse_multipart.go, batch.go); (2) Execution phase handles resolver execution errors using `WriteError` in GraphQLHandler.ServeHTTP. Because all `httpGraphqlError` instances are caught in the prehandler before ServeHTTP is invoked, any error type checks for `httpGraphqlError` in the execution-phase WriteError method are unreachable code.
Applied to files:
router/core/engine_loader_hooks.go
📚 Learning: 2025-08-12T13:50:45.964Z
Learnt from: Noroth
Repo: wundergraph/cosmo PR: 2132
File: router-plugin/plugin.go:139-146
Timestamp: 2025-08-12T13:50:45.964Z
Learning: In the Cosmo router plugin system, the plugin framework creates its own logger independently. When creating a logger in NewRouterPlugin, it's only needed for the gRPC server setup (passed via setup.GrpcServerInitOpts.Logger) and doesn't need to be assigned back to serveConfig.Logger.
Applied to files:
router/core/engine_loader_hooks.go
📚 Learning: 2025-08-20T22:13:25.222Z
Learnt from: StarpTech
Repo: wundergraph/cosmo PR: 2157
File: router-tests/go.mod:16-16
Timestamp: 2025-08-20T22:13:25.222Z
Learning: github.com/mark3labs/mcp-go v0.38.0 has regressions and should not be used in the wundergraph/cosmo project. v0.36.0 is the stable version that should be used across router-tests and other modules.
Applied to files:
router/core/engine_loader_hooks.go
🧬 Code graph analysis (2)
router/pkg/grpcconnector/grpcremote/grpc_remote.go (1)
router/pkg/grpcconnector/grpcremote/header_interceptor.go (1)
HeaderForwardingInterceptor(44-74)
router/core/engine_loader_hooks.go (1)
router/pkg/grpcconnector/grpcremote/header_interceptor.go (1)
WithHTTPHeaders(77-79)
🔇 Additional comments (6)
router/pkg/grpcconnector/grpccommon/grpc_plugin_client.go (1)
8-9: LGTM: Import reordering only.This change only reorders the import statement with no functional impact. The
tracepackage was already in use and remains unchanged.router/core/engine_loader_hooks.go (1)
107-111: LGTM! HTTP headers injection for gRPC forwarding is correctly implemented.The nil checks on
reqContext.requestandreqContext.request.Headerare appropriate defensive coding. The placement before the tracer span ensures headers are available in context for downstream gRPC calls.Note: All HTTP headers are stored in the context unconditionally here; the filtering of which headers to actually forward happens in the
HeaderForwardingInterceptorbased on the configuredheadersToForwardlist.router/pkg/grpcconnector/grpcremote/header_interceptor.go (2)
12-35: LGTM!metadataCarriercorrectly implements theTextMapCarrierinterface.The adapter properly bridges gRPC metadata with OpenTelemetry's text map propagation.
76-79: LGTM! Simple and correct context value helper.router/pkg/grpcconnector/grpcremote/grpc_remote.go (1)
74-74: Streaming is explicitly not supported in this gRPC connector implementation. TheNewStream()method returns an error with the message "streaming is currently not supported", and theHeaderForwardingInterceptoris designed as aUnaryClientInterceptorfor unary calls only. The configuration is correct and intentional for the supported call types.Likely an incorrect or invalid review comment.
router/core/graph_server.go (1)
1579-1598: LGTM! Header forwarding configuration for remote gRPC providers.The graceful degradation approach (logging a warning and continuing without headers on error) is appropriate and prevents header rule parsing issues from breaking gRPC connectivity.
FetchURLRulesworks correctly with gRPC endpoint URLs since it uses simple string matching (RoutingUrl == routingURL) rather than URL scheme parsing, making it scheme-agnostic. The ignored second return value fromPropagatedHeaders(regex patterns for header matching) is intentional—only the header names are needed for gRPC header forwarding.
| for _, headerName := range headersToForward { | ||
| if values := httpHeaders.Values(headerName); len(values) > 0 { | ||
| // gRPC metadata keys are lowercase | ||
| md.Append(headerName, values...) | ||
| } |
There was a problem hiding this comment.
Header names should be lowercased before appending to gRPC metadata.
The comment on line 63 correctly notes that "gRPC metadata keys are lowercase", but headerName is used directly without conversion. HTTP header names are case-insensitive and may arrive in mixed case (e.g., X-Request-ID), which could cause mismatches when the gRPC server looks up headers using lowercase keys.
🔧 Proposed fix to lowercase header names
+import "strings"
+
// Forward configured headers from HTTP headers to gRPC metadata
for _, headerName := range headersToForward {
if values := httpHeaders.Values(headerName); len(values) > 0 {
// gRPC metadata keys are lowercase
- md.Append(headerName, values...)
+ md.Append(strings.ToLower(headerName), values...)
}
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| for _, headerName := range headersToForward { | |
| if values := httpHeaders.Values(headerName); len(values) > 0 { | |
| // gRPC metadata keys are lowercase | |
| md.Append(headerName, values...) | |
| } | |
| for _, headerName := range headersToForward { | |
| if values := httpHeaders.Values(headerName); len(values) > 0 { | |
| // gRPC metadata keys are lowercase | |
| md.Append(strings.ToLower(headerName), values...) | |
| } |
🤖 Prompt for AI Agents
In @router/pkg/grpcconnector/grpcremote/header_interceptor.go around lines 61 -
65, The loop that appends HTTP headers to gRPC metadata uses headerName directly
(see headersToForward, httpHeaders.Values and md.Append) but gRPC metadata keys
must be lowercase; update the code to lowercase the header name (e.g., via
strings.ToLower) before calling md.Append so keys like "X-Request-ID" become
"x-request-id" when added to metadata.
maxbol
force-pushed
the
feature/grpc-header-forwarding
branch
from
21e0360 to
151dec1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
router/pkg/grpcconnector/grpcremote/grpc_remote.go (1)
69-86: Add mutex protection toStart()method to matchGetClient()andStop()locking pattern.The method has a race condition: it reads and writes
g.ccwithout holding the mutex whileGetClient()andStop()both useg.mu. Concurrent calls toStart()(or concurrent calls withGetClient()/Stop()) create a check-then-set race where multiple goroutines could readg.cc == nilsimultaneously and each create their own connection.Suggested fix
func (g *RemoteGRPCProvider) Start(ctx context.Context) error { + g.mu.Lock() + defer g.mu.Unlock() + if g.cc == nil { // Create gRPC client with header forwarding interceptor opts := []grpc.DialOption{ grpc.WithTransportCredentials(insecure.NewCredentials()), grpc.WithUnaryInterceptor(HeaderForwardingInterceptor(g.headersToForward)), } clientConn, err := grpc.NewClient(g.endpoint, opts...) if err != nil { return fmt.Errorf("failed to create client connection: %w", err) } g.cc = clientConn } return nil }
🤖 Fix all issues with AI agents
In `@router/core/graph_server.go`:
- Around line 1579-1598: The PropagatedHeaders result currently discards
regex-based patterns when building headersToForward for gRPC subgraphs; update
the block around PropagatedHeaders/FetchURLRules (used when s.headerRules !=
nil) and the grpcremote.NewRemoteGRPCProvider call to include a brief comment
that regex-based header propagation is intentionally not supported for gRPC
subgraphs because the HeaderForwardingInterceptor only accepts exact header
names (HeadersToForward) and lacks runtime regex evaluation, while HTTP
subscriptions support both explicit names and regex rules—mention the deliberate
limitation and reference PropagatedHeaders, HeaderForwardingInterceptor, and
HeadersToForward so future maintainers understand why regex patterns are
ignored.
♻️ Duplicate comments (1)
router/pkg/grpcconnector/grpcremote/header_interceptor.go (1)
61-65: Header names should be lowercased before appending to gRPC metadata.The comment on line 63 correctly notes that "gRPC metadata keys are lowercase", but
headerNameis used directly without conversion. HTTP header names are case-insensitive and may arrive in mixed case (e.g.,X-Request-ID), causing mismatches when the gRPC server looks up headers using lowercase keys.🔧 Proposed fix to lowercase header names
+import "strings" + // Forward configured headers from HTTP headers to gRPC metadata for _, headerName := range headersToForward { if values := httpHeaders.Values(headerName); len(values) > 0 { // gRPC metadata keys are lowercase - md.Append(headerName, values...) + md.Append(strings.ToLower(headerName), values...) } }
| // Extract header forwarding configuration for this subgraph | ||
| var headersToForward []string | ||
| if s.headerRules != nil { | ||
| dataSourceRules := FetchURLRules(s.headerRules, configSubgraphs, sg.RoutingUrl) | ||
| forwardedHeaders, _, err := PropagatedHeaders(dataSourceRules) | ||
| if err != nil { | ||
| s.logger.Warn("error parsing header rules for gRPC subgraph, headers will not be forwarded", | ||
| zap.String("subgraph", sg.Name), | ||
| zap.Error(err), | ||
| ) | ||
| } else { | ||
| headersToForward = forwardedHeaders | ||
| } | ||
| } | ||
|
|
||
| remoteProvider, err := grpcremote.NewRemoteGRPCProvider(grpcremote.RemoteGRPCProviderConfig{ | ||
| Logger: s.logger, | ||
| Endpoint: sg.RoutingUrl, | ||
| Logger: s.logger, | ||
| Endpoint: sg.RoutingUrl, | ||
| HeadersToForward: headersToForward, | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
# Find the PropagatedHeaders function definition and understand what it does
rg -n "func PropagatedHeaders" --type go -A 30Repository: wundergraph/cosmo
Length of output: 2613
🏁 Script executed:
# Search for how headerNameRegexps is used throughout the codebase
rg -n "headerNameRegexps" --type go -B 2 -A 5Repository: wundergraph/cosmo
Length of output: 1824
🏁 Script executed:
# Check if headerNameRegexps is used in any gRPC-related code
rg -n "gRPC|grpc" --type go -A 5 | grep -A 5 -B 5 "headerNameRegexps" || echo "No gRPC + headerNameRegexps found together"Repository: wundergraph/cosmo
Length of output: 103
🏁 Script executed:
# Look at the header rules configuration in the schema
cat router/pkg/config/config.schema.json | grep -A 20 '"header' | head -50Repository: wundergraph/cosmo
Length of output: 2285
🏁 Script executed:
# Find all calls to PropagatedHeaders
rg -n "PropagatedHeaders\(" --type go -B 2 -A 5Repository: wundergraph/cosmo
Length of output: 2249
🏁 Script executed:
# Search for usage of headerNameRegexps in graphql_datasource or other files
rg -n "headerNameRegexps" --type go --type tsRepository: wundergraph/cosmo
Length of output: 458
🏁 Script executed:
# Check how regex headers are handled in HTTP datasources
rg -n "RegularExpression" --type go -B 3 -A 3Repository: wundergraph/cosmo
Length of output: 1945
🏁 Script executed:
# Check the RemoteGRPCProviderConfig struct definition
rg -n "type RemoteGRPCProviderConfig" --type go -A 15Repository: wundergraph/cosmo
Length of output: 1673
🏁 Script executed:
# Look for grpcremote package to understand gRPC provider capabilities
fd -t f "grpcremote" --type go | head -5Repository: wundergraph/cosmo
Length of output: 230
🏁 Script executed:
# Compare with HTTP datasource to see how it uses regex headers
rg -n "ForwardedClientHeaderRegularExpressions" --type go -B 5 -A 5Repository: wundergraph/cosmo
Length of output: 1091
🏁 Script executed:
# Look for gRPC interceptor implementations that might handle headers
rg -n "interceptor|Interceptor" router/pkg/grpcconnector/ --type go -B 2 -A 5Repository: wundergraph/cosmo
Length of output: 2895
🏁 Script executed:
# Check if headers are processed anywhere in the gRPC remote provider
rg -n "headersToForward|HeadersToForward" --type go -B 3 -A 10Repository: wundergraph/cosmo
Length of output: 9457
Regex-based header propagation rules are not supported for gRPC subgraphs.
The PropagatedHeaders function returns both explicit header names and regex-based patterns, but only the explicit names are forwarded to gRPC subgraphs (the regex patterns are discarded). The HeaderForwardingInterceptor only supports exact header name matching and has no infrastructure for evaluating regex patterns. Consider adding a comment explaining this is a deliberate limitation of the gRPC implementation, as HTTP subscriptions support both header name and regex-based propagation rules.
🤖 Prompt for AI Agents
In `@router/core/graph_server.go` around lines 1579 - 1598, The PropagatedHeaders
result currently discards regex-based patterns when building headersToForward
for gRPC subgraphs; update the block around PropagatedHeaders/FetchURLRules
(used when s.headerRules != nil) and the grpcremote.NewRemoteGRPCProvider call
to include a brief comment that regex-based header propagation is intentionally
not supported for gRPC subgraphs because the HeaderForwardingInterceptor only
accepts exact header names (HeadersToForward) and lacks runtime regex
evaluation, while HTTP subscriptions support both explicit names and regex
rules—mention the deliberate limitation and reference PropagatedHeaders,
HeaderForwardingInterceptor, and HeadersToForward so future maintainers
understand why regex patterns are ignored.
1 participant
Summary by CodeRabbit
✏️ Tip: You can customize this high-level summary in your review settings.
Checklist