feat(grpc): configurable default headers on shared gRPC client

[Freyskeyd]

Make the default metadata headers on the shared gRPC client configurable
per client, so x-client-version / x-client-type can be overridden
(e.g. for tracing/observability).

🔄 Changes Summary

• Add optional Headers map to grpc.ClientConfig; entries are appended
  to every outgoing request on that client.
• Configured x-client-version / x-client-type override the built-in
  defaults (single value, not duplicated); other keys are added alongside.
• Generalize the version-header interceptor into HeaderInterceptor(headers)
  fed by mergeHeaders(cfg.Headers); empty config preserves current behavior.

📋 Config Updates

• New optional per-client field GRPC.Headers (map). Example:

[AggSender.AgglayerClient.GRPC.Headers]
    "x-client-type"    = "aggkit-aggsender"
    "x-client-version" = "v1.2.3-rc1"

✅ Testing

• Unit: mergeHeaders (defaults / overrides / case-normalization) and
  HeaderInterceptor (override-once, arbitrary header, existing cases).
• go build ./..., gofmt, go vet, package tests for grpc + consumers,
  and golangci-lint v2.4.0 (0 issues).

📝 Notes

• Internal rename: exported VersionHeaderInterceptor() ->
  HeaderInterceptor(map[string]string) (only consumer is NewClient).
• No startup validation of header keys/values (kept minimal); invalid
  metadata surfaces at gRPC call time.

[claude]

Claude finished @Freyskeyd's task in 2m 21s —— View job

---

Code Review Complete ✅

---

🚀 What's New

This PR adds configurable default metadata headers to the shared gRPC client, allowing per-client customization of x-client-version and x-client-type headers for improved tracing and observability. The implementation includes a new optional Headers map in ClientConfig that merges with built-in defaults, with case-insensitive key normalization to prevent duplicates.

🐛 Bug Fixes

None

📋 Config Updates

New optional per-client field GRPC.Headers (map[string]string). Example:

[AggSender.AgglayerClient.GRPC.Headers]
    "x-client-type"    = "aggkit-aggsender"
    "x-client-version" = "v1.2.3-rc1"

Default behavior: If Headers is not specified or empty, the client uses built-in defaults:

• x-client-version: Current aggkit version
• x-client-type: "aggkit"

⚠️ Breaking Changes

🟡 Suggested - Minor API Change: Exported function VersionHeaderInterceptor() has been renamed to HeaderInterceptor(map[string]string). However, grep search confirms no external consumers exist in the codebase, so this is effectively internal-only and safe.

---

Review Findings

🟢 Strengths

1. Excellent test coverage: Added 8 comprehensive test cases covering defaults, overrides, case normalization, and edge cases
2. Clean implementation: The mergeHeaders function is simple and correct
3. Backward compatible: Empty/nil Headers config preserves existing behavior
4. Well-documented: Clear comments explain the purpose and behavior

🟡 Suggested Improvements

1. Header validation considerations (grpc/client.go:49-58)

The implementation doesn't validate header keys/values. While invalid metadata will surface at gRPC call time (as noted in the PR description), consider these scenarios:

• Empty keys: map[string]string{"": "value"} would create an invalid metadata entry
• Control characters: Keys/values with newlines or other control characters could cause issues
• Reserved prefixes: gRPC reserves the grpc- prefix for internal use

Recommendation: Add basic validation in ClientConfig.Validate() to catch obvious issues early:

func (c *ClientConfig) Validate() error {
    // ... existing validation ...
    
    for k, v := range c.Headers {
        if k == "" {
            return fmt.Errorf("header key cannot be empty")
        }
        if strings.HasPrefix(strings.ToLower(k), "grpc-") {
            return fmt.Errorf("header key %q uses reserved 'grpc-' prefix", k)
        }
        // Optionally check for control characters
    }
    return nil
}

2. Potential header duplication issue (grpc/client.go:69-71)

The interceptor uses metadata.AppendToOutgoingContext, which appends to existing metadata. If a user sets custom metadata in their context before calling a gRPC method, headers could be duplicated:

// User code:
ctx := metadata.AppendToOutgoingContext(ctx, "x-client-type", "custom")
// Interceptor appends again:
ctx = metadata.AppendToOutgoingContext(ctx, "x-client-type", "aggkit-aggsender")
// Result: Two x-client-type headers

Current behavior analysis: The test "PreservesExistingMetadata" (line 573) doesn't test for duplicate keys, only distinct keys. gRPC metadata allows multiple values per key, so this would result in ["custom", "aggkit-aggsender"] being sent.

Is this a problem? Depends on use case:

• For tracing headers, having duplicates might confuse downstream systems
• The current implementation matches the description's intent ("appended to every outgoing request")

Options:

1. Document this behavior clearly (user should not set these headers manually)
2. Use a different approach that replaces instead of appends (though this requires more complex implementation)

3. Missing test for empty string value (grpc/client_test.go:777-822)

The TestMergeHeaders suite covers nil, empty map, overrides, and case normalization, but doesn't test what happens with empty string values:

mergeHeaders(map[string]string{"x-client-type": ""})

This would override the default with an empty string, which might not be the desired behavior. Consider either:

• Adding validation to reject empty values
• Adding a test to document the current behavior

4. ClientConfig.String() doesn't include Headers (grpc/client.go:124-135)

The String() method formats config for logging but omits the new Headers field. This could make debugging harder. Consider adding:

return fmt.Sprintf("GRPC Client Config: "+
    "URL=%s, MinConnectTimeout=%s, "+
    "RequestTimeout=%s, UseTLS=%t, Retry=%s, Headers=%d",
    c.URL, c.MinConnectTimeout.String(),
    c.RequestTimeout.Duration, c.UseTLS, c.Retry.String(), len(c.Headers))

(Showing header count instead of values to avoid logging sensitive data)

🟢 Optional Enhancements

1. Consider streaming interceptor support

The current implementation only provides UnaryClientInterceptor. If the codebase uses streaming gRPC calls, you might want to add a StreamClientInterceptor as well:

func StreamHeaderInterceptor(headers map[string]string) grpc.StreamClientInterceptor {
    return func(ctx context.Context, desc *grpc.StreamDesc, cc *grpc.ClientConn, 
                method string, streamer grpc.Streamer, opts ...grpc.CallOption) (grpc.ClientStream, error) {
        for k, v := range headers {
            ctx = metadata.AppendToOutgoingContext(ctx, k, v)
        }
        return streamer(ctx, desc, cc, method, opts...)
    }
}

2. Documentation update

Consider adding a brief comment in the PR or in code documentation about:

• The fact that header keys are normalized to lowercase
• That configured headers win over defaults
• Common use cases (e.g., per-environment tracing identifiers)

---

Code Quality Assessment

✅ Follows Go best practices: Clean, idiomatic Go code
✅ Test coverage: Comprehensive unit tests (98 new lines of tests for 30 lines of prod code)
✅ CLAUDE.md compliance: Uses require (not assert), proper error wrapping, good naming
✅ No security issues: No injection risks, no credential exposure
✅ Performance: Negligible impact - header merging happens once per client creation

---

CI Status

⚠️ Note: The "Go Vulnerability Check" workflow failed. This appears to be unrelated to the changes in this PR (grpc client config), but should be investigated separately.

Other CI checks (Lint, Unit Tests, E2E Tests, CodeQL) are currently in progress.

---

Summary

This is a well-implemented feature with excellent test coverage and clean code. The suggested improvements are minor and mostly about edge case handling and documentation. The implementation correctly achieves the stated goal of making default headers configurable for observability purposes.

Recommendation: ✅ Approve with minor suggestions

The code is ready to merge as-is, though implementing the suggested header validation in ClientConfig.Validate() would provide better user experience by catching configuration errors early rather than at runtime.