Skip to content

Commit 80763f7

Browse files
feat(observability): RPC + workflow + DB-pool metrics for the agent JSON-RPC endpoint (#379)
## Summary Brings the agent JSON-RPC endpoint (`POST /agents/{id}/rpc` — `task/create` / `message/send` / `event/send`) up to the metrics observability contract, and adds high-signal Postgres connection-pool saturation metrics. Everything here is OTel-only (no StatsD dual-emit — these are new series with no existing consumer), and a cheap no-op when no OTLP endpoint is configured. ### RPC RED metrics (`agentex.rpc.*`) - `agentex.rpc.request.duration` (timed dispatch → final byte, so streaming responses are covered end-to-end), `agentex.rpc.requests`, `agentex.rpc.errors`. - Attributes follow the OTel RPC semantic conventions: `rpc.system`, `rpc.method`, `rpc.jsonrpc.error_code`, `error.type`, `streaming`. No high-cardinality identifiers (no task/agent/request ids). JSON-RPC failures return HTTP 200, so the error code rides on `rpc.jsonrpc.error_code` (set only on failure), not an HTTP status. - Never-raise emission; a single terminal emit per request covering the whole stream. ### Workflow-level GenAI metric - `gen_ai.workflow.duration` for `task/create`, labeled `gen_ai.operation.name=invoke_workflow`, so workflow-level duration is directly queryable instead of only inferable from RPC duration. `invoke_agent` stays reserved for the agent loop's own duration, so the two operation names never mix gateway and in-pod durations. ### DB connection-pool metrics (OTel DB semconv) - `db.client.connection.wait_time`, `db.client.connection.pending_requests`, `db.client.connection.timeouts` — the pre-outage early-warning signals for pool exhaustion. - Sourced from an `InstrumentedAsyncAdaptedQueuePool` that wraps `Pool.connect()` (the single, non-recursive acquisition entry point, run inside the acquiring greenlet). The existing checkout/checkin listeners can't produce these: by the time `checkout` fires the wait is over, there's no waiter count, and a pool timeout raises before any connection is handed out. - `wait_time` uses explicit second-scale histogram buckets so quantiles stay usable (the SDK default boundaries assume a millisecond unit). ## Testing - Unit tests in `tests/unit/utils/test_rpc_metrics.py` and `test_db_metrics.py`: unconfigured no-op, never-raise on backend faults, metric name/attribute assertions, and the pool success / timeout / non-timeout-error paths. All green; `ruff` clean. ## Notes / follow-ups - Requires an OTLP endpoint configured on the pods to export. - HTTP auto-instrumentation (route-templated `http_server_request_duration_seconds`) lands on a separate branch. - Dashboard/Mimir verification and matrix backfill to follow once the target environment is available. 🤖 Generated with [Claude Code](https://claude.com/claude-code) <!-- greptile_comment --> <h3>Greptile Summary</h3> This PR adds OTel-only RED metrics for the agent JSON-RPC endpoint (`agentex.rpc.*`), a GenAI workflow-duration histogram for `task/create`, and Postgres connection-pool acquisition metrics (`db.client.connection.wait_time/pending_requests/timeouts/failures`) sourced from a new `InstrumentedAsyncAdaptedQueuePool` subclass. It also removes the `namespace: agentex` from the local OTel collector's Prometheus exporter to fix a double-prefix bug that would have appeared for new `agentex.*`-prefixed instrument names. - **RPC metrics**: A synchronous `@contextmanager` (`rpc_request_timing`) wraps both sync and streaming handlers; a `RpcCallOutcome` dataclass lets handlers classify JSON-RPC error codes, with `BaseException` catching client disconnects (e.g., `GeneratorExit`) separately from structured RPC errors. Never-raise contract is enforced throughout. - **DB pool metrics**: `InstrumentedAsyncAdaptedQueuePool.connect()` is the correct interception seam (checkout/checkin events can\u2019t see the wait or a timeout), instruments are attached post-construction and carried across `recreate()`, and the `wait_time` histogram uses explicit second-scale buckets to keep quantiles usable. - **Collector config**: Removing `namespace: agentex` aligns local PromQL with the production Mimir pipeline but renames existing `agentex_db_*` local series to `db_*`; teams with local Grafana dashboards querying the old prefixed names will need to update their queries. <details><summary><h3>Confidence Score: 5/5</h3></summary> Safe to merge; all changes are additive instrumentation with a never-raise emission contract and no mutations to the core RPC execution path. The RPC and DB pool metric additions are purely observational. The rpc_request_timing contextmanager correctly handles BaseException including GeneratorExit, the pool override degrades to a passthrough when OTel is unconfigured, and recreate() carries instrumentation forward. Unit tests cover the no-op path, never-raise contract, double-count regression, and the streaming GeneratorExit scenario. **Files Needing Attention:** No files require special attention. The otel-collector-config.yaml namespace removal warrants a local dashboard audit if Prometheus queries exist against the old agentex_db_*-prefixed series. </details> <details><summary><h3>Important Files Changed</h3></summary> | Filename | Overview | |----------|----------| | agentex/src/utils/rpc_metrics.py | New module: RPC RED metrics and workflow-duration histogram with correct OTel semconv, never-raise emission, and single-record-per-request guarantee via contextmanager. | | agentex/src/utils/db_metrics.py | Adds InstrumentedAsyncAdaptedQueuePool and three new OTel connection-acquisition metrics; pending_requests increment occurs outside the try block, so an OTel SDK raise there leaves the counter unbalanced without a compensating decrement. | | agentex/src/api/routes/agents.py | Wraps sync and streaming RPC handlers with rpc_request_timing; streaming handler correctly carries instrumentation across yields and GeneratorExit. | | agentex/src/config/dependencies.py | Adds poolclass=InstrumentedAsyncAdaptedQueuePool to all three engine constructors; straightforward wiring change. | | agentex/otel/otel-collector-config.yaml | Removes namespace: agentex from Prometheus exporter to prevent double-prefixing for agentex.* metrics; renames existing local db.* metric series as a side effect. | | agentex/tests/unit/utils/test_rpc_metrics.py | Comprehensive unit tests covering no-op, never-raise, attribute correctness, double-count regression, GeneratorExit, and handler-classification precedence. | | agentex/tests/unit/utils/test_db_metrics.py | Unit tests for the pool instrumentation covering passthrough, success, timeout, non-timeout error, recreate propagation, and end-to-end wiring. | </details> <details><summary><h3>Sequence Diagram</h3></summary> ```mermaid sequenceDiagram participant Client participant agents_py as agents.py participant rpc_timing as rpc_request_timing participant use_case as agents_acp_use_case participant otel as OTel SDK Client->>agents_py: "POST /agents/{id}/rpc" agents_py->>rpc_timing: __enter__ (start timer) rpc_timing-->>agents_py: RpcCallOutcome alt Sync request agents_py->>use_case: handle_rpc_request() use_case-->>agents_py: result_entity agents_py->>rpc_timing: __exit__ (normal) rpc_timing->>otel: record duration/requests agents_py-->>Client: AgentRPCResponse (200) else Streaming request agents_py->>use_case: handle_rpc_request() use_case-->>agents_py: AsyncIterator loop Each chunk agents_py-->>Client: NDJSON chunk (yield) end alt Stream error agents_py->>rpc_timing: rpc_call.fail(-32603, e) agents_py-->>Client: error frame (yield) end agents_py->>rpc_timing: __exit__ (GeneratorExit or normal) rpc_timing->>otel: record duration/requests/errors end Note over agents_py,otel: task/create also emits gen_ai.workflow.duration ``` </details> <sub>Reviews (6): Last reviewed commit: ["Merge branch &#39;main&#39; into stephen-wang/rp..."](0e754eb) | [Re-trigger Greptile](https://app.greptile.com/api/retrigger?id=48001757)</sub> <!-- /greptile_comment --> --------- Co-authored-by: Claude Fable 5 <noreply@anthropic.com>
1 parent 32a9acc commit 80763f7

7 files changed

Lines changed: 945 additions & 94 deletions

File tree

agentex/otel/otel-collector-config.yaml

Lines changed: 6 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -21,10 +21,14 @@ exporters:
2121
sampling_initial: 5
2222
sampling_thereafter: 200
2323

24-
# Expose Prometheus endpoint for scraping
24+
# Expose Prometheus endpoint for scraping.
25+
# No `namespace:` on purpose — the cluster's otel-operator -> daemonset
26+
# pipeline adds no prefix, so metric names there are exactly their instrument
27+
# names (e.g. agentex_rpc_requests_total, db_client_connection_wait_time_seconds).
28+
# A namespace here would double-prefix agentex.* metrics (agentex_agentex_...)
29+
# and make locally-tested PromQL diverge from Mimir.
2530
prometheus:
2631
endpoint: 0.0.0.0:8889
27-
namespace: agentex
2832
send_timestamps: true
2933
metric_expiration: 5m
3034

agentex/src/api/routes/agents.py

Lines changed: 107 additions & 91 deletions
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,9 @@
4343
DAuthorizedResourceIds,
4444
)
4545
from src.utils.logging import make_logger
46+
from src.utils.rpc_metrics import (
47+
rpc_request_timing,
48+
)
4649
from src.utils.task_authorization import check_task_or_collapse_to_404
4750

