[FE / chore] Move tracing away from legacy endpoints#4573
Open
ardaerzin wants to merge 6 commits into
Open
Conversation
…-3788)
Move the web tracing calls (sessions, delete, single-trace, flat-span,
batch-trace + rate-limit meta) from the deprecated /tracing/* stack onto the
Fern /spans/* and /traces/* endpoints under @agenta/entities, keeping Zod
validation at the boundary.
- New scaffolding: trace/api/{client,request,adapters}.ts + envelope Zod schemas
- Repoint OSS drawers, observability + sessions consumers to @agenta/entities/trace
- Flip span/trace_type enum catch-all "undefined" -> "unknown" (backend value)
- Fix sessions enrichment grouping for the new endpoints' canonicalized ag.* attrs
- Add unit + integration tests for the migrated functions and adapters
Only /tracing/spans/analytics remains (Phase 6, gated on the MetricSpec contract).
…e-tracing-away-from-deprecated-and-then-legacy-endpoints
…endpoint (AGE-3788) The ETL hydration + mapping resolver now read traces via the migrated fetchAllPreviewTraces (Fern queryTraces, POST /traces/query) adapted to the legacy map shape, not the deprecated POST /tracing/spans/query. Comment-only.
…ry (AGE-3788) Move the observability generations dashboard off the deprecated POST /tracing/spans/analytics onto the Fern client's traces.querySpansAnalytics (POST /spans/analytics/query). - entities: add fetchSpansAnalytics + analyticsResponseSchema/metricsBucketSchema. Omit specs so the backend applies DEFAULT_ANALYTICS_SPECS; filter is a JSON-string query param; project/app scope rides queryParams. - oss: rewrite fetchGenerationsDashboardData to use fetchSpansAnalytics and analyticsToGeneration, which reads the spec-based metrics dict (buckets[].metrics keyed by dotted MetricSpec.path) instead of the old total/errors bucket split. Drop the last raw /tracing/* fetch. - remove the now-dead TracingDashboardData type. - add 6 unit tests for fetchSpansAnalytics.
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
📝 WalkthroughSummary by CodeRabbit
WalkthroughMigrates tracing consumers and analytics from legacy /tracing endpoints to Fern-backed traces/spans APIs: adds Zod schemas and request builders, Fern client wrapper and adapters, updates OSS UI/state to pass projectId and tag session traces, and adds unit and integration tests. ChangesFern-Based Trace API Migration
Estimated code review effort🎯 4 (Complex) | ⏱️ ~75 minutes Possibly related issues
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
ardaerzin
changed the title
ardaerzin
changed the title
dosubot
Bot
added
size:XXL
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
🧹 Nitpick comments (2)
web/packages/agenta-entities/tests/unit/trace-migration-adapters.test.ts (1)
125-129: ⚡ Quick winIncomplete test fixture could mask validation gaps.
Line 127 casts
{spans: {}}toTraceOutputwithout providing thetrace_idfield. While the current code skips entries withouttrace_id, this incomplete fixture could cause false positives if upstream schema validation tightens.✨ Strengthen the fixture
- const out = fernTracesToLegacyTraceMap([{spans: {}} as TraceOutput]) + const out = fernTracesToLegacyTraceMap([{trace_id: undefined, spans: {}} as TraceOutput])Or test the skip behavior explicitly with a proper partial:
- const out = fernTracesToLegacyTraceMap([{spans: {}} as TraceOutput]) + // Explicitly test missing trace_id behavior + const out = fernTracesToLegacyTraceMap([{spans: {}, trace_id: undefined} as unknown as TraceOutput])web/packages/agenta-entities/tests/unit/trace-migration-api.test.ts (1)
212-234: 💤 Low valueConsider stronger typing for the adapter response.
Lines 221-224 use a loose inline type that only declares
countandtracesasRecord<string, unknown>. While this works for the test, a more precise type would catch regressions in the adapter's output shape.💡 Optional: Use the actual return type
- const res = (await fetchAllPreviewTraces({focus: "trace", filter}, "", "proj-9")) as { - count: number - traces: Record<string, unknown> - } + const res = (await fetchAllPreviewTraces({focus: "trace", filter}, "", "proj-9")) as TracesResponseThen access
res.tracesknowing it matches the expected schema.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: ed419b8f-554f-4ecf-b72d-ea787f77377c
📒 Files selected for processing (22)
web/oss/src/components/SharedDrawers/SessionDrawer/store/sessionDrawerStore.tsweb/oss/src/components/SharedDrawers/TraceDrawer/components/DeleteTraceModal/index.tsxweb/oss/src/components/SharedDrawers/TraceDrawer/components/TraceHeader/index.tsxweb/oss/src/components/SharedDrawers/TraceDrawer/store/traceDrawerStore.tsweb/oss/src/services/tracing/api/index.tsweb/oss/src/services/tracing/lib/helpers.tsweb/oss/src/services/tracing/types/index.tsweb/oss/src/state/newObservability/atoms/queries.tsweb/oss/src/state/newObservability/atoms/queryHelpers.tsweb/packages/agenta-entities/src/evaluationRun/etl/hydrateScenariosTransform.tsweb/packages/agenta-entities/src/evaluationRun/etl/resolveMappings.tsweb/packages/agenta-entities/src/trace/api/adapters.tsweb/packages/agenta-entities/src/trace/api/api.tsweb/packages/agenta-entities/src/trace/api/client.tsweb/packages/agenta-entities/src/trace/api/index.tsweb/packages/agenta-entities/src/trace/api/request.tsweb/packages/agenta-entities/src/trace/core/index.tsweb/packages/agenta-entities/src/trace/core/schema.tsweb/packages/agenta-entities/src/trace/index.tsweb/packages/agenta-entities/tests/integration/trace-migration.integration.test.tsweb/packages/agenta-entities/tests/unit/trace-migration-adapters.test.tsweb/packages/agenta-entities/tests/unit/trace-migration-api.test.ts
Comment on lines
136
to
143
| return { | ||
| timestamp: formatTick(b.timestamp, range), | ||
| success_count: succC, | ||
| failure_count: errC, | ||
| cost: succCost + errCost, | ||
| latency: succC ? succDurS / Math.max(succC, 1) : 0, // avg latency per success in the bucket | ||
| total_tokens: succTok + errTok, | ||
| success_count: success, | ||
| failure_count: failure, | ||
| cost, | ||
| latency: durationCount ? durationS / durationCount : 0, // avg latency in the bucket | ||
| total_tokens: tokens, | ||
| } |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Check if any code accesses the missing fields from the dashboard data
rg -n -C2 'prompt_tokens|completion_tokens|enviornment|variant' --type=ts --type=tsx -g '!**/types/**' web/oss/src/components/pages/observability/Repository: Agenta-AI/agenta
Length of output: 88
🏁 Script executed:
#!/bin/bash
# Check GenerationDashboardData interface definition and usages
rg -n 'GenerationDashboardData' --type=ts web/oss/src/Repository: Agenta-AI/agenta
Length of output: 1236
Fix analyticsToGeneration bucket shape to match GenerationDashboardData.data[] fields.
analyticsToGeneration currently returns only { timestamp, success_count, failure_count, cost, latency, total_tokens }, but GenerationDashboardData.data requires prompt_tokens, completion_tokens, enviornment, and variant; the as GenerationDashboardData cast in web/oss/src/services/tracing/api/index.ts masks this mismatch at compile time. If the observability UI reads these fields, it will get undefined; either populate them (with defaults) in analyticsToGeneration or update the interface to reflect reality.
CodeRabbit follow-ups (quick wins; no behavior change): - toFilteringInput: reject array-shaped filters (typeof [] === 'object') before casting to FilteringInput. - GenerationDashboardData: make the never-populated, unread prompt_tokens / completion_tokens / enviornment / variant fields optional in both the services/tracing and types_ee copies, and drop the masking 'as GenerationDashboardData' cast (annotate analyticsToGeneration's return). - integration test: parse AGENTA_TEST_EXPECT_RATELIMIT explicitly so 'false'/'0' don't enable the EE-only assertion path. - golden-convergence test: replace 'as never' with 'as unknown as TracesResponse'; tighten the focus=trace adapter assertion to TracesResponse. - adapters test: exercise the missing-trace_id skip path explicitly.
Contributor
Railway Preview Environment
|
…on tests (AGE-3788) Verified against live project data via a gated integration test: ag.metrics.duration.cumulative is in MILLISECONDS (avg ~1229ms across 8.5k traces), but analyticsToGeneration divided by 1000 while the dashboard renders latency with an 'ms' suffix — so avg/per-bucket latency displayed 1000x too small (1229ms shown as '1.23ms'). The legacy transform had the same bug on the same attribute; this corrects it now that real data confirms the unit. Also adds /spans/analytics/query integration coverage: - shape smoke (gated on hasBackend) — proves the Fern call + envelope parse. - live-data assertions (gated on AGENTA_LIVE_* env, no secrets committed) — prove the default-spec dotted metric paths the OSS transform reads are present with numeric aggregates.
There was a problem hiding this comment.
Actionable comments posted: 1
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 583389c6-287e-4459-a7fd-44231d6140a8
📒 Files selected for processing (2)
web/oss/src/services/tracing/lib/helpers.tsweb/packages/agenta-entities/tests/integration/trace-migration.integration.test.ts
🚧 Files skipped from review as they are similar to previous changes (1)
- web/oss/src/services/tracing/lib/helpers.ts
Comment on lines
+215
to
+219
| beforeAll(() => { | ||
| process.env.AGENTA_HOST = LIVE_API_URL | ||
| process.env.AGENTA_API_KEY = LIVE_API_KEY | ||
| getAgentaSdkClient({host: LIVE_API_URL, apiKey: LIVE_API_KEY}) | ||
| }) |
There was a problem hiding this comment.
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
# Description: Check if getAgentaSdkClient supports re-initialization or reads env vars on each call
# Find the SDK client implementation to understand singleton behavior
rg -nP --type=ts -C5 'export.*getAgentaSdkClient' --glob '!**/node_modules/**'
# Check if the client reads from env vars on each call or caches the first config
rg -nP --type=ts -C5 'AGENTA_HOST|AGENTA_API_KEY' --glob '**/sdk/**' --glob '!**/node_modules/**'Repository: Agenta-AI/agenta
Length of output: 13190
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="web/packages/agenta-entities/tests/integration/trace-migration.integration.test.ts"
echo "---- Context: ${FILE} (lines 1-140) ----"
sed -n '1,140p' "$FILE" | nl -ba | sed -n '1,140p'
echo "---- Context: ${FILE} (lines 180-280) ----"
sed -n '180,280p' "$FILE" | nl -ba
echo "---- Find getTracesClient usage in the test ----"
rg -n "getTracesClient|getAgentaSdkClient|init\(" "$FILE"
echo "---- Locate getTracesClient implementation ----"
rg -n "function getTracesClient|getTracesClient\s*\(" web/packages -S --glob '!**/node_modules/**' | head -n 50Repository: Agenta-AI/agenta
Length of output: 237
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="web/packages/agenta-entities/tests/integration/trace-migration.integration.test.ts"
echo "---- Context: ${FILE} (lines 1-220) ----"
if [ -f "$FILE" ]; then
cat -n "$FILE" | sed -n '1,220p'
else
echo "File not found: $FILE"
exit 1
fi
echo "---- Context: ${FILE} (lines 220-320) ----"
cat -n "$FILE" | sed -n '220,320p' | head -n 120
echo "---- Find getTracesClient usage in the test ----"
rg -n "getTracesClient|getAgentaSdkClient|AGENTA_HOST|AGENTA_API_KEY|init\(" "$FILE" || true
echo "---- Locate getTracesClient implementation ----"
rg -n "getTracesClient" web/packages --glob '!**/node_modules/**' | head -n 50 || true
echo "---- Locate SDK init/entrypoint used by traces helpers ----"
rg -n "getAgentaSdkClient\(" web/packages --glob '!**/node_modules/**' | head -n 50 || trueRepository: Agenta-AI/agenta
Length of output: 18686
🏁 Script executed:
#!/bin/bash
set -euo pipefail
FILE="web/packages/agenta-sdk/src/index.ts"
echo "---- File: $FILE ----"
wc -l "$FILE"
cat -n "$FILE" | sed -n '1,160p'Repository: Agenta-AI/agenta
Length of output: 4391
Fix live-data Phase 6 test to avoid stale getAgentaSdkClient singleton config.
getAgentaSdkClient is a lazy singleton that ignores subsequent arguments once _singleton is set (web/packages/agenta-sdk/src/index.ts). In trace-migration.integration.test.ts, the top-level beforeAll seeds the singleton with TEST_CONFIG (lines 64-69), so the Phase 6 “real data” beforeAll calling getAgentaSdkClient({host: LIVE_API_URL, apiKey: LIVE_API_KEY}) (lines 215-219) won’t switch the backend to LIVE.
Adjust the test so the first SDK initialization uses LIVE creds when hasLiveAnalytics is true (or route fetchSpansAnalytics through an init(LIVE...) client / add a way to reset the singleton between configurations).
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
1 participant
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.
Summary
Testing
Verified locally
Added or updated tests
QA follow-up
Demo
Checklist
Contributor Resources