feat: promote allowlisted caller context onto every agent span - #2575
feat: promote allowlisted caller context onto every agent span#2575rtemperini wants to merge 5 commits into
Conversation
krisztianfekete
left a comment
krisztianfekete
left a comment
There was a problem hiding this comment.
Thanks, added a few initial comments!
| Agent spans describe *what the agent did*, but they say nothing about *who asked | ||
| for it*. Kagent can promote a configurable allowlist of caller-supplied values — | ||
| the signed-in user's email, a Slack thread, a support ticket ID — onto every | ||
| span of a request, so traces can be filtered and grouped by the caller in | ||
| Langfuse, Jaeger, Grafana Tempo, or any other OTLP backend. |
There was a problem hiding this comment.
Totally agree with the goal, but as per the OTel's recommendations: https://opentelemetry.io/docs/security/handling-sensitive-data/, email addresses and names should never be attributes at all.
OIDC already hands you an opaque sub, which is what user.id should use. Could we make sub the example instead of the email?
There was a problem hiding this comment.
Thanks for the review, I agree, examples now use an OIDC sub mapped to user.id rather than an email. Docs, Helm values, and the PR description all follow that. Locked by TestCallerContextAttributes/maps_source_keys_onto_registry_and_kagent_names (Go) and test_maps_source_keys_onto_registry_and_kagent_names (Python).
| # as kagent.context.<key>. Values are read from W3C baggage and A2A message | ||
| # metadata. Empty (the default) disables promotion. | ||
| # e.g. ["user.email", "user.name", "thread_id", "channel"] | ||
| contextKeys: [] |
There was a problem hiding this comment.
Let's use subs everywhere as exampels as mentioned in the md file above.
There was a problem hiding this comment.
Updated. values.yaml now shows {from: sub, to: user.id} (and thread_id / channel), not names or addresses. Helm unittest should join otel.tracing.contextKeys / should encode contextKeys mappings as JSON lock the rendered forms.
| Every promoted value becomes a span attribute named `kagent.context.<key>`: | ||
|
|
||
| ``` | ||
| baggage: user.email=ada@example.com → kagent.context.user.email = "ada@example.com" | ||
| metadata: {"thread_id": "1717171.42"} → kagent.context.thread_id = "1717171.42" |
There was a problem hiding this comment.
Same as above, let's not document it like this as it's an anti-pattern.
There was a problem hiding this comment.
Removed. The landing example is now sub → user.id. The "Sensitive values" section points at the OTel guidance and tells operators not to put names or addresses on spans.
| | Oversized spans | Values are truncated to 256 characters, keys to 64 | | ||
| | Log or trace injection | Control characters are stripped from values | | ||
| | Shadowing semantic conventions | Every attribute is namespaced under `kagent.context.`, so `service.name` and friends cannot be overwritten even if an operator allowlists them | | ||
| | Leaking secrets into a trace backend | Nothing is promoted unless an operator names the key; raw values are never logged | |
There was a problem hiding this comment.
Again, emails and username should not be here as per the guidance because it's a compliance question. Operators should also not do this. Also baggage is on HTTP headers, and today this data goes to api.openai.com and every HTTP MCP server too.
There was a problem hiding this comment.
Documented, and the implementation follows that. Hashing at promotion time only changes the span attribute — baggage is still an HTTP header, so a value placed there still reaches model providers and HTTP MCP servers. The docs say to hash or replace at the edge before the request enters the cluster, and hash: hmac-sha256 never falls back to writing the original onto the span if the HMAC key is missing (TestCallerContextAttributes_HashWithoutKeyEmitsNothing / test_hash_without_key_emits_nothing).
| KagentTraceContextKeys = RegisterStringVar( | ||
| "KAGENT_TRACE_CONTEXT_KEYS", | ||
| "", | ||
| "Comma-separated allowlist of caller-supplied context keys promoted onto every agent span as "+ | ||
| "kagent.context.<key>. Values are read from W3C baggage and A2A message metadata. "+ | ||
| "Empty (the default) disables promotion.", | ||
| ComponentAgentRuntime, | ||
| ) |
There was a problem hiding this comment.
If you don't want to lose the OIDC format the recommendation is to hash it, and user.hash is a registry attribute that exists for exactly that. Something like:
contextKeys:
- {from: sub, to: user.id}
- {from: email, to: user.hash, hash: hmac-sha256}
- {from: thread_id, to: kagent.thread_id}
There was a problem hiding this comment.
Implemented this mapping shape: {from, to, hash}. hash: hmac-sha256 writes user.hash using KAGENT_TRACE_CONTEXT_HASH_KEY (Helm contextHashKeySecret). If the key is unset or the algorithm is unknown, the attribute is omitted rather than emitted in plaintext. A Harness cannot supply or replace the key.
Tests: TestCallerContextAttributes_HashesWithHMACSHA256, TestCallerContextAttributes_HashWithoutKeyEmitsNothing, TestCallerContextAttributes_UnknownHashEmitsNothing, and the matching Python cases. Tenant-cannot-set is TestCompileAgentTemplateForwardsTraceContextPolicy/harness_cannot_supply_the_HMAC_key.
| // contextAttributePrefix namespaces every promoted value. Because the prefix | ||
| // is applied unconditionally, caller-supplied data cannot shadow a semantic | ||
| // convention attribute such as service.name. | ||
| contextAttributePrefix = "kagent.context." |
There was a problem hiding this comment.
The prefix is right for custom keys e.g. channel, but it also blocks the names that do exist in the registry. user.id, enduser.id, session.id are all real attributes, and we should use the semconv names before inventing new ones.
Could we let a small fixed set through unprefixed (user.*, enduser.*, session.id) and prefix everything else?
There was a problem hiding this comment.
Also reflect this in tests.
There was a problem hiding this comment.
Done. user.*, enduser.*, and session.id pass through unprefixed; names already in the kagent. namespace stay as-is; everything else is still kagent.context.<name>, so service.name cannot be shadowed.
Tests: TestCallerContextAttributes/registry_attributes_stay_unprefixed, session.id is unprefixed but session.foo is not, CannotShadowSemanticConventions, and the matching Python cases.
Agent spans record what an agent did but not who asked for it, so traces cannot be filtered or grouped by the calling user, Slack thread, or ticket that triggered them. Read an operator-defined allowlist of context keys from W3C baggage and A2A message metadata and merge them into the request-scoped attribute bag. The existing span processor then stamps them on every span of the request, which is what trace-level filtering in Langfuse and comparable backends requires; attaching them to the root span alone leaves most views unfilterable. Baggage is the primary source because it already survives the controller, agent, sub-agent, and tool hops under the composite propagator, so a value set once at the edge needs no further plumbing. A2A message metadata covers callers that cannot set headers and, being per-message, takes precedence. Caller data is untrusted, so only allowlisted keys are read, the allowlist is capped, values are truncated and stripped of control characters, and every attribute is namespaced under kagent.context. so it cannot shadow a semantic convention attribute such as service.name. The allowlist is empty by default, which disables promotion entirely. Signed-off-by: Ricardo Temperini <29879569+rtemperini@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Bring the Python runtimes to parity with the Go ADK. The Go runtime already reads A2A message metadata into span attributes (kagent-dev#1734, kagent-dev#1737); the Python runtimes ignored inbound metadata entirely, so agents on the Python runtime had no way to get caller identity onto their traces. Mirror the Go implementation exactly: the same KAGENT_TRACE_CONTEXT_KEYS allowlist, the same baggage-then-metadata precedence, the same limits, the same control character stripping, and the same kagent.context. namespace, so the two runtimes cannot drift. Values are merged into the request-scoped attribute bag that KagentAttributesSpanProcessor stamps onto every span. The ADK, LangGraph, and CrewAI executors all promote context through the shared helper. Reading a Message's protobuf Struct metadata moves into kagent.core.a2a.read_message_metadata rather than being repeated per package. Also assert that the OTel SDK's default propagator carries baggage: the Python runtime relies on that default rather than configuring a propagator, so an SDK change that dropped it would silently break the baggage path. Signed-off-by: Ricardo Temperini <29879569+rtemperini@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Expose the allowlist as the Helm value otel.tracing.contextKeys, rendered into the controller ConfigMap as KAGENT_TRACE_CONTEXT_KEYS and forwarded to the agents the controller creates. The variable needs explicit forwarding because collectOtelEnvFromProcess carries only OTEL_ prefixed names. Which caller-supplied data reaches a trace backend is cluster-wide operator policy, so the value is applied after the Harness environment, and any inherited entry of the same name is dropped first. Without that second step a Harness could enable promotion whenever the operator had configured nothing. The value defaults to an empty list, so the ConfigMap key is absent and the runtimes promote nothing unless an operator opts in. Signed-off-by: Ricardo Temperini <29879569+rtemperini@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Cover the configuration knob, why baggage is the primary propagation mechanism, why the attributes are stamped on every span rather than the root, the safety properties that bound untrusted caller input, and how to rename attributes in the OTel Collector for a backend that expects its own names. Signed-off-by: Ricardo Temperini <29879569+rtemperini@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
Address review of the caller-context tracing allowlist: use an OIDC subject as the user.id example, leave user.*, enduser.*, and session.id unprefixed, and hash mapped values with HMAC-SHA256 instead of putting the original on the span. Signed-off-by: Ricardo Temperini <29879569+rtemperini@users.noreply.github.com> Co-authored-by: Cursor <cursoragent@cursor.com>
7c4da91 to
2056591
Compare
github-actions
Bot
added
enhancement
Thanks for the review @krisztianfekete , just updated with your comments, let me know if anything else required |
| } | ||
| // Allowlisted caller context joins the request-scoped bag rather than a | ||
| // single span, so tool, sub-agent, and model spans all carry it. | ||
| maps.Copy(spanAttributes, telemetry.CallerContextAttributes(ctx, reqCtx.Message.Metadata)) |
There was a problem hiding this comment.
The registry passthrough is great, but kagent. names now pass through too and maps.Copy lets caller context override them.
It'ss the exact thing the prefix was protecting. Could caller context only fill keys that aren't already set, instead of copying over them?
| // spanAttributeName is the name written onto the span. | ||
| // | ||
| // user.*, enduser.*, and session.id pass through unprefixed so operators can | ||
| // use the semantic convention names. Names already in the kagent. namespace | ||
| // are left as-is. Everything else is placed under kagent.context. so a | ||
| // caller-supplied service.name cannot shadow the real one. | ||
| func spanAttributeName(name string) string { | ||
| if isRegistryAttribute(name) || strings.HasPrefix(name, "kagent.") { | ||
| return name | ||
| } | ||
| return contextAttributePrefix + name | ||
| } | ||
|
|
There was a problem hiding this comment.
If the merge above in executor gets fixed this should be fine, but specifying the actual attrs kagent.* has can make it more robust.
| func isRegistryAttribute(name string) bool { | ||
| return strings.HasPrefix(name, "user.") || | ||
| strings.HasPrefix(name, "enduser.") || | ||
| name == "session.id" | ||
| } |
There was a problem hiding this comment.
If we just match the user. prefix then user.asdasd goes through unprefixed.
Can we use an explicit set of allowed names here?
| for _, mapping := range mappings { | ||
| value := sanitizeContextValue(bag.Member(mapping.source).Value()) | ||
| if scalar, ok := scalarString(metadata[mapping.source]); ok { | ||
| value = sanitizeContextValue(scalar) | ||
| } | ||
| if value == "" { | ||
| continue | ||
| } |
There was a problem hiding this comment.
Let's only override when the sanitised scalar is non-empty.
| for mapping in mappings: | ||
| value = _sanitize_context_value(bag.get(mapping.source)) | ||
| if metadata is not None: | ||
| scalar = _scalar_string(metadata.get(mapping.source)) | ||
| if scalar is not None: | ||
| value = _sanitize_context_value(scalar) | ||
| if not value: | ||
| continue |
There was a problem hiding this comment.
| // scalarString renders a JSON scalar from A2A message metadata as a string. | ||
| // Objects and arrays are skipped: they are unbounded in size and carry no | ||
| // useful meaning as a span attribute value. | ||
| func scalarString(value any) (string, bool) { | ||
| switch v := value.(type) { | ||
| case string: | ||
| return v, true | ||
| case bool: | ||
| return strconv.FormatBool(v), true | ||
| case float64: | ||
| return strconv.FormatFloat(v, 'g', -1, 64), true | ||
| case int: | ||
| return strconv.Itoa(v), true | ||
| case int64: | ||
| return strconv.FormatInt(v, 10), true | ||
| default: | ||
| return "", false | ||
| } | ||
| } |
There was a problem hiding this comment.
'g' flips to scientific notation and int() doesn't in the matching logic on Python side, so these disagree on ordinary values.
strconv.FormatFloat(v, 'f', -1, 64) matches what Python already does. Also worth noting the int/int64 cases below are unreachable.
| func newContextMapping(from, to, hash string) (contextMapping, bool) { | ||
| from = strings.TrimSpace(from) | ||
| to = strings.TrimSpace(to) | ||
| hash = strings.TrimSpace(hash) | ||
| if from == "" || len(from) > maxContextKeyLength || !isAttributeKey(from) { | ||
| return contextMapping{}, false | ||
| } | ||
| if to == "" { | ||
| to = from | ||
| } | ||
| if len(to) > maxContextKeyLength || !isAttributeKey(to) { | ||
| return contextMapping{}, false | ||
| } | ||
| if hash != "" && hash != hashHMACSHA256 { | ||
| return contextMapping{}, false | ||
| } | ||
| return contextMapping{source: from, attribute: to, hash: hash}, true | ||
| } |
There was a problem hiding this comment.
len() is bytes here and code points in _new_context_mapping, so a 40-character CJK key (120 bytes) is dropped by Go and kept by Python.
len([]rune(from))on both from and to should fix it.
| # Allowlisted caller context joins the request-scoped bag rather | ||
| # than a single span, so tool, sub-agent, and model spans all | ||
| # carry it. | ||
| span_attributes.update(caller_context_attributes(read_message_metadata(context.message))) |
There was a problem hiding this comment.
Let's pass the Message and decode lazily after the allowlist check to avoid running these when this is off.
This should be also fixed for crewai and langgraph as well.
| // allowedContextMappings parses the KAGENT_TRACE_CONTEXT_KEYS allowlist. | ||
| // Entries that are empty, over-long, or contain whitespace or control | ||
| // characters are dropped, and the list is capped at maxContextKeys so a | ||
| // misconfigured allowlist cannot inflate span cardinality without bound. | ||
| func allowedContextMappings() []contextMapping { | ||
| raw := strings.TrimSpace(os.Getenv(traceContextKeysEnvVar)) | ||
| if raw == "" { | ||
| return nil | ||
| } | ||
| if strings.HasPrefix(raw, "[") { | ||
| return capMappings(parseJSONAllowlist(raw)) | ||
| } | ||
| return capMappings(parseCommaAllowlist(raw)) | ||
| } |
There was a problem hiding this comment.
This reparses on every request now that there's JSON in it. Wrap it in sync.OnceValue? The allowlist can't change without a restart.
| // Accepts a comma-separated list of source keys, or a JSON array of strings | ||
| // and {from, to, hash} objects. See allowedContextMappings. | ||
| traceContextKeysEnvVar = "KAGENT_TRACE_CONTEXT_KEYS" | ||
|
|
||
| // traceContextHashKeyEnvVar is the HMAC key used when a mapping sets | ||
| // hash: hmac-sha256. Required for those entries; without it the hashed | ||
| // attribute is skipped rather than emitted in plaintext. | ||
| traceContextHashKeyEnvVar = "KAGENT_TRACE_CONTEXT_HASH_KEY" |
There was a problem hiding this comment.
These names are also declared in go/core/pkg/env/otel.go. Can we use env.KagentTraceContextKeys.Name()` here so there's one source of truth?
2 participants
Motivation
Agent spans record what an agent did but not who asked for it. Once a request
enters kagent the caller's identity and context are gone, so traces cannot be
filtered or grouped by the calling user, the conversation thread, or the ticket
that triggered the run.
Two common deployment shapes hit this:
sub. That opaque identifier should appear on every downstream operation asuser.id— tool calls, A2A delegations, MCP calls, model calls.the thread, and the channel. None of it reaches the trace.
#1734 raised the second case
and was closed by #1737, which
promotes A2A
message.metadataintoa2a.message.metadata.*attributes. Thatleft two gaps:
message.metadataentirely.SetMessageMetadataAttributeswrites to the span currentat A2A entry, so descendant spans inherit nothing.
Gap 2 is the one that breaks the use case: Langfuse and comparable backends
resolve trace-level filters against the attributes present on each span, so an
attribute on the invocation span alone leaves most views unfilterable.
What this changes
An operator names the context keys they want traced. Both runtimes read those
keys from W3C Baggage and from A2A
message.metadata, sanitise them, andmerge them into the request-scoped attribute bag that
KagentAttributesSpanProcessor/kagentAttributesSpanProcessoralready stampsonto every span of the request.
Registry names (
user.*,enduser.*,session.id) are left unprefixed sooperators can use the semantic convention names. Names already in the
kagent.namespace are left as-is. Everything else is emitted as
kagent.context.<name>.Adding a new traced value is a configuration change, not a code change.
Design rationale
Why baggage
Baggage is the vendor-neutral OTel answer to this problem, and the plumbing
already exists: the controller, both runtimes, and every instrumented HTTP client
run a composite
tracecontext + baggagepropagator. A value set once at the edgesurvives controller → agent → sub-agent → tool without kagent adding any
hop-specific mechanism, and it requires no kagent-specific knowledge from the
caller — any OTel SDK or proxy can set it.
A2A
message.metadataremains supported as the complement, for callers that canset message fields but not transport headers. Because it is scoped to one message
it is the more specific source, so it wins on conflict.
Alternatives considered and rejected: a fixed set of
user/thread/channelfields (not extensible, needs a code change per new field); custom HTTP headers
(reinvents baggage, does not cross hops); stamping only the root span (does not
satisfy the per-span requirement above).
Why the attributes go in the request-scoped bag
The bag is the only place where a value is applied by the span processor at
OnStartfor every span, including spans created by upstream ADK, the MCPclient, and the model instrumentation — code kagent does not own and cannot
instrument individually.
Why one knob instead of an enable flag plus a list
An
enabled: truewith an empty allowlist is a state that does nothing but lookslike it should. Making the allowlist itself the switch removes that state: empty
means off, non-empty means on, and a contradiction cannot be expressed.
Feature flag
KAGENT_TRACE_CONTEXT_KEYS(env) /otel.tracing.contextKeys(Helm){from, to, hash}objectsKAGENT_TRACE_CONTEXT_HASH_KEY/otel.tracing.contextHashKeySecretforhash: hmac-sha256The controller forwards both variables to the agents it creates. They need
explicit forwarding because
collectOtelEnvFromProcesscarries onlyOTEL_prefixed names.
Which caller data reaches a trace backend is cluster-wide operator policy, so the
value is applied after the
Harnessenvironment and any inherited entry of thesame name is dropped first. Without that second step a
Harnesscould enablepromotion whenever the operator had configured nothing at all; there is a test
for exactly that. The HMAC key is the same class of policy: a
Harnesscannotset or replace it.
Security considerations
Caller-supplied context is untrusted input on both paths, so promotion is
constrained on every axis:
kagent.context.; onlyuser.*,enduser.*, andsession.idpass through unprefixedHarnessenv entry is dropped, not inheritedNon-scalar metadata (objects, arrays) is skipped: unbounded in size, meaningless
as an attribute value.
Prefer an opaque identifier such as an OIDC
subforuser.id. Do not putnames or email addresses on spans
(OTel guidance).
If a stable identifier must be derived from a value that should not appear on a
span, map it with
hash: hmac-sha256ontouser.hash. Hashing at promotion timeonly affects the span: baggage travels on HTTP headers, so a value placed in
baggage is still visible to every downstream hop, including model providers and
HTTP MCP servers.
The docs state plainly that anything allowlisted is visible to everyone with
access to the trace backend, and that callers control the values.
Backwards compatibility
Fully backwards compatible.
contextKeysset, the ConfigMap key is absent, the envvar is unset, the helper returns immediately, and not a single span attribute
changes.
a2a.message.metadata.*attributes from feat(go-adk): propagate A2A message metadata as OTEL span attributes #1737 are untouched and still unconditional in the Go runtime.
go.opentelemetry.io/otelandopentelemetry-api, both already required.Runtime parity
The Go and Python implementations share the same allowlist parsing, the same
baggage-then-metadata precedence, the same limits, the same control-character
definition (Python's
_is_controldeliberately matches Go'sunicode.IsControl), rune-based rather than byte-based truncation on both sides,and the same prefix rules. Both are covered by equivalent test cases so the two
cannot drift silently.
The ADK, LangGraph, and CrewAI Python executors all promote context. Reading a
Message's protobufStructmetadata was extracted intokagent.core.a2a.read_message_metadatarather than repeated three times.Testing
Added, in both runtimes:
helper's return value)
message.metadatapathStructfloat-integer case) and non-scalar skipping
rejection, allowlist cap
service.namecannot be shadoweduser.*,enduser.*, andsession.idstay unprefixed; othersession.*keys do not{from, to}mappings onto registry andkagent.nameshash: hmac-sha256emitsuser.hashand never the original; missing key orunknown algorithm emit nothing
attribute is present on all three
Plus: Helm unittest for ConfigMap rendering of string keys and JSON mappings, and
for injecting the HMAC key from a Secret; Go tests that the controller forwards
both variables and that a
Harnesscan neither widen nor enable the allowlistnor supply the HMAC key; and a regression test that the OTel Python SDK's
default propagator still carries baggage, since the Python runtime relies on that
default rather than configuring a propagator.
Not in scope
lifecycle surface, and it is fully covered by unit tests against a real
TracerProvider. Happy to add an E2E case if maintainers would prefer one.baggage it receives. Deriving baggage from an OIDC token at the edge is the
gateway's job, not kagent's.
Docs
New
docs/architecture/trace-context.md,linked from the architecture index. Covers configuration, why baggage, the
per-span guarantee, the prefix rules, hashing, the baggage-on-HTTP-headers
warning, the safety properties, and the Collector rename recipe.
Commits
feat(adk)feat(python)feat(core)docs(architecture)fix{from, to, hash}mappings, HMAC-SHA256All commits are DCO signed off.
This follows on from #1734 / #1737 rather than starting a new discussion, but
happy to write it up as an enhancement proposal under
design/first if that ispreferred for a change of this size.