Skip to content

adding app id to spans#2779

Merged
shagun-singh-inkeep merged 1 commit intomainfrom
TI-36
Mar 19, 2026
Merged

adding app id to spans#2779
shagun-singh-inkeep merged 1 commit intomainfrom
TI-36

Conversation

@shagun-singh-inkeep
Copy link
Collaborator

No description provided.

@vercel
Copy link

vercel bot commented Mar 19, 2026

The latest updates on your projects. Learn more about Vercel for GitHub.

Project Deployment Actions Updated (UTC)
agents-api Ready Ready Preview, Comment Mar 19, 2026 9:25pm
agents-docs Ready Ready Preview, Comment Mar 19, 2026 9:25pm
agents-manage-ui Ready Ready Preview, Comment Mar 19, 2026 9:25pm

Request Review

@changeset-bot
Copy link

changeset-bot bot commented Mar 19, 2026

🦋 Changeset detected

Latest commit: 113ca0d

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 10 packages
Name Type
@inkeep/agents-api Patch
@inkeep/agents-manage-ui Patch
@inkeep/agents-cli Patch
@inkeep/agents-core Patch
@inkeep/agents-email Patch
@inkeep/agents-mcp Patch
@inkeep/agents-sdk Patch
@inkeep/agents-work-apps Patch
@inkeep/ai-sdk-provider Patch
@inkeep/create-agents Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

@pullfrog
Copy link
Contributor

pullfrog bot commented Mar 19, 2026

TL;DR — Adds the app.id attribute to the active OpenTelemetry span during run API key authentication, making it possible to filter and correlate traces by application.

Key changes

  • runAuth.ts — Sets app.id on the active span in both the dev-context and standard auth paths of runApiKeyAuthHandler, gated on reqData.appId being present.

Summary | 2 files | 1 commit | base: mainTI-36


app.id span attribute in runApiKeyAuthHandler

Before: Traces from the run API had no app.id attribute, making it impossible to filter spans by application.
After: When reqData.appId is present, app.id is set on the active span via trace.getActiveSpan()?.setAttribute(...) in both the dev-context early-return path and the standard auth path.

The attribute is added after authentication succeeds but before next() is called, so downstream middleware and handlers inherit the span context. The dev-context path additionally checks attempt.authResult to avoid attributing spans for unauthenticated dev requests.

agents-api/src/middleware/runAuth.ts

Pullfrog  | View workflow run | Triggered by Pullfrogpullfrog.com𝕏

Copy link
Contributor

@pullfrog pullfrog bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small, focused change — adds app.id to the active span in the two runApiKeyAuthHandler paths. The naming convention (app.id) matches the existing pattern (tenant.id, agent.id, project.id). One minor inconsistency worth addressing.

Pullfrog  | Fix all ➔Fix 👍s ➔View workflow runpullfrog.com𝕏

Comment on lines +712 to +714
if (reqData.appId && attempt.authResult) {
trace.getActiveSpan()?.setAttribute('app.id', reqData.appId);
}
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The dev-environment guard checks attempt.authResult in addition to reqData.appId, but the production path (line 764) only checks reqData.appId. In the dev fallback branch (line 702–709), when appId is present but auth fails, the span won't get the attribute — even though the request is clearly associated with that app.

Since reqData.appId is the only thing that matters for attributing the span, consider dropping the attempt.authResult check to match the production path:

Suggested change
if (reqData.appId && attempt.authResult) {
trace.getActiveSpan()?.setAttribute('app.id', reqData.appId);
}
if (reqData.appId) {
trace.getActiveSpan()?.setAttribute('app.id', reqData.appId);
}

"@inkeep/agents-api": patch
---

adding app.id to span attributes
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: changelog messages should start with a capitalized action verb per the repo conventions (e.g. "Add app.id to span attributes").

Suggested change
adding app.id to span attributes
Add `app.id` to span attributes

Copy link
Contributor

@claude claude bot left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

PR Review Summary

(0) Total Issues | Risk: Low

This PR adds the app.id attribute to OpenTelemetry spans during run API authentication, enabling trace filtering and correlation by application. The implementation is clean, correct, and follows established patterns.

🟢 What's Good

  • Correct pattern usage: Uses trace.getActiveSpan()?.setAttribute() consistent with existing telemetry code
  • Proper naming: Follows the {resource}.id convention (tenant.id, project.id, agent.id)
  • Low cardinality: App IDs are admin-created credentials, not user-generated, so this won't cause cardinality explosion
  • No PII exposure: The attribute is an opaque identifier
  • Intentional guard logic: The different conditions between dev (&& attempt.authResult) and prod paths are correct — prod throws earlier on null auth, while dev should only attribute spans when real auth succeeded (not dev fallback)

💭 Consider (C) 💭

Inline Comments:

  • 💭 Consider: runAuth.ts:713 — Optionally add APP_ID to SPAN_KEYS constants for consistency (though mixed patterns exist in codebase)

✅ APPROVE

Summary: Clean, focused observability improvement. The code is correct, follows established patterns, and enables useful trace filtering by app. Ship it! 🚀

Reviewers (3)
Reviewer Returned Main Findings Consider While You're Here Inline Comments Pending Recs Discarded
pr-review-standards 0 0 0 0 0 0 0
pr-review-sre 0 0 0 0 0 0 0
pr-review-consistency 2 0 0 0 1 0 1
Total 2 0 0 0 1 0 1
Discarded (1)
Location Issue Reason Discarded
runAuth.ts:712,764 Inconsistent guard conditions between dev/prod paths Analyzed and found to be intentional — prod path throws before line 764 if authResult is null, while dev path needs the extra check to avoid attributing spans when using fallback dev context

}

if (reqData.appId && attempt.authResult) {
trace.getActiveSpan()?.setAttribute('app.id', reqData.appId);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💭 Consider: Add app.id to SPAN_KEYS constants

Issue: The new 'app.id' attribute is defined as a string literal, while the codebase has an established pattern of centralizing span attribute keys in SPAN_KEYS in packages/agents-core/src/constants/otel-attributes.ts.

Why: Centralizing attribute names helps maintain a consistent telemetry schema and makes it easier to audit what attributes are being emitted. However, I note the codebase has some mixed patterns (e.g., chat.ts also uses inline strings like 'user.type'), so this is a suggestion rather than a requirement.

Fix: Optionally add to otel-attributes.ts:

APP_ID: 'app.id',

Then import and use SPAN_KEYS.APP_ID here.

Refs:

@github-actions github-actions bot deleted a comment from claude bot Mar 19, 2026
@shagun-singh-inkeep shagun-singh-inkeep added this pull request to the merge queue Mar 19, 2026
Merged via the queue into main with commit 65c151d Mar 19, 2026
26 of 29 checks passed
@shagun-singh-inkeep shagun-singh-inkeep deleted the TI-36 branch March 19, 2026 22:01
dimaMachina pushed a commit that referenced this pull request Mar 20, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant