Skip to content

Agnosticize integrations HTTP error handling (batch 3 of 3)#24548

Merged
mwdd146980 merged 5 commits into
mwdd146980/http-agnostic-protocol-surfacefrom
mwdd146980/agnosticize-integrations-batch-c
Jul 17, 2026
Merged

Agnosticize integrations HTTP error handling (batch 3 of 3)#24548
mwdd146980 merged 5 commits into
mwdd146980/http-agnostic-protocol-surfacefrom
mwdd146980/agnosticize-integrations-batch-c

Conversation

@mwdd146980

@mwdd146980 mwdd146980 commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

What does this PR do?

Removes backend-specific requests references from the final set of integrations in this sweep, leaving each on the HTTP-agnostic surface only. Per integration it narrows self.http catch sites to datadog_checks.base.utils.http_exceptions.* (no dual-catch), swaps bare from requests import ... to agnostic names (including requests.Response type hints to HTTPResponseProtocol), drops now-unused requests imports, and updates the interception-path tests (agnostic exceptions, MockHTTPResponse, and the mock_http/mock_openmetrics_http fixtures).

Integrations covered (batch 3 of 3): mesos_slave, n8n, nifi, nutanix, octopus_deploy, prefect, proxmox, rabbitmq, sonarqube, sonatype_nexus, spark, squid, teamcity, traefik_mesh, yarn.

The backend stays requests. Behavior is unchanged. This PR is based directly on #24516 for now. When #24516 is approved, it will be merged into #22676, and then this PR will be stacked on #22676.

Motivation

Checks and their tests should depend only on the agnostic HTTP surface (http_exceptions.*), never on requests-specific APIs, so the backend can later be swapped without touching consumers.

Verification

  • Touched production packages are free of requests imports/attribute use.
  • Catch sites reference only http_exceptions.*. No (RequestException, HTTPError) dual-catch remains.
  • Per-integration unit tests were run and pass in the source work. ddev test --lint is green across the touched integrations.

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • Add qa/required if this PR needs QA validation, or qa/skip-qa if it does not. Exactly one of the two is required.
  • If you need to backport this PR to another branch, you can add the backport/<branch-name> label to the PR and it will automatically open a backport PR once this one is merged

@datadog-official

datadog-official Bot commented Jul 14, 2026

Copy link
Copy Markdown
Contributor

Pipelines  Tests

⚠️ Warnings

🚦 29 Pipeline jobs failed

PR | test / test (linux, ubuntu-22.04, nifi, Apache NiFi (py3.13-2.8), py3.13-2.8) / Apache NiFi (py3.13-2.8)-py3.13-2.8   View in Datadog   GitHub Actions

PR | test / test (linux, ubuntu-22.04, nutanix, Nutanix (py3.13), py3.13) / Nutanix (py3.13)-py3.13   View in Datadog   GitHub Actions

PR | test / test (linux, ubuntu-22.04, sonatype_nexus, sonatype_nexus (py3.13), py3.13) / sonatype_nexus (py3.13)-py3.13   View in Datadog   GitHub Actions

View all 29 failed jobs.

🧪 1 Test failed in 1 job

PR | run   GitHub Actions

test_get_version from test_unit.py   View in Datadog
No module named &#39;datadog_checks.base.utils.http_testing&#39;

Useful? React with 👍 / 👎

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 7929b0c | Docs | Datadog PR Page | Give us feedback!

@mwdd146980
mwdd146980 force-pushed the mwdd146980/agnosticize-integrations-batch-c branch from 0c5647f to 8d6f525 Compare July 14, 2026 16:15
@mwdd146980
mwdd146980 force-pushed the mwdd146980/agnosticize-integrations-batch-b branch from 8677a25 to 633eb98 Compare July 14, 2026 16:15
@mwdd146980
mwdd146980 changed the base branch from mwdd146980/agnosticize-integrations-batch-b to mwdd146980/http-agnostic-protocol-surface July 14, 2026 16:15
@mwdd146980 mwdd146980 changed the title Agnosticize integrations HTTP error handling (batch C) Agnosticize integrations HTTP error handling (batch 3 of 3) Jul 14, 2026
@mwdd146980
mwdd146980 marked this pull request as ready for review July 14, 2026 19:23
@mwdd146980
mwdd146980 requested a review from a team as a code owner July 14, 2026 19:23

