feat(tasks): add task-stream lifecycle metrics - #387
Conversation
Instrument the task-event SSE stream lifecycle with opened/closed/active/ duration/stall metrics, dual-emitted via OpenTelemetry and StatsD. Fills the gap that request-level RED cannot express: concurrency, close outcome (completed vs client disconnect vs error), and quiet-stream stalls. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
statsd increment/decrement submit a COUNT, so the active metric rendered as a rate of change rather than the live concurrency level. DogStatsD does not honor gauge deltas, so track a process-local running total and report it via statsd.gauge as an absolute value. The OTel UpDownCounter path already handled this natively and is unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
A process-local gauge is wrong under multiple workers: DogStatsD gauges are last-write-wins per flush, so N workers emitting their own counts make the metric flap between workers instead of summing to true concurrency. Remove the StatsD active gauge and its bookkeeping; the OTel UpDownCounter already sums correctly across per-instance series and remains the sole concurrency source. The additive StatsD emits (opened/closed counters, duration, stall) aggregate correctly across workers and are unchanged. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Move record_stream_opened() inside the try/finally and gate record_stream_closed on an `opened` flag, so a failure between marking the stream open and entering the try can no longer leave the active gauge permanently over-counted. Also tag the duration histogram with `outcome` (matching the closed counter) so stream lifetime can be sliced by completed / client_disconnect / error on both the OTel and StatsD paths. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
| if _STATSD_ENABLED: | ||
| statsd.increment("agentex.task_stream.closed", tags=[f"outcome:{outcome}"]) | ||
| # Datadog histograms conventionally take milliseconds for durations. | ||
| statsd.histogram( | ||
| "agentex.task_stream.duration", | ||
| duration_seconds * 1000, | ||
| tags=[f"outcome:{outcome}"], | ||
| ) | ||
| # No StatsD "active" gauge — concurrency is OTel-only; a per-worker | ||
| # DogStatsD gauge would flap rather than sum (see record_stream_opened). |
There was a problem hiding this comment.
Given that these are new metrics, and in the final state we want otel to emit to datadog instead of emitting to dd directly, i think we can remove the double emission here.
…2a-task-stream-lifecycle-metrics # Conflicts: # agentex/src/domain/use_cases/streams_use_case.py
These are new metrics and the target state routes OTel to Datadog through the collector, so the parallel DogStatsD emission was a redundant second copy of every point. Remove the StatsD path (and its os/datadog imports) and update the tests to cover the OTel-only and no-op paths. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
| Agent directly — so a second, DogStatsD-native copy of every point would just be | ||
| redundant. | ||
|
|
||
| **Why this exists (see AGX1-616/AGX1-618):** HTTP request-level RED for |
There was a problem hiding this comment.
nit: this repo is public and we keep internal ticket IDs out of anything pushed, including docstrings. Suggest dropping the parenthetical ticket reference, the rationale text stands on its own.
| unit="{stream}", | ||
| ) | ||
| _duration_histogram = meter.create_histogram( | ||
| name="agentex.task_stream.duration", |
There was a problem hiding this comment.
The duration histogram doesn't pass explicit_bucket_boundaries_advisory, so the SDK falls back to its default boundaries (0, 5, 10, 25, ... 10000), which are tuned for milliseconds. Your local test output shows it: the 81 second stream landed in the le=100 bucket, and anything past roughly 2.8 hours goes to +Inf. rpc_metrics.py and db_metrics.py hit the same issue and pass explicit boundaries in seconds. Since this is now the only duration signal, suggest a seconds scale advisory suited to stream lifetimes, for example (1, 5, 15, 30, 60, 120, 300, 600, 1800, 3600, 7200, 14400). The exact values matter less than being seconds scale.
…ring Public repo — keep internal ticket IDs out of pushed content. The rationale text stands on its own. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
The stream-lifetime histogram records seconds but inherited the SDK's millisecond-scale default buckets, so sub-10s streams collapsed into one bucket and anything past ~2.8h overflowed to +Inf. Pass a seconds-scale explicit_bucket_boundaries_advisory (1s..4h), matching rpc_metrics.py and db_metrics.py. Co-Authored-By: Claude Opus 4.7 <noreply@anthropic.com>
|
@greptileai review |
Summary
Adds stream-lifecycle instrumentation for the task-event SSE stream (
GET /tasks/{task_id}/stream). HTTP request-level RED is already covered by auto-instrumentation — but a single long-lived SSE request emits exactly one duration sample at connection close, which says nothing about what happened during the stream's life. This fills that gap with five hand-instrumented series:agentex_task_stream_opened_total— streams openedagentex_task_stream_closed_total{outcome}— streams closed, taggedcompleted/client_disconnect/erroragentex_task_stream_duration_seconds— stream lifetime histogramagentex_task_stream_active— currently-open streams (gauge)agentex_task_stream_stall_total— streams that went idle (no event pushed) past a configurable thresholdLocal Testing:
Details
src/utils/stream_metrics.pymirrors the dual-emit pattern incache_metrics.py: records through the OpenTelemetry SDK when an OTLP endpoint is configured, emits StatsD when the Datadog Agent host is set, and is a cheap no-op otherwise. Every emit path is wrapped so an instrumentation fault can never disrupt the live SSE path.outcomelabel (no ids, nohttp_route), so cardinality stays flat.try/finallyinstream_task_eventsso the active gauge stays balanced even if setup raises. Stall detection tracks last data event separately from keepalive pings, so a persistently quiet stream still trips the stall signal while the connection stays alive. One stall episode increments the counter once (onset), not once per idle cycle.SSE_STREAM_STALL_THRESHOLD_SECONDS(default 30) controls the stall window.Test plan
make test FILE=tests/unit/utils/test_stream_metrics.py— no-op-when-unconfigured, error-swallowing, and StatsD/OTel emission assertionsoutcomedistribution (completed/client_disconnect/error) on a real stream close🤖 Generated with Claude Code
Greptile Summary
This PR adds five hand-instrumented OTel series that fill the observability gap left by auto-instrumented HTTP RED metrics on the long-lived
GET /tasks/{task_id}/streamSSE endpoint. Each series is scoped to a boundedoutcomeattribute, so cardinality stays flat across any deployment scale.stream_metrics.py: New OTel-only module with lazily-created instruments, exception-swallowing emit functions, and clear separation from the older StatsD dual-emit path used bycache_metrics.py.streams_use_case.py: Instruments the generator using anopenedflag inside atry/finallyso the active gauge is always balanced; stall detection uses a separatelast_event_timeclock so keepalive pings don't mask real idle periods, and astalledboolean ensures each stall episode increments the counter exactly once.environment_variables.py: AddsSSE_STREAM_STALL_THRESHOLD_SECONDS(default 30 s) with consistent enum key, model field, andfrom_envwiring.Confidence Score: 5/5
Safe to merge — instrumentation is additive, all emit functions are exception-safe, and the active gauge is correctly balanced via the opened flag in try/finally.
All five metric series are wired correctly, the outcome attribute stays bounded to three values, the opened guard prevents double-decrementing the active gauge, and stall detection uses a separate clock so keepalive pings do not mask real idle periods. Unit tests cover the no-op, error-swallowing, and emit paths. No metric cardinality violations found.
Files Needing Attention: No files require special attention.
Important Files Changed
Sequence Diagram
sequenceDiagram participant Client participant StreamsUseCase participant stream_metrics participant OTel Client->>StreamsUseCase: "GET /tasks/{id}/stream" Note over StreamsUseCase: stream_start_time = loop.time() StreamsUseCase->>stream_metrics: record_stream_opened() stream_metrics->>OTel: opened_counter +1, active_updown +1 StreamsUseCase-->>Client: data connected loop Live polling StreamsUseCase->>StreamsUseCase: read_messages() alt Messages received StreamsUseCase-->>Client: data event Note over StreamsUseCase: last_event_time=now, stalled=False else Idle alt stall threshold exceeded StreamsUseCase->>stream_metrics: record_stream_stall() stream_metrics->>OTel: stall_counter +1 end StreamsUseCase-->>Client: :ping end end alt Normal completion StreamsUseCase->>stream_metrics: record_stream_closed(completed, duration) else Client disconnect StreamsUseCase->>stream_metrics: record_stream_closed(client_disconnect, duration) else Error StreamsUseCase->>stream_metrics: record_stream_closed(error, duration) end stream_metrics->>OTel: closed_counter, duration_histogram, active_updown -1Reviews (8): Last reviewed commit: "fix(tasks): set seconds-scale buckets on..." | Re-trigger Greptile