Skip to content

Add request trace classification and pseudonymous ids - #203

Merged
cdgamarose-nv merged 5 commits into
NVIDIA-AI-Blueprints:developfrom
AjayThorve:athorve/trace-request-classification
Apr 24, 2026
Merged

cdgamarose-nv merged 5 commits into
NVIDIA-AI-Blueprints:developfrom
AjayThorve:athorve/trace-request-classification

Conversation

@AjayThorve

Copy link
Copy Markdown
Member

Summary

  • add always-on request trace classification tags plus optional pseudonymous user and client ids in aiq_api auth middleware
  • support explicit low-cardinality access channel tagging and have the public UI proxy routes send ui
  • document the new env vars and observability contract and cover the middleware behavior with auth tests

Testing

  • uv run pytest frontends/aiq_api/tests/test_auth.py -k "tags_active or common_tags or missing_secret or explicit_access_channel or client_ip_mode or does_not_tag_active_span or none_mode"

@greptile-apps

greptile-apps Bot commented Apr 24, 2026

Copy link
Copy Markdown
Contributor

Greptile Summary

This PR adds always-on request classification tags (aiq.caller.type, aiq.auth.transport, aiq.auth.verified, aiq.access.channel) plus opt-in HMAC-pseudonymous user identity and client IP tags to the aiq_api auth middleware, backed by five new environment variables and well-scoped test coverage. The UI proxy routes now uniformly inject X-AIQ-Access-Channel: ui via a newly shared constant, replacing per-file duplication.

The implementation is generally clean, but two P2 concerns are worth addressing before relying on the privacy properties in production:

  • _load_trace_client_id_secret silently falls back to the user-identity HMAC secret, linking both pseudonymous dimensions under one key without any warning log.
  • _extract_client_ip takes the leftmost (client-supplied) entry from X-Forwarded-For, which is forgeable without a trusted reverse proxy; neither the docs nor the .env.example flag this dependency.

Confidence Score: 5/5

Safe to merge; findings are P2 quality-of-life and documentation gaps, not runtime defects.

All findings are P2: a missing warning log on shared-secret fallback and an undocumented trusted-proxy dependency for X-Forwarded-For IP extraction. No data loss, auth bypass, or correctness bugs were identified.

frontends/aiq_api/src/aiq_api/auth/utils.py — shared secret fallback and IP header trust model.

Important Files Changed

Filename Overview
frontends/aiq_api/src/aiq_api/auth/utils.py New utility module for trace enrichment; HMAC pseudonymization is correct, but silent fallback to user-identity secret when client-ID secret is absent can link both pseudonymous dimensions under one key.
frontends/aiq_api/src/aiq_api/auth/middleware.py Config loading and _call_app wiring look correct; trust model for X-AIQ-Access-Channel (trust_access_channel_override) properly restricts external anonymous callers.
frontends/aiq_api/tests/test_auth.py Comprehensive coverage of new trace tagging paths; one test name (test_unverified_internal_token_does_not_tag_active_span) is misleading since common tags are in fact asserted.
frontends/ui/src/adapters/auth/constants.ts New shared constant file consolidating ACCESS_CHANNEL_HEADER, resolving the previous duplication across three route files.
frontends/ui/src/app/api/chat/route.ts Imports shared constant and injects X-AIQ-Access-Channel: ui header correctly.
frontends/ui/src/app/api/jobs/async/[...path]/route.ts Injects ui channel header in both the auth-required and no-auth branches correctly.
frontends/ui/src/app/api/v1/[...path]/route.ts Same pattern as other routes: ui channel header injected in both auth paths.

Sequence Diagram

sequenceDiagram
    participant Client
    participant UI as Next.js UI Proxy
    participant MW as AuthMiddleware
    participant Utils as auth/utils.py
    participant Tracer as DD/OTEL Tracer

    Client->>UI: HTTP request
    UI->>MW: Forward + X-AIQ-Access-Channel: ui
    MW->>MW: resolve_request_user()
    MW->>Utils: attach_request_to_active_trace(headers, scope, user, ...)
    Utils->>Utils: _build_common_trace_tags() → caller.type, transport, verified, channel
    Utils->>Utils: _build_trace_user_tags() → enduser.id (HMAC) if mode≠none
    Utils->>Utils: _build_pseudonymous_trace_client_id() if client_id_mode=ip
    Utils->>Tracer: _tag_current_ddtrace_span(tags)
    Utils->>Tracer: _tag_current_otel_span(tags)
    MW->>MW: user_context(user) → call downstream app
Loading

Reviews (5): Last reviewed commit: "restrict access channel overrides" | Re-trigger Greptile

Comment thread frontends/aiq_api/src/aiq_api/auth/middleware.py Outdated
Comment thread frontends/ui/src/app/api/chat/route.ts Outdated
Comment thread frontends/aiq_api/src/aiq_api/jobs/access.py
Comment thread frontends/aiq_api/src/aiq_api/websocket_reconnect.py
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
@AjayThorve
AjayThorve force-pushed the athorve/trace-request-classification branch from a411dac to 862c491 Compare April 24, 2026 17:29
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
@cdgamarose-nv

Copy link
Copy Markdown
Collaborator

Do we need to add the placeholder variables to .env.example?

Also, nitpick but the middleware.py file is now very long with many helper functions. Would it make sense to move into a utils.py?

@cdgamarose-nv

Copy link
Copy Markdown
Collaborator

But otherwise, looks good to me

Signed-off-by: Ajay Thorve <athorve@nvidia.com>
Signed-off-by: Ajay Thorve <athorve@nvidia.com>
@AjayThorve

Copy link
Copy Markdown
Member Author

@cdgamarose-nv good suggestions, update .env.example and refactored to add utils.py

Signed-off-by: Ajay Thorve <athorve@nvidia.com>

@cdgamarose-nv cdgamarose-nv left a comment

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

LGTM

@cdgamarose-nv
cdgamarose-nv merged commit 86f075b into NVIDIA-AI-Blueprints:develop Apr 24, 2026
9 checks passed
taylorjordanNC pushed a commit to taylorjordanNC/rh-research that referenced this pull request May 27, 2026
…prints#203)

* add request trace classification and pseudonymous ids

Signed-off-by: Ajay Thorve <athorve@nvidia.com>

* harden trace user tagging guards

Signed-off-by: Ajay Thorve <athorve@nvidia.com>

* document trace env examples

Signed-off-by: Ajay Thorve <athorve@nvidia.com>

* extract auth trace helpers into utils

Signed-off-by: Ajay Thorve <athorve@nvidia.com>

* restrict access channel overrides

Signed-off-by: Ajay Thorve <athorve@nvidia.com>

---------

Signed-off-by: Ajay Thorve <athorve@nvidia.com>
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.

2 participants