@NouemanKHAL NouemanKHAL left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM for the code!

Blocker: changelog entries should either by a .added or omitted in this PR and only generated when merging to master as this is a dev change, we only want a changelog for the user-facing changes (when merging to master)

We'd also probably want ti have a single changelog entry for a migration to an agnostic HTTP layer, for now I see different changelog entries such as:

  • Migrate off requests-specific exceptions to the HTTP-agnostic exception library.
  • Use library-agnostic HTTP exceptions when handling API request failures.
  • Use library-agnostic HTTP exceptions when handling request timeouts.
  • Use library-agnostic HTTP exceptions when a token request returns an unexpected status.

Which are a bit too specific for a user-facing changelog.

Narrow self.http catch sites to backend-agnostic http_exceptions and swap
interception-path tests to the agnostic surface for: mesos_slave, n8n,
nifi, nutanix, octopus_deploy. Keeps the existing n8n/nutanix changelog
entries already present on the base.
Narrow self.http catch sites to backend-agnostic http_exceptions and swap
interception-path tests to the agnostic surface for: proxmox, rabbitmq,
sonarqube, sonatype_nexus, spark, squid, teamcity, traefik_mesh, yarn,
and complete prefect.
…C review)

The agnostic ResponseWrapper.json() raises a stdlib json.JSONDecodeError
(ValueError), not an HTTPError, so a 200 with a non-JSON body escaped
`except HTTPError` in check() and skipped the connect service check that
master emitted (master caught it via the broad RequestException). Also
catch json.JSONDecodeError.
Address non-blocking review nits on the new nifi test helpers:

- Drop the leading underscore from the module-level helpers introduced in
  this PR (_dispatch, _mock_http_responses -> dispatch, mock_http_responses)
  per the repo naming convention for test-file helpers. Pre-existing
  underscore helpers are left untouched (out of scope).
- Route test_overlapping_process_groups_no_duplicates through
  mock_http_responses instead of a hand-rolled side_effect, matching its
  siblings. Behavior is identical (same URL-substring dispatch).
@mwdd146980
mwdd146980 force-pushed the mwdd146980/agnosticize-integrations-batch-c branch from db61d14 to 0d15c84 Compare July 17, 2026 13:18
This PR merges into a feature branch, not master, so changelog
entries aren't needed until the feature branch itself ships.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
@mwdd146980
mwdd146980 requested a review from NouemanKHAL July 17, 2026 13:19
@mwdd146980
mwdd146980 merged commit cffde7e into mwdd146980/http-agnostic-protocol-surface Jul 17, 2026
45 of 75 checks passed
@mwdd146980
mwdd146980 deleted the mwdd146980/agnosticize-integrations-batch-c branch July 17, 2026 13:22
@dd-octo-sts

dd-octo-sts Bot commented Jul 17, 2026

Copy link
Copy Markdown
Contributor

Validation Report

All 21 validations passed.

Show details
Validation Description Status
agent-reqs Verify check versions match the Agent requirements file
ci Validate CI configuration and code coverage settings
codeowners Validate every integration has a CODEOWNERS entry
config Validate default configuration files against spec.yaml
dep Verify dependency pins are consistent and Agent-compatible
http Validate integrations use the HTTP wrapper correctly
imports Validate check imports do not use deprecated modules
integration-style Validate check code style conventions
jmx-metrics Validate JMX metrics definition files and config
labeler Validate PR labeler config matches integration directories
legacy-signature Validate no integration uses the legacy Agent check signature
license-headers Validate Python files have proper license headers
licenses Validate third-party license attribution list
metadata Validate metadata.csv metric definitions
models Validate configuration data models match spec.yaml
openmetrics Validate OpenMetrics integrations disable the metric limit
package Validate Python package metadata and naming
qa-label Validate the pull request declares whether it needs QA for the next Agent release
readmes Validate README files have required sections
saved-views Validate saved view JSON file structure and fields
version Validate version consistency between package and changelog

