perf(http): use shared pooled HTTP client in monitoring_integration (#12979)

[mrveiss]

Thinking Path

Pilot file for #12979, chosen in the issue discussion because its 11 sites are uniform enough to establish a pattern but contained enough that a mistake is reviewable.

The naive substitution here is wrong. Call sites currently nest two context managers:

async with aiohttp.ClientSession() as session:
    async with session.get(url, ...) as response:

The inner async with is the connection-release point. HTTPClientManager.get() returns a ClientResponse rather than yielding one, so dropping that async with during conversion would trade per-request session creation for leaked pooled connections — strictly worse, because a finite pool exhausts rather than merely paying a handshake.

So the file splits by how each site consumes the response, not by shape:

shape	count	conversion
raise_for_status() + json()	9	get_json() / post_json() — semantically exact, lifetime handled by the helper
if response.status == 200: → structured error	2	async with await client.get(...), async with mandatory

The 2 exceptions are both test_connection() (Datadog :66, New Relic :289). They return an IntegrationHealth(status=ERROR, message=f"... {response.status}") object instead of raising. get_json() calls raise_for_status(), so converting them uniformly would silently turn "report the failure" into "raise" — precisely in the error paths of two monitoring integrations, i.e. where behaviour change is least welcome and least likely to be noticed.

What Changed

autobot-backend/integrations/monitoring_integration.py only.

• Added from autobot_shared.http_client import get_http_client.
• 7 GET sites → await get_http_client().get_json(url, headers=..., timeout=...)
  (_list_monitors, _get_metrics, _list_hosts, _get_events on Datadog; _list_applications, _list_alerts, _get_app_health on New Relic)
• 2 POST sites → await get_http_client().post_json(url, payload, headers=..., timeout=...)
  (_create_monitor, New Relic NRQL GraphQL _get_metrics)
• 2 status-inspecting sites → async with await get_http_client().get(...) as response: with the existing if response.status == 200: ... else: ... logic preserved verbatim. Each carries a comment explaining why the async with is load-bearing.

import aiohttp is retained — still needed for aiohttp.ClientTimeout and the except aiohttp.ClientError handlers. Per-call timeouts and headers are unchanged; HTTPClientManager.request() forwards both straight through to session.request().

No behaviour change: the 9 converted sites already called raise_for_status() by hand, which is exactly what get_json/post_json do. The 2 error-reporting paths keep returning structured IntegrationHealth objects.

Incidental gain: request() merges W3C trace context into caller headers via OTel inject(), so all 11 calls now propagate distributed traces. Raw per-request sessions missed this entirely.

Verification

Counts, before → after:

metric	before	after
aiohttp.ClientSession( constructions	11	0
get_json( call sites	0	7
post_json( call sites	0	2
async with await get_http_client() sites	0	2
total converted	—	11 / 11

Static checks:

$ python3 -m py_compile integrations/monitoring_integration.py
PY_COMPILE OK

$ python3 -m flake8 integrations/monitoring_integration.py
flake8 exit=0

$ python3 -m black --check --line-length 120 integrations/monitoring_integration.py
1 file would be left unchanged.

$ python3 -m isort --check-only integrations/monitoring_integration.py
isort exit=0

$ grep -c "aiohttp.ClientSession(" integrations/monitoring_integration.py
0

Import smoke:

IMPORT OK .../integrations/monitoring_integration.py
DatadogIntegration <class 'integrations.monitoring_integration.DatadogIntegration'>
NewRelicIntegration <class 'integrations.monitoring_integration.NewRelicIntegration'>

Functional — all 11 converted paths exercised end-to-end against a local aiohttp test server, including the 503 branch that the 2 special-cased sites exist to serve:

dd.test_connection 200 -> IntegrationStatus.HEALTHY
dd.test_connection 503 -> IntegrationStatus.ERROR | Datadog API error: 503 | {'error': 'nope'}
list_monitors    -> {'monitors': [{'id': 1}, {'id': 2}], 'count': 2}
dd get_metrics   -> {'series': []}
list_hosts       -> {'hosts': ['h1'], 'total': 1}
get_events       -> {'events': [{'id': 9}]}
create_monitor   -> {'id': 42}
nr.test_connection -> IntegrationStatus.HEALTHY
list_applications -> {'applications': [{'id': 1}], 'count': 1}
nr get_metrics   -> {'results': [1, 2, 3], 'count': 3}
list_alerts      -> {'policies': [{'id': 3}], 'count': 1}
get_app_health   -> {'id': 1, 'name': 'x', 'health_status': 'green', 'reporting': True, 'summary': {}}

The 503 line is the regression this split exists to prevent: a uniform get_json() conversion would have raised there instead of returning IntegrationStatus.ERROR.

Connection-lifetime check — the trap this PR is guarding against. After 12 requests through the shared pool:

pooled idle conns=1 still-acquired=0

One connection reused across all 12 requests, zero still acquired. Both the get_json helpers and the two hand-written async with blocks release correctly; had either dropped its async with, still-acquired would be non-zero.

No existing tests reference this module (grep -rl "monitoring_integration\|DatadogIntegration\|NewRelicIntegration" --include=*.py returns only the module itself, api/integration_monitoring.py, and an unrelated infrastructure script), so the local server exercise above stands in for regression coverage.

Discovered, not fixed here

Filed #12981: HTTPClientManager._active_requests increments on every request() but only decrements on the error path, and get_json/post_json never decrement — so the counter grows monotonically (active_requests: 12 after 12 clean requests, visible in the stats above). This drives pool auto-sizing utilization permanently upward and means a deferred pool recreation never applies. Exactly one caller in the repo honours the contract, against ~151 call sites.

Pre-existing and not introduced by this PR — it affects every shared-client call site equally. Left out of scope deliberately: the fix belongs in autobot_shared/http_client.py and would change behaviour for all 151 sites, which does not belong in a single-file pilot conversion.

Model Used

claude-opus-5

Refs #12979.

[github-actions]

✅ SSOT Configuration Compliance: Passing

🎉 No hardcoded values detected that have SSOT config equivalents!