Getting trace id from context

[shagun-singh-inkeep]

No description provided.

[vercel]

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

Project	Deployment	Preview	Comments	Updated (UTC)
agents-manage-api	Ready	Preview	Comment	Sep 15, 2025 8:54pm
agents-manage-ui	Ready	Preview	Comment	Sep 15, 2025 8:54pm
agents-run-api	Ready	Preview	Comment	Sep 15, 2025 8:54pm

1 Skipped Deployment

Project	Deployment	Preview	Comments	Updated (UTC)
agents-docs	Skipped			Sep 15, 2025 8:54pm

[claude]

Claude finished @shagun-singh-inkeep's task —— View job

---

PR Review: Getting trace id from context

Review Progress:

• Read and understand the changes in agents-run-api/src/routes/chatDataStream.ts
• Research existing tracing patterns and context handling in the codebase
• Analyze changes for correctness, consistency, and architectural considerations
• Provide detailed review feedback covering engineering quality and system design

---

🔍 Code Review Summary

Changes: This PR adds 4 lines of code to expose the OpenTelemetry trace ID as an HTTP response header trace-id in the /chat streaming endpoint.

const spanContext = trace.getActiveSpan()?.spanContext();
if (spanContext) {
  c.header('trace-id', spanContext.traceId);
}

✅ Strengths

1. Defensive Programming: Proper null safety with optional chaining and conditional check
2. Integration with Existing Infrastructure: Leverages already-configured OpenTelemetry tracing
3. Clear Purpose: Enables client-side correlation with distributed traces for debugging

⚠️ Architectural & Design Concerns

1. Inconsistent API Surface (High Priority)

• Issue: Only the /chat (data stream) endpoint exposes trace IDs, but not the /completions endpoint
• Impact: Creates inconsistent debugging experience across similar APIs
• Recommendation: Apply the same pattern to /completions and other user-facing endpoints for consistency

2. HTTP Header Naming Convention (Medium Priority)

• Issue: Uses trace-id instead of standard X-Trace-Id convention
• Context: Standard observability headers typically use X- prefix (like X-Request-Id)
• Risk: Non-standard naming may cause integration issues with observability tools
• Recommendation: Consider X-Trace-Id or follow existing header patterns in your organization

3. Missing Documentation (Medium Priority)

• Issue: No OpenAPI schema documentation for the new response header
• Impact: Clients won't know about this header's availability
• Current Schema: Lines 75-79 define response headers but don't include trace-id
• Recommendation: Add header to OpenAPI specification:

headers: z.object({
  'Content-Type': z.string().default('text/plain; charset=utf-8'),
  'x-vercel-ai-data-stream': z.string().default('v1'),
  'trace-id': z.string().optional().describe('OpenTelemetry trace ID for request correlation'),
}),

4. System Design Considerations (Medium Priority)

• Extensibility: Consider if other span context data might be useful (span ID, trace flags)
• Configuration: Should trace ID exposure be configurable? (e.g., disabled in production for security)
• Performance: Minimal impact, but accessing active span on every request - acceptable for this use case

5. Security Considerations (Low Priority)

• Information Exposure: Trace IDs are generally safe to expose but consider if they reveal timing/usage patterns
• Current Risk: Low - trace IDs are designed to be shareable identifiers

🔧 Recommended Actions

Immediate (This PR):

1. Add OpenAPI Documentation: Update response schema to document the new header
2. Consider Header Naming: Evaluate using X-Trace-Id for better standards compliance

Follow-up (Separate PR):

1. Apply Pattern Consistently: Add same trace ID header to /completions endpoint
2. Add Configuration: Consider environment-based toggle for trace ID exposure
3. Update Client Libraries: If you have SDK/client libraries, update them to utilize this header

💡 Alternative Approaches Considered

• Trace ID in Response Body: Less standard, would require protocol changes
• Custom Header with Tenant Context: Overkill for this use case
• Multiple Trace Headers: Could include span ID too, but trace ID is sufficient for most debugging

🎯 Overall Assessment

Verdict: ✅ Approve with recommendations

The implementation is technically sound and adds valuable debugging capability. The code quality is good with proper error handling. Main concerns are around API consistency and documentation completeness rather than correctness.

Priority for addressing: Medium - not blocking, but would benefit from the consistency improvements mentioned above.

---