View full run

mwdd146980 added a commit that referenced this pull request Jul 20, 2026
…24516)

* feat(base): add agnostic HTTP protocol capabilities and auth/factory seam

Freeze the backend-neutral HTTP surface in http_protocol.py and implement it on
RequestsWrapper so consumers depend on the protocol, not requests:

- Add connection close(), get_cookie(), and a trust_env property to
  HTTPClientProtocol/RequestsWrapper (capabilities expose behavior, never a
  backend object).
- Add an agnostic auth hook (HTTPRequestAuth + OutgoingRequest) with a
  requests.auth.AuthBase adapter, so checks contribute headers/params without a
  requests-family auth type.
- Add an overridable AgentCheck.create_http_wrapper() factory seam behind
  self.http, so the backend is swappable without touching consumers.

Backend stays requests; changes are additive. Foundation for the httpx2 backend.

* fix(base): harden get_cookie and annotate agnostic auth adapter

Address code-review findings on the agnostic HTTP surface:

- get_cookie: return default without eagerly building a session when none exists
  yet, and catch requests CookieConflictError so the documented value-or-default
  contract holds even with duplicate cookie names; fix the docstring.
- Annotate the new _OutgoingRequest and _RequestsAuthHookAdapter helpers, matching
  the typed capabilities and protocols added in the same change.
- Add regression tests: idempotent close after open, no-eager-session on cookie
  lookup, and conflict-returns-default.

* fix(base): address review findings on the agnostic auth adapter

- Trim the _RequestsAuthHookAdapter docstring to describe actual behavior: header
  edits apply via the shared dict, params are merged into the URL query, and the
  URL is read-only (finding 1).
- Document that only per-call auth= is adapted for HTTPRequestAuth hooks; a hook
  set persistently via options['auth'] is unsupported (finding 4).
- Revert the get_cookie hardening back to the minimal RequestsCookieJar.get
  delegation; that robustness was out of scope for a behavior-preserving
  foundation PR (finding 3).

Keeps the new-code type annotations (finding 5) and the idempotent-close test
(finding 7).

* fix(base): honor get_cookie contract and document url read-only in protocol

Round-2 review follow-ups on the agnostic HTTP surface:

- get_cookie: catch requests CookieConflictError and return default so the
  method honors its own documented value-or-default contract. The method is new
  and the frozen protocol declares that contract, and the exception is
  requests-specific with no httpx2 equivalent, so leaving it unhandled would
  diverge per backend. Adds a cross-domain collision regression test.
- OutgoingRequest protocol: document that url is inspection-only and writes are
  not guaranteed to be honored, in the frozen contract file where a second
  backend author will read it.

* fix(base): adopt injected session trust_env in RequestsWrapper

When a session is injected via session=, initialize the tracked _trust_env from
that session's trust_env instead of hard-coding True, so the reported trust_env
matches the session it wraps. getattr guards sessions that do not expose the
attribute (e.g. autospec mocks). Adds a regression test for the injected-session
case. Addresses round-2 review finding 2.

* refactor(base): rename OutgoingRequest to HTTPRequest and create_http_wrapper to create_http_client

Rename the agnostic auth-hook request view OutgoingRequest (and its concrete
_OutgoingRequest) to HTTPRequest/_HTTPRequest, and the factory seam
AgentCheck.create_http_wrapper to create_http_client, aligning with the
HTTPClientProtocol return type. Behavior unchanged.

* style(base): fix ruff import-order and B018 after the rename

ruff isort reordered the renamed http_protocol imports (HTTPRequest before
HTTPRequestAuth) in http.py and test_auth_hook.py, and the double-close test's
bare 'http.session' expression is now an assertion to satisfy B018.

