Add resource attribute support to ChipIngress client#2267
Open
pkcll wants to merge 10 commits into
Open
Conversation
Propagate node resource attributes via gRPC metadata headers and sanitized CloudEvent extensions.
Root module must pin the submodule commit that contains the new chipingress API.
pkcll
added a commit
that referenced
this pull request
Jul 21, 2026
Propagate node resource attributes from beholder.Config.ResourceAttributes to the cre topic via ChipIngress CloudEvent extensions. Depends on the new chipingress API introduced in #2267.
Contributor
There was a problem hiding this comment.
Pull request overview
Adds resource-attribute propagation support to the pkg/chipingress client so node resource attributes can reach downstream consumers both via gRPC metadata headers and via CloudEvents extensions (sanitized per the CloudEvents extension name requirements). This aligns ChipIngress emission with existing telemetry/resource-attribute propagation patterns and prepares for future server-side forwarding of arbitrary metadata.
Changes:
- Add CloudEvent
EventOptsupport (NewEventWithOpts) and a resource-attribute → CloudEvent-extension helper (WithResourceAttributeExtensions) with deterministic collision handling. - Add gRPC-metadata-focused helpers (
NewStaticHeaderProvider,SanitizeMetadataHeaders,SanitizeMetadataValue) to safely attach resource attributes as outgoing metadata. - Fix gRPC unary interceptor registration so multiple header providers compose (via
grpc.WithChainUnaryInterceptor), plus dependency bumps and tests.
Reviewed changes
Copilot reviewed 7 out of 9 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/chipingress/client.go | Adds EventOpt plumbing, extension-name sanitization, resource-attribute extension support, and fixes interceptor chaining for multiple header providers. |
| pkg/chipingress/client_test.go | Adds unit/regression tests for extension-name sanitization, resource-attribute extension behavior, and chained header provider interception. |
| pkg/chipingress/header_provider.go | Exposes a static header provider plus gRPC-metadata sanitization utilities for safe outgoing metadata. |
| pkg/chipingress/header_provider_test.go | Adds tests covering new static provider + metadata sanitization, including a regression test preventing RPC failure on invalid metadata. |
| pkg/chipingress/types.go | Introduces reserved-name sets to prevent resource attributes from colliding with CloudEvents core/internal attributes and gRPC-reserved headers. |
| pkg/chipingress/go.mod | Updates pkg/chipingress module dependency versions (grpc/protobuf/zap and related indirects). |
| pkg/chipingress/go.sum | Syncs sums to the updated pkg/chipingress module dependencies. |
| go.mod | Bumps root module’s pkg/chipingress pseudo-version to this branch’s commit. |
| go.sum | Syncs sums for the root module’s updated pkg/chipingress pseudo-version. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Comment on lines
+112
to
+117
| // NewStaticHeaderProvider returns a HeaderProvider that always returns the given headers, | ||
| // for use with WithHeaderProvider to attach fixed, non-auth gRPC metadata (e.g. resource | ||
| // attributes) to every request. | ||
| func NewStaticHeaderProvider(headers map[string]string) HeaderProvider { | ||
| return newStaticHeaderProvider(headers, false) | ||
| } |
Comment on lines
+35
to
+38
| // reservedMetadataKeys holds gRPC-reserved header names that could otherwise be reached by | ||
| // SanitizeExtensionName's [a-z0-9] sanitization. Verified against grpc-go v1.79.1's | ||
| // isReservedHeader: every other reserved header (pseudo-headers, "content-type", "grpc-*") | ||
| // contains a ':' or '-' that sanitization strips, so "te" is the only one actually reachable. |
pkcll
added a commit
that referenced
this pull request
Jul 21, 2026
Propagate node resource attributes from beholder.Config.ResourceAttributes to the cre topic via ChipIngress CloudEvent extensions. Depends on the new chipingress API introduced in #2267.
pkcll
added a commit
that referenced
this pull request
Jul 21, 2026
Root module must pin the submodule commit from #2267 that contains the new chipingress API.
thomaska
previously approved these changes
Jul 21, 2026
| // extension-name character set. Entries that sanitize to an empty string, or that collide with a | ||
| // reserved extension name (see reservedExtensionNames), are skipped. Keys are applied in sorted | ||
| // order so that if two distinct keys sanitize to the same name, the result is deterministic. | ||
| func WithResourceAttributeExtensions(attrs map[string]string) EventOpt { |
Collaborator
There was a problem hiding this comment.
Opus picked up this:
WithResourceAttributeExtensions (client.go) and SanitizeMetadataHeaders (header_provider.go) both implement the same "collect keys → sort → sanitize via SanitizeExtensionName → skip empty/reserved/dupe" loop, differing only in the extra reservedMetadataKeys check and what happens to a surviving key (event.SetExtension vs. sanitize-value-and-store). Worth extracting a shared helper that returns the deduplicated, sorted (sanitizedName, originalKey) pairs, with each caller supplying just its own "apply" step. Saves ~15 duplicated lines and means any future change to the collision/ordering rule (e.g. "last wins" instead of "first wins") only has to happen once.
jmank88
reviewed
Jul 21, 2026
|
|
||
| // SanitizeExtensionName lower-cases name and strips every rune outside [a-z0-9], the character | ||
| // set the CloudEvents spec requires for extension attribute names. | ||
| func SanitizeExtensionName(name string) string { |
Contributor
There was a problem hiding this comment.
Does this need to be exported?
Contributor
✅ API Diff Results -
|
Contributor
📊 API Diff Results
|
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 9 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (1)
pkg/chipingress/types.go:40
- The comment says the reserved metadata keys were verified against grpc-go v1.79.1, but this module now depends on google.golang.org/grpc v1.80.0. Keeping the referenced version in sync (or removing the specific version) avoids stale documentation as grpc-go’s reserved-header set changes over time.
// reservedMetadataKeys holds gRPC-reserved header names that could otherwise be reached by
// SanitizeExtensionName's [a-z0-9] sanitization. Verified against grpc-go v1.79.1's
// isReservedHeader: every other reserved header (pseudo-headers, "content-type", "grpc-*")
// contains a ':' or '-' that sanitization strips, so "te" is the only one actually reachable.
// SanitizeMetadataHeaders consults this set so that edge case is handled deterministically
// rather than relying on grpc's own (silent) handling of a reserved header.
Comment on lines
+355
to
+359
| // TestSanitizeMetadataHeaders_AvoidsRPCFailure is a regression/guard test for the core reason | ||
| // SanitizeMetadataHeaders exists: grpc-go hard-fails an entire RPC (codes.Internal) when an | ||
| // outgoing metadata pair fails its charset validation. An unsanitized resource-attribute key or | ||
| // value (dots, non-printable characters) reproduces that failure; running it through | ||
| // SanitizeMetadataHeaders first must not. |
Comment on lines
+33
to
+36
| golang.org/x/net v0.49.0 // indirect | ||
| golang.org/x/sys v0.42.0 // indirect | ||
| golang.org/x/text v0.33.0 // indirect | ||
| google.golang.org/genproto/googleapis/rpc v0.0.0-20260120221211-b8f7ae30c516 // indirect |
pkcll
marked this pull request as ready for review
July 22, 2026 05:44
product-security-plaid-production
Bot
requested review from
4of9,
engnke,
fouadkada,
kirqz23 and
thomaska
July 22, 2026 05:45
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 7 out of 9 changed files in this pull request and generated 2 comments.
Comments suppressed due to low confidence (3)
pkg/chipingress/types.go:36
- This comment references a specific grpc-go version (v1.79.1), but the module now requires grpc v1.80.0. To avoid the comment becoming stale on future bumps, consider removing the pinned version reference.
// sanitizeExtensionName's [a-z0-9] sanitization. Verified against grpc-go v1.79.1's
pkg/chipingress/go.mod:35
- This change downgrades indirect golang.org/x/* dependencies (x/net, x/sys, x/text) compared to the previous versions. If this wasn’t intentional, it’s safer to keep them at the newer versions to avoid losing bug/security fixes.
golang.org/x/net v0.49.0 // indirect
golang.org/x/sys v0.42.0 // indirect
golang.org/x/text v0.33.0 // indirect
pkg/chipingress/header_provider_test.go:359
- This test comment implies that dots in metadata keys contribute to gRPC metadata validation failures, but grpc-go allows '.' in metadata keys. In this test case, the RPC failure is driven by the non-printable byte in the metadata value; the comment should reflect that to avoid confusion.
// TestSanitizeMetadataHeaders_AvoidsRPCFailure is a regression/guard test for the core reason
// SanitizeMetadataHeaders exists: grpc-go hard-fails an entire RPC (codes.Internal) when an
// outgoing metadata pair fails its charset validation. An unsanitized resource-attribute key or
// value (dots, non-printable characters) reproduces that failure; running it through
// SanitizeMetadataHeaders first must not.
Comment on lines
+115
to
+117
| func NewStaticHeaderProvider(headers map[string]string) HeaderProvider { | ||
| return newStaticHeaderProvider(headers, false) | ||
| } |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 10 changed files in this pull request and generated no new comments.
Comments suppressed due to low confidence (1)
pkg/chipingress/types.go:38
- The comment for reservedMetadataKeys hard-codes grpc-go v1.79.1, but this module now depends on google.golang.org/grpc v1.80.0 (see pkg/chipingress/go.mod). This makes the reference stale and potentially misleading for future maintenance.
// reservedMetadataKeys holds gRPC-reserved header names that could otherwise be reached by
// sanitizeExtensionName's [a-z0-9] sanitization. Verified against grpc-go v1.79.1's
// isReservedHeader: every other reserved header (pseudo-headers, "content-type", "grpc-*")
// contains a ':' or '-' that sanitization strips, so "te" is the only one actually reachable.
thomaska
previously approved these changes
Jul 22, 2026
thomaska
reviewed
Jul 22, 2026
| // NewStaticHeaderProvider returns a HeaderProvider that always returns the given headers, | ||
| // for use with WithHeaderProvider to attach fixed, non-auth gRPC metadata (e.g. resource | ||
| // attributes) to every request. | ||
| func NewStaticHeaderProvider(headers map[string]string) HeaderProvider { |
Collaborator
There was a problem hiding this comment.
it would be useful to chain the creation of HeaderProvider with sanitization, considering how important it is.
ie. Could we add a convenience Opt that composes sanitize + construct so the safe path is the only path, e.g.:
func WithResourceAttributeHeaders(attrs map[string]string) Opt {
return WithHeaderProvider(NewStaticHeaderProvider(SanitizeMetadataHeaders(attrs)))
}
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 10 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (2)
pkg/chipingress/types.go:38
- Comment references grpc-go v1.79.1, but this module now requires google.golang.org/grpc v1.80.0. Please update the version in the comment (or avoid pinning a specific version) so the documentation stays accurate.
// reservedMetadataKeys holds gRPC-reserved header names that could otherwise be reached by
// sanitizeExtensionName's [a-z0-9] sanitization. Verified against grpc-go v1.79.1's
// isReservedHeader: every other reserved header (pseudo-headers, "content-type", "grpc-*")
// contains a ':' or '-' that sanitization strips, so "te" is the only one actually reachable.
pkg/chipingress/go.mod:35
- This change downgrades golang.org/x/net, golang.org/x/sys, and golang.org/x/text compared to the previous versions (and compared to the repo root go.mod). Downgrading x/* packages can reintroduce fixed CVEs; if this wasn’t intentional, consider keeping them at the prior/higher versions.
golang.org/x/net v0.49.0 // indirect
golang.org/x/sys v0.42.0 // indirect
golang.org/x/text v0.33.0 // indirect
| grpcOpts = append(grpcOpts, grpc.WithPerRPCCredentials(cfg.perRPCCredentials)) | ||
| } | ||
| // Add headers as a unary interceptor, use for non-auth headers | ||
| // Add headers as unary interceptors, use for non-auth headers. |
Contributor
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 8 out of 10 changed files in this pull request and generated 1 comment.
Comments suppressed due to low confidence (1)
pkg/chipingress/types.go:38
- The comment references grpc-go v1.79.1, but this module now depends on google.golang.org/grpc v1.80.0 (see pkg/chipingress/go.mod). This makes the rationale for the reserved-key set look stale and may confuse future audits; update the version reference (or remove the hard-coded version) to match the current dependency.
// reservedMetadataKeys holds gRPC-reserved header names that could otherwise be reached by
// sanitizeExtensionName's [a-z0-9] sanitization. Verified against grpc-go v1.79.1's
// isReservedHeader: every other reserved header (pseudo-headers, "content-type", "grpc-*")
// contains a ':' or '-' that sanitization strips, so "te" is the only one actually reachable.
Comment on lines
+238
to
+252
| func TestNewEvent_UnchangedSignature(t *testing.T) { | ||
| payload := []byte("body") | ||
| attributes := map[string]any{"subject": "example-subject"} | ||
|
|
||
| viaNewEvent, err := NewEvent("domain", "entity", payload, attributes) | ||
| require.NoError(t, err) | ||
|
|
||
| viaNewEventWithOpts, err := NewEventWithOpts("domain", "entity", payload, attributes) | ||
| require.NoError(t, err) | ||
|
|
||
| assert.Equal(t, viaNewEventWithOpts.Subject(), viaNewEvent.Subject()) | ||
| assert.Equal(t, viaNewEventWithOpts.Extensions()["recordedtime"].(ce.Timestamp).Truncate(time.Second), | ||
| viaNewEvent.Extensions()["recordedtime"].(ce.Timestamp).Truncate(time.Second)) | ||
| assert.Equal(t, viaNewEventWithOpts.Data(), viaNewEvent.Data()) | ||
| } |
thomaska
approved these changes
Jul 22, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Add resource-attribute propagation to the ChipIngress client so that node resource attributes from
[Telemetry.ResourceAttributes]can reach thecretopic via two mechanisms:beholder__platform__messagesonce the chip-ingress server forwards arbitrary metadata).^[a-z0-9]+$), surfaced asce_<name>Kafka headers by the CloudEvents Kafka binding.Changes
pkg/chipingress/client.go: addNewEventWithOpts/WithResourceAttributeExtensions,SanitizeExtensionName, and fix interceptor chaining so header providers compose.pkg/chipingress/header_provider.go: addNewStaticHeaderProvider,SanitizeMetadataHeaders,SanitizeMetadataValue.pkg/chipingress/types.go: addreservedExtensionNamesandreservedMetadataKeys.go.mod: bumppkg/chipingresspseudo-version to this branch.Related PRs
Test plan
go test ./pkg/chipingress/...Notes
ce_<name>headers immediately.Co-authored-by: Cursorattribution has been removed from the commit history.