4851
logger = make_logger(__name__)
@@ -549,53 +552,62 @@ async def _handle_sync_rpc(
549552
request_headers: dict[str, str] | None = None,
550553
) -> AgentRPCResponse:
551554
"""Handle synchronous JSON-RPC requests."""
552-
try:
553-
result_entity = await agents_acp_use_case.handle_rpc_request(
554-
agent_id=agent_id,
555-
agent_name=agent_name,
556-
method=request.method,
557-
params=request.params,
558-
request_headers=request_headers,
559-
)
555+
with rpc_request_timing(request.method.value, streaming=False) as rpc_call:
556+
try:
557+
result_entity = await agents_acp_use_case.handle_rpc_request(
558+
agent_id=agent_id,
559+
agent_name=agent_name,
560+
method=request.method,
561+
params=request.params,
562+
request_headers=request_headers,
563+
)
560564

561-
if isinstance(result_entity, AsyncIterator):
562-
raise ValueError(f"Expected non-async iterator, got {type(result_entity)}")
565+
if isinstance(result_entity, AsyncIterator):
566+
raise ValueError(
567+
f"Expected non-async iterator, got {type(result_entity)}"
568+
)
563569

564-
if isinstance(result_entity, list):
565-
serialized_result = [item.model_dump() for item in result_entity]
566-
else:
567-
serialized_result = result_entity.model_dump()
568-
569-
# if request.method == AgentRPCMethod.MESSAGE_SEND:
570-
# if isinstance(result_entity, list):
571-
# result = [TaskMessage.model_validate(task_message_entity) for task_message_entity in result_entity]
572-
# else:
573-
# raise ValueError(f"Expected list of TaskMessage entities, got {type(result_entity)}")
574-
# elif request.method == AgentRPCMethod.TASK_CREATE:
575-
# result = Task.model_validate(result_entity)
576-
# elif request.method == AgentRPCMethod.TASK_CANCEL:
577-
# result = Task.model_validate(result_entity)
578-
# elif request.method == AgentRPCMethod.EVENT_SEND:
579-
# result = Event.model_validate(result_entity)
580-
# else:
581-
# raise ValueError(f"Unsupported method: {request.method}")
582-
# logger.info(f"AgentRPCResponse Result: {result}")
583-
return AgentRPCResponse.model_validate(
584-
{
585-
"id": request.id,
586-
"result": serialized_result,
587-
"error": None,
588-
}
589-
)
570+
if isinstance(result_entity, list):
571+
serialized_result = [item.model_dump() for item in result_entity]
572+
else:
573+
serialized_result = result_entity.model_dump()
574+
575+
# if request.method == AgentRPCMethod.MESSAGE_SEND:
576+
# if isinstance(result_entity, list):
577+
# result = [TaskMessage.model_validate(task_message_entity) for task_message_entity in result_entity]
578+
# else:
579+
# raise ValueError(f"Expected list of TaskMessage entities, got {type(result_entity)}")
580+
# elif request.method == AgentRPCMethod.TASK_CREATE:
581+
# result = Task.model_validate(result_entity)
582+
# elif request.method == AgentRPCMethod.TASK_CANCEL:
583+
# result = Task.model_validate(result_entity)
584+
# elif request.method == AgentRPCMethod.EVENT_SEND:
585+
# result = Event.model_validate(result_entity)
586+
# else:
587+
# raise ValueError(f"Unsupported method: {request.method}")
588+
# logger.info(f"AgentRPCResponse Result: {result}")
589+
return AgentRPCResponse.model_validate(
590+
{
591+
"id": request.id,
592+
"result": serialized_result,
593+
"error": None,
594+
}
595+
)
590596