* docs(base): document get_cookie ambiguous-collision fallback in contract

The HTTPClientProtocol.get_cookie contract said only value or default; the concrete
RequestsWrapper returns default on an ambiguous same-name cookie (CookieConflictError)
but the frozen backend-neutral protocol gave no signal that a second backend must do the
same. Align the contract so a future backend implements the edge case identically.

* refactor(base): drop the HTTPRequestAuth hook seam

Remove HTTPRequest/HTTPRequestAuth from the protocol surface and the
_HTTPRequest/_RequestsAuthHookAdapter plumbing plus its per-call dispatch, and
delete the seam-only test. The seam had a single intended consumer (voltdb),
which now sends credentials as query params without it, so the frozen surface
should not carry the hook. Net diff of this PR no longer introduces it.

* docs(base): drop stale auth-hook re-export comment

* fix(base): address review findings on protocol capabilities

Soften the frozen-surface claim to provisional, disambiguate the
options_method comment, use collections.abc.Iterator, trim a
restating comment, cover the trust_env getattr fallback, and merge a
weak cookie assertion into its stronger sibling test.

* refactor(base): rename protocol classes to HTTPClient/HTTPResponse

Drop the Protocol suffix now that these are the canonical
backend-neutral HTTP surface names shared by the requests and
future httpx backends.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(base): move MockHTTPResponse to datadog_checks_dev

MockHTTPResponse is test-support code that was living under
datadog_checks/base/utils/, so it shipped in the base wheel bundled
with the Agent. Move it next to its sibling MockResponse in
datadog_checks_dev/datadog_checks/dev/http.py, which is dev-only and
never bundled.

Import the HTTPStatusError dependency lazily inside raise_for_status()
so importing dev.http does not pull datadog_checks_base at module load,
matching the existing lazy-import convention in the dev pytest plugin
(datadog_checks_dev does not declare base as a dependency).

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(base): declare response attrs and get_peer_cert on protocol

* feat(base): add ResponseWrapper.get_peer_cert capability

* feat(dev): enforce HTTPResponse surface on MockHTTPResponse

* fix(dev): enforce guard on public protocol surface only

MockHTTPResponse's __getattr__/__setattr__ rejected any name not on
the HTTPResponse protocol, including leading-underscore names. When
wrapped by wrapt's ResponseWrapper, wrapt forwards private attribute
sets (e.g. _ResponseWrapper__default_chunk_size) to the wrapped
object, and the guard raised AttributeError on those, breaking the
request path. A protocol is a public contract, so only public names
should be checked against it; underscore-prefixed names pass through.

* fix(dev): add history attribute to mock response

* fix(base): parse kubelet pod list via response.json instead of raw

* fix(http_check): read peer cert via get_peer_cert capability

Swaps the .raw.connection.sock.getpeercert access for the
HTTPResponse.get_peer_cert protocol member, so the check no longer
depends on the requests-specific raw urllib3 response.

