chipingress: send resource attributes as prefixed gRPC metadata - #2288
Draft
pkcll wants to merge 1 commit into
Draft
chipingress: send resource attributes as prefixed gRPC metadata#2288pkcll wants to merge 1 commit into
pkcll wants to merge 1 commit into
Conversation
Resource attributes describe the producer, not any individual event. Carrying
them as per-event CloudEvent extensions repeated identical bytes for every
event in a batch — for a thousand events with ten attributes, roughly 300 KB
that counts against maxGRPCRequestSize and so reduces how many events fit per
batch. OTLP factors resource out of the payload for the same reason. Send them
once per request as gRPC metadata instead.
Every emitted key is ResourceHeaderPrefix ("resource_") followed by a key
normalized to grpc's charset, which grpc-go gives as [0-9a-z-_.]
(internal/metadata.ValidateKey). Structure therefore survives —
csa_public_key becomes resource_csa_public_key rather than collapsing to
csapublickey — which is what lets chip-ingress emit the forwarded header
verbatim. Values still go through SanitizeMetadataValue, because grpc-go fails
an entire RPC, auth header included, on one non-printable value. A trailing
"-bin" is rewritten so grpc does not try to base64-decode a plain-text
attribute.
The prefix is a wire contract with chip-ingress, which forwards metadata
carrying it onto every Kafka record a request produces. Requiring it inbound
and preserving it outbound keeps the namespace closed, and that is what
removes the need for a reserved-key set on either side. The header interceptor
appends to outgoing metadata rather than replacing, so an attribute named
X-Beholder-Node-Auth-Token would have sent a second value under the key
carrying the CSA node auth token and broken authentication; prefixed, it
becomes resource_x-beholder-node-auth-token and collides with nothing. The
same holds for authorization, te, content-type, the grpc- prefix and
pseudo-headers, so reservedMetadataHeaderNames, reservedMetadataKeys and
isReservedMetadataKey are all deleted rather than extended. A test asserts the
property directly, in place of the set it replaces.
Removes EventOpt, NewEventWithOpts and WithResourceAttributeExtensions, which
existed only for the extension path and have no callers in either repository;
NewEvent returns to being the single event constructor. With
SanitizeMetadataHeaders the sole consumer of the shared key helper, fold it in
and delete resource_attributes.go and the resourceAttrKey pair type — the
sanitized output map doubles as the dedupe set. SanitizeMetadataKey becomes
unexported, since nothing outside the package used it.
Adds ResourceHeaderPrefix. The same constant exists in chip-ingress as
constants.ResourceHeaderPrefix; duplicating a wire contract across
repositories matches how authHeaderKey is already spelled in both
pkg/beholder and pkg/chipingress, and the two must stay byte-identical or
forwarding silently stops.
Contributor
|
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates pkg/chipingress resource-attribute propagation to use request-scoped gRPC metadata only, namespaced under a resource_ prefix, and removes the prior per-event CloudEvent extension mechanism. This aligns resource attributes with their producer-scoped semantics and avoids repeated per-event payload bloat while ensuring attributes cannot collide with reserved/auth metadata keys.
Changes:
- Introduces
ResourceHeaderPrefix = "resource_"as the wire contract for resource attributes sent via gRPC metadata. - Reworks
SanitizeMetadataHeadersto (1) structure-preserving normalize keys to gRPC’s allowed charset and (2) always prefix withresource_, replacing the previous reserved-key deny-list approach. - Removes the CloudEvent extension path (
EventOpt,NewEventWithOpts,WithResourceAttributeExtensions) and deletes the shared resource-attribute helper file, consolidating logic around metadata.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| pkg/chipingress/types.go | Adds ResourceHeaderPrefix constant and removes reserved-key sets tied to the deleted CE-extension path. |
| pkg/chipingress/resource_attributes.go | Deletes the shared resource-attribute key helper now made redundant by the new metadata sanitization approach. |
| pkg/chipingress/header_provider.go | Implements structure-preserving metadata key normalization, applies resource_ prefix to all emitted keys, and removes reliance on reserved-key lists. |
| pkg/chipingress/header_provider_test.go | Updates/expands tests to assert prefixing, normalization rules, -bin rewrite, deterministic collision handling, and non-collision with reserved/auth keys. |
| pkg/chipingress/client.go | Removes CE-extension event opts and documents that resource attributes are request-scoped via WithResourceAttributeHeaders; small refactor for NOP lookup header key. |
| pkg/chipingress/client_test.go | Removes CE-extension tests and adds an integration-style test asserting auth token coexists with prefixed resource attributes without collision. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
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.
Follow-up to #2267, which added resource-attribute support to the ChipIngress client via two
mechanisms: per-event CloudEvent extensions and per-request gRPC metadata. This keeps the metadata
path, gives it a namespace, and deletes the extension path.
Why metadata rather than per-event extensions
Resource attributes describe the producer, not any individual event. That is why OTLP factors
resourceout of its payload rather than repeating it on every log record, and the same reasoningapplies here.
Per-event stamping also duplicated bytes: ~10 attributes across a 1,000-event batch is roughly 300 KB
of identical content, which counts against
maxGRPCRequestSizeand therefore reduces how many eventsfit per batch.
Chip-ingress already fans connection-scoped values out onto every Kafka record — the verified auth
token becomes
ce_csapublickey, the NOP lookup becomesce_nodeoperatorname— viabaseKafkaHeaders,cached per
(domain, entity, specVersion). Resource attributes now follow that same path.The
resource_contractEvery emitted metadata key is
resource_+ a key normalized to grpc's charset, which grpc-go gives as[0-9a-z-_.](internal/metadata.ValidateKey). Structure therefore survives —csa_public_keybecomes
resource_csa_public_keyrather than collapsing tocsapublickey— which is what letschip-ingress emit the forwarded header verbatim.
The prefix is why neither side needs a reserved-key set. The header interceptor appends to
outgoing metadata rather than replacing it, so an attribute landing on an existing header name sends
two values under one key. For the CSA auth token that breaks authentication. Prefixed, that attribute
becomes
resource_x-beholder-node-auth-tokenand collides with nothing — and the same holds forauthorization,te,content-type, thegrpc-prefix and pseudo-headers.So
reservedMetadataHeaderNames,reservedMetadataKeysandisReservedMetadataKeyare deletedrather than extended, replaced by a test asserting the property directly. Enumerated deny-lists have
to be maintained correctly forever; this one is closed by construction.
Changes
types.go— addResourceHeaderPrefix; delete the reserved-key set and its predicate.header_provider.go—SanitizeMetadataHeadersprefixes every key;SanitizeMetadataKeybecomesunexported (nothing outside the package used it).
client.go— deleteEventOpt,NewEventWithOptsandWithResourceAttributeExtensions;NewEventreturns to being the single event constructor.resource_attributes.go— deleted. WithSanitizeMetadataHeadersthe sole consumer of the sharedkey helper, it folds in and the sanitized output map doubles as the dedupe set.
Exported API
EventOpt,NewEventWithOpts,WithResourceAttributeExtensionsResourceHeaderPrefixSanitizeMetadataHeadersAll three removals were added by #2267 and have zero callers in either chainlink-common or
chainlink — verified by grep across both repos.
SanitizeMetadataValue,NewStaticHeaderProviderandWithResourceAttributeHeaderskeep their signatures.Paired changes
resource_onto every Kafkarecord. Independent of this PR; consumers see nothing until both are deployed.
WithResourceAttributeHeaders, soprefixing happens in here, and its test asserts through
SanitizeMetadataHeadersrather thanhardcoded keys. Verified by running beholder: send resource attributes to chip ingress as gRPC metadata #2216's suite against this branch via a local
replace. It doesneed its
pkg/chipingresspin bumped once this merges.ResourceHeaderPrefixis deliberately duplicated asconstants.ResourceHeaderPrefixinchip-ingress — the same cross-repo duplication
authHeaderKeyalready has betweenpkg/beholderandpkg/chipingress. The two must stay byte-identical or forwarding silently stops.Test plan
Tests bind
127.0.0.1:0, so they need a sandbox that permits listening. Notable cases: prefixing andstructure preservation, the
-binrewrite, deterministic collapse of keys that normalize to the samename, and — on a live gRPC connection — that the CSA auth token arrives exactly once alongside
prefixed attributes, including one deliberately named after the auth header.