591-
except ValidationError as e:
592-
logger.error(f"Validation error in RPC request: {e}", exc_info=True)
593-
error = JSONRPCError(code=-32602, message=f"Invalid parameters: {e}")
594-
return AgentRPCResponse(id=request.id, error=error.model_dump(), result=None)
595-
except Exception as e:
596-
logger.error(f"Error handling JSON-RPC request: {e}", exc_info=True)
597-
error = JSONRPCError(code=-32603, message=str(e))
598-
return AgentRPCResponse(id=request.id, error=error.model_dump(), result=None)
597+
except ValidationError as e:
598+
logger.error(f"Validation error in RPC request: {e}", exc_info=True)
599+
error = JSONRPCError(code=-32602, message=f"Invalid parameters: {e}")
600+
rpc_call.fail(error.code, e)
601+
return AgentRPCResponse(
602+
id=request.id, error=error.model_dump(), result=None
603+
)
604+
except Exception as e:
605+
logger.error(f"Error handling JSON-RPC request: {e}", exc_info=True)
606+
error = JSONRPCError(code=-32603, message=str(e))
607+
rpc_call.fail(error.code, e)
608+
return AgentRPCResponse(
609+
id=request.id, error=error.model_dump(), result=None
610+
)
599611

600612

601613
async def _handle_streaming_rpc(
@@ -608,56 +620,60 @@ async def _handle_streaming_rpc(
608620
"""Handle streaming JSON-RPC requests."""
609621

610622
async def rpc_response_generator():
611-
result_entity_async_iterator = None
612-
try:
613-
result_entity_async_iterator = await agents_acp_use_case.handle_rpc_request(
614-
agent_id=agent_id,
615-
agent_name=agent_name,
616-
method=request.method,
617-
params=request.params,
618-
request_headers=request_headers,
619-
)
620-
621-
if not isinstance(result_entity_async_iterator, AsyncIterator):
622-
raise ValueError(
623-
f"Expected AsyncIterator, got {type(result_entity_async_iterator)}"
623+
with rpc_request_timing(request.method.value, streaming=True) as rpc_call:
624+
result_entity_async_iterator = None
625+
try:
626+
result_entity_async_iterator = (
627+
await agents_acp_use_case.handle_rpc_request(
628+
agent_id=agent_id,
629+
agent_name=agent_name,
630+
method=request.method,
631+
params=request.params,
632+
request_headers=request_headers,
633+
)
624634
)
625635

626-
# At this point we know it's an AsyncIterator[TaskMessage]
627-
async for task_message_update_entity in result_entity_async_iterator:
628-
logger.debug(
629-
f"Streaming message chunk type: {type(task_message_update_entity).__name__}"
630-
)
631-
rpc_response = AgentRPCResponse.model_validate(
632-
{
633-
"id": request.id,
634-
"result": task_message_update_entity.model_dump(),
635-
"error": None,
636-
}
637-
)
638-
# Yield JSON bytes with newline for NDJSON format
639-
yield rpc_response.model_dump_json().encode() + b"\n"
636+
if not isinstance(result_entity_async_iterator, AsyncIterator):
637+
raise ValueError(
638+
f"Expected AsyncIterator, got {type(result_entity_async_iterator)}"
639+
)
640640

641-
except Exception as e:
642-
logger.error(f"Error in streaming RPC response: {e}", exc_info=True)
643-
# Yield error response
644-
error_response = AgentRPCResponse(
645-
id=request.id,
646-
result=None,
647-
error=JSONRPCError(code=-32603, message=str(e)).model_dump(),
648-
)
649-
yield error_response.model_dump_json().encode() + b"\n"
650-
finally:
651-
# CRITICAL: Ensure the async iterator is properly closed
652-
# This ensures HTTP connections are released back to the pool
653-
if result_entity_async_iterator is not None and hasattr(
654-
result_entity_async_iterator, "aclose"
655-
):
656-
try:
657-
await result_entity_async_iterator.aclose()
658-
logger.debug("Closed streaming iterator properly")
659-
except Exception as e:
660-
logger.warning(f"Error closing streaming iterator: {e}")
641+
# At this point we know it's an AsyncIterator[TaskMessage]
642+
async for task_message_update_entity in result_entity_async_iterator:
643+
logger.debug(
644+
f"Streaming message chunk type: {type(task_message_update_entity).__name__}"
645+
)
646+
rpc_response = AgentRPCResponse.model_validate(
647+
{
648+
"id": request.id,
649+
"result": task_message_update_entity.model_dump(),
650+
"error": None,
651+
}
652+
)
653+
# Yield JSON bytes with newline for NDJSON format
654+
yield rpc_response.model_dump_json().encode() + b"\n"
655+
656+
except Exception as e:
657+
logger.error(f"Error in streaming RPC response: {e}", exc_info=True)
658+
rpc_call.fail(-32603, e)
659+
# Yield error response
660+
error_response = AgentRPCResponse(
661+
id=request.id,
662+
result=None,
663+
error=JSONRPCError(code=-32603, message=str(e)).model_dump(),
664+
)
665+
yield error_response.model_dump_json().encode() + b"\n"
666+
finally:
667+
# CRITICAL: Ensure the async iterator is properly closed
668+
# This ensures HTTP connections are released back to the pool
669+
if result_entity_async_iterator is not None and hasattr(
670+
result_entity_async_iterator, "aclose"
671+
):
672+
try:
673+
await result_entity_async_iterator.aclose()
674+
logger.debug("Closed streaming iterator properly")
675+
except Exception as e:
676+
logger.warning(f"Error closing streaming iterator: {e}")
661677

662678
return StreamingResponse(
663679
rpc_response_generator(),

agentex/src/config/dependencies.py

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,10 @@
1919

2020
from src.config.environment_variables import Environment, EnvironmentVariables
2121
from src.utils.database import async_db_engine_creator
22-
from src.utils.db_metrics import PostgresMetricsCollector
22+
from src.utils.db_metrics import (
23+
InstrumentedAsyncAdaptedQueuePool,
24+
PostgresMetricsCollector,
25+
)
2326
from src.utils.logging import make_logger
2427

2528
logger = make_logger(__name__)
@@ -97,6 +100,7 @@ async def load(self):
97100
self.environment_variables.DATABASE_URL,
98101
),
99102
echo=echo_db_engine,
103+
poolclass=InstrumentedAsyncAdaptedQueuePool, # emits pool wait_time/pending_requests/timeouts
100104
pool_size=async_db_pool_size,
101105
max_overflow=20, # Allow 20 additional connections beyond pool_size when needed
102106
pool_pre_ping=True,
@@ -109,6 +113,7 @@ async def load(self):
109113
self.environment_variables.DATABASE_URL,
110114
),
111115
echo=echo_db_engine,
116+
poolclass=InstrumentedAsyncAdaptedQueuePool, # emits pool wait_time/pending_requests/timeouts
112117
pool_size=middleware_db_pool_size,
113118
max_overflow=10, # Allow 10 additional connections for middleware
114119
pool_pre_ping=True,
@@ -188,6 +193,7 @@ async def load(self):
188193
"postgresql+asyncpg://",
189194
async_creator=async_db_engine_creator(read_only_db_url),
190195
echo=echo_db_engine,
196+
poolclass=InstrumentedAsyncAdaptedQueuePool, # emits pool wait_time/pending_requests/timeouts
191197
pool_size=async_db_pool_size,
192198
max_overflow=20,
193199
pool_pre_ping=True,

0 commit comments

Comments
 (0)