* chore: add changelog entries for HTTP response surface (#24516)

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* feat(base): extend HTTP client protocol surface

Declare ignore_tls_warning and persist_connections as writable client
attributes, add a should_bypass_proxy capability, and document the
per-request persist option on the verb methods. These cover the client
touchpoints (items 2-5 of the surface audit) integrations reach for
today, so they can move onto the agnostic protocol without reaching into
RequestsWrapper internals.

RequestsWrapper already carried the two attributes as slots and consumed
persist in _request; only should_bypass_proxy is new, delegating to the
existing module-level helper.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore(base): tighten HTTPResponse protocol comments

Shorten multi-line comments to single lines and drop backticks.

* chore(dev): tighten MockHTTPResponse guard comment

Shorten the comment to one line and drop the semicolon.

* chore(base): tighten HTTPClient protocol comments

Shorten multi-line comments to single lines and drop backticks.

* style: fix import order flagged by ruff --fix

Run ddev test --fmt across the PR's changed files.

* chore: renumber changelog entries from 24516 to 22676

Entries land on master through feature branch #22676, so the
changelog filenames must reference that PR number.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Agnosticize integrations HTTP error handling (batch 1 of 3) (#24546)

* Agnosticize integrations batch A1 (catch-narrowing + tests)

Narrow self.http catch sites to backend-agnostic http_exceptions and swap
interception-path tests to the agnostic surface for: activemq_xml, ambari,
arangodb, citrix_hypervisor, cloud_foundry_api, consul.

* Agnosticize integrations batch A2 (catch-narrowing + tests)

Same agnosticization for: control_m, couch, couchbase, druid, ecs_fargate,
elastic, envoy.

* Preserve error handling after agnostic exception narrowing (batch A review)

Address review findings on #24546 where narrowed catch sites changed
behavior versus master:

- couchbase: the agnostic ResponseWrapper.json() raises a stdlib
  json.JSONDecodeError (ValueError), not an HTTPError, so a 200 with a
  non-JSON body escaped `except HTTPError` and aborted the run. Master
  caught it via the broad RequestException. Also catch json.JSONDecodeError
  at the three sync-gateway/query-monitoring/index-stats sites.
- activemq_xml: ConnectTimeout translates to HTTPTimeoutError (a sibling of
  HTTPConnectionError), so connect timeouts escaped `except
  HTTPConnectionError` and defeated suppress_errors. Catch HTTPTimeoutError
  as well.
- druid: add explicit parametrize ids to the can-connect failure test.

* chore: drop changelog entries for feature-branch merge

This PR merges into a feature branch, not master, so changelog
entries aren't needed until the feature branch itself ships.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

* Agnosticize integrations HTTP error handling (batch 2 of 3) (#24547)

* Agnosticize integrations batch B (catch-narrowing + tests)

Narrow self.http catch sites to backend-agnostic http_exceptions and swap
interception-path tests to the agnostic surface for: fly_io, gitlab, harbor,
hdfs_datanode, hdfs_namenode, ibm_db2, kubelet, kyototycoon, mapreduce,
marathon, marklogic, mesos_master.

* Catch connect timeouts in marklogic after agnostic narrowing (batch B review)

ConnectTimeout translates to HTTPTimeoutError, a sibling of
HTTPConnectionError, so it escaped `except (HTTPStatusError,
HTTPConnectionError)` and skipped the CRITICAL connect service check that
master emitted (master caught it via requests ConnectionError). Add
HTTPTimeoutError to the catch.

* Rename kubelet node-metrics test to match agnostic exception (batch 2 review)

The test body raises HTTPStatusError, not a requests HTTPError, so rename
test_report_node_metrics_kubernetes1_18_requests_httperror to
..._httpstatuserror. Test-only, no behavior change.

* chore: drop changelog entries for feature-branch merge

This PR merges into a feature branch, not master, so changelog
entries aren't needed until the feature branch itself ships.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

* Agnosticize integrations HTTP error handling (batch 3 of 3) (#24548)

* Agnosticize integrations batch C1 (catch-narrowing + tests)

Narrow self.http catch sites to backend-agnostic http_exceptions and swap
interception-path tests to the agnostic surface for: mesos_slave, n8n,
nifi, nutanix, octopus_deploy. Keeps the existing n8n/nutanix changelog
entries already present on the base.

* Agnosticize integrations batch C2 (catch-narrowing + tests)

Narrow self.http catch sites to backend-agnostic http_exceptions and swap
interception-path tests to the agnostic surface for: proxmox, rabbitmq,
sonarqube, sonatype_nexus, spark, squid, teamcity, traefik_mesh, yarn,
and complete prefect.

* Preserve malformed-JSON handling in sonarqube after narrowing (batch C review)

The agnostic ResponseWrapper.json() raises a stdlib json.JSONDecodeError
(ValueError), not an HTTPError, so a 200 with a non-JSON body escaped
`except HTTPError` in check() and skipped the connect service check that
master emitted (master caught it via the broad RequestException). Also
catch json.JSONDecodeError.

* Tidy nifi test helpers after agnostic migration (batch 3 review)

Address non-blocking review nits on the new nifi test helpers:

- Drop the leading underscore from the module-level helpers introduced in
  this PR (_dispatch, _mock_http_responses -> dispatch, mock_http_responses)
  per the repo naming convention for test-file helpers. Pre-existing
  underscore helpers are left untouched (out of scope).
- Route test_overlapping_process_groups_no_duplicates through
  mock_http_responses instead of a hand-rolled side_effect, matching its
  siblings. Behavior is identical (same URL-substring dispatch).

* chore: drop changelog entries for feature-branch merge

This PR merges into a feature branch, not master, so changelog
entries aren't needed until the feature branch itself ships.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>

---------

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

* Migrate openstack_controller to the backend-neutral HTTP protocol (#24594)

* fix(openstack_controller): use backend-neutral HTTP protocol for keystone session

Build the keystoneauth Session from the backend-neutral HTTPClient
protocol surface (verify, cert, headers) instead of handing keystoneauth
the underlying requests.Session. This removes the last openstack_controller
coupling to the requests backend while preserving TLS config and the Nova
and Ironic microversion headers on the SDK data-plane connection.

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* chore(openstack_controller): add changelog entry for 24594

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* Delete openstack_controller/changelog.d/24594.fixed

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

* fix: repair HTTP symbol references broken by branch refactors

Two stragglers from this branch's own HTTP refactors were left with
stale references and broke CI:

- MockHTTPResponse moved from datadog_checks.base.utils.http_testing to
  datadog_checks.dev.http; update the four test files still importing
  the old path (sonatype_nexus, nifi, traefik_mesh, nutanix).
- HTTPResponseProtocol was renamed to HTTPResponse in http_protocol;
  update the two production files still referencing the old name
  (nutanix at runtime, control_m under TYPE_CHECKING).

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(http): address self-review gaps in agnostic surface

Close protocol-compliance and hardening gaps surfaced by the automated
self review, and finish decoupling http_check from requests internals.

- Wrap ResponseWrapper.history so redirect items satisfy HTTPResponse
  and translate exceptions, instead of leaking raw requests objects.
- Guard get_peer_cert against bare non-TLS sockets that lack
  getpeercert, so plain http:// requests return None rather than raise.
- Route http_check through trust_env and close() instead of reaching
  into the raw session, removing its last requests-specific internals.
- Drop the leading underscore from CaseInsensitiveDict and prune the
  redundant dunders from protocol_members().

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* chore(http): drop changelog entries until master-merge prep

This PR merges into the httpx-migration-base feature branch, not
master, so per-integration changelog entries are premature. Remove the
entries added here and restore the base-branch entries this branch had
deleted, leaving the PR changelog-neutral.

* Migrate remaining integrations to the backend-neutral HTTP protocol (#24597)

* fix(http): migrate remaining consumers to backend-neutral HTTP protocol

Replace the last direct reaches into the requests Session across four
integrations with the backend-neutral HTTPClient protocol:

- avi_vantage: http.close() and http.get_cookie()
- cisco_aci: http.close()
- hpe_aruba_edgeconnect: http.get_cookie() and http.set_header()
- http_check: http.trust_env setter and http.close()

Behavior is preserved; only the access path changes. Updates the
hpe_aruba_edgeconnect unit tests to drive the protocol methods.


Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

* Decouple integration tests from the requests library (#24602)

* test(crio): use mock_http_response fixture

* test(flink): use mock_http_response fixture

* test(nginx_ingress_controller): use mock_http_response fixture

* test(nginx): drive vts unit test through mock_http seam

* test(silk): unit-test error_msg path via mock_http_response

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(http_check): suppress network via mock_http_response

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>

* test(nvidia_nim): drive unit test through mock_http seam

* test(dell_powerflex): migrate fixtures to mock_http seam

conftest.py's mock_http_call, mock_auth, and mock_http_get fixtures
patched requests.Session directly, coupling tests to the requests
backend. Route them through the auto-available mock_http fixture and
agnostic MockHTTPResponse/HTTPStatusError instead.

* test(dell_powerflex): use agnostic exceptions in unit tests

Five tests patched requests.Session.get/post directly and imported
requests.exceptions. Swap them onto the mock_http fixture and the
agnostic http_exceptions module so the suite no longer depends on
requests.

---------

Co-authored-by: Claude Opus 4.8 <noreply@anthropic.com>

* Narrow HTTP exception catches in kube_* health checks (#24604)

* fix(kube): catch agnostic HTTP exceptions in health checks

* test(kube): cover non-status HTTP subtype in health checks

The health-check catches were widened to the base agnostic HTTPError, but
tests only raised HTTPStatusError. Add an HTTPConnectionError case per
check so a non-status subtype is verified to still map to CRITICAL, pinning
the widened contract against an accidental narrowing back to HTTPStatusError.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* Add an HTTP client factory seam and route wrapper construction through it (#24622)

* feat(base): add create_http_client factory seam

Introduce a module-level create_http_client(instance, init_config,
remapper, logger) in base.utils.http as the single place that names the
HTTP backend, and have AgentCheck.create_http_client delegate to it with
an optional instance override.

This lets callers that build a client from a config other than the
check's own instance (per-endpoint scraper configs, bespoke health
handlers) share one seam, and lets non-check contexts reuse the module
function directly. Callers keep the agnostic HTTPClient protocol as the
return type, so the concrete backend stays confined to this factory.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(base): route mixin http handlers through the factory

The OpenMetrics and Prometheus v1 scraper mixins built their per-endpoint
RequestsWrapper directly. Route them through self.create_http_client so
the framework has a single construction seam, and drop the now-unused
RequestsWrapper imports. Behavior is unchanged.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* refactor(http): route integration wrapper construction through the factory

argocd, hpe_aruba_edgeconnect, and the five kube_* checks built their own
RequestsWrapper for a per-endpoint or health handler. Route each through
create_http_client (self.create_http_client for checks, the check's own
for the argocd resources helper) and drop the RequestsWrapper imports.

The kube_* conftests patched the removed RequestsWrapper symbol. Since
the metrics path is already intercepted at get_http_handler, the health
handler is create_http_client's only live caller under those fixtures, so
retarget the patch to AgentCheck.create_http_client.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* style(base): condense create_http_client docstring to one line

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* test(base): pin factory contract and fix review nits

Address review feedback on the create_http_client seam:
- Type the module function's instance param as dict | None, matching the
  delegating method that can pass self.instance (None) into the or {} guard.
- Soften the module docstring, which overclaimed a single repo-wide seam
  while vsphere still constructs a wrapper directly.
- Fix the override example in test_factory_override_swaps_client to keep
  the instance parameter, so it does not model a Liskov-incompatible override.
- Add a test that get_http_handler routes through create_http_client, pinning
  the new coupling that overriding the factory now governs the scraper handler.
- Add a test that an explicit empty-dict instance override is honored and does
  not fall back to the check's own instance.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

* fix(base): route mixin handlers through the module-level factory

The OpenMetrics/Prometheus mixins called self.create_http_client, which
only exists on AgentCheck. PrometheusScraper composes PrometheusScraperMixin
without being an AgentCheck (it holds a check reference and uses its own
init_config/HTTP_CONFIG_REMAPPER/log), so the mixin raised AttributeError
and the Prometheus integration test failed.

Route both mixins through the module-level create_http_client with explicit
config, matching the original RequestsWrapper arguments exactly. This is
backend-agnostic and does not assume the mixin is mixed into a check. Update
the coverage test to pin the module-level seam instead of the method.

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>

---------

Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Co-authored-by: NouemanKHAL <noueman.khalikine@datadoghq.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment