add HTTP client request metrics for scaler metric requests by aliaqel-stripe · Pull Request #7644 · kedacore/keda

aliaqel-stripe · 2026-04-13T01:30:27Z

Add HTTP client request metrics for scaler metric collection (keda_scaler_http_requests_total / keda_scaler_http_request_duration_seconds for Prometheus, keda.scaler.http.requests.count / keda.scaler.http.request.duration.seconds for OTel). Recording is gated on all five scaler context keys being present in the request context, so only metric collection calls emitted through GetMetricsAndActivityForScaler are counted — not scaler-initialization requests.

The request counter metric (keda_scaler_http_requests_total) is labelled by namespace, scaled_resource, scaler, trigger_name, metric_name, status_code.

The latency histogram (keda_scaler_http_request_duration_seconds) uses only scaler and status_code to keep MTS cardinality low — latency by scaler type probably matters most here.

status_code is the numeric HTTP status, or "error" for transport-level failures with no HTTP response.

Changes

pkg/util/http_roundtripper.go (new): InstrumentedRoundTripper wraps any http.RoundTripper and records metrics after each round-trip when all scaler context keys are present.
pkg/util/http.go: CreateHTTPTransportWithTLSConfig now wraps the transport with NewInstrumentedRoundTripper, so all scalers using these helpers gain instrumentation automatically.
pkg/scaling/cache/scalers_cache.go: New buildScalerRequestCtx injects the five context keys before calling GetMetricsAndActivity. The retry path re-fetches the ScalerBuilder after refresh so labels reflect the current scaler config.
pkg/scalers/aws/aws_sigv4.go: Transport is now initialised once in NewSigV4RoundTripper rather than per-request, eliminating double instrumentation and restoring connection pooling.
pkg/metricscollector/: New RecordHTTPClientRequest in the interface and both backends; httpStatusCodeLabel moved to the shared file; OTel error log message corrected.
Tests: New unit test files for http_roundtripper and prommetrics; e2e assertions added to both Prometheus and OTel sequential test suites.

Checklist

I have verified that my change is according to the deprecations & breaking changes policy
Tests have been added
Changelog has been updated
A PR is opened to update the documentation on (repo) — add keda_scaler_http_requests_total and keda_scaler_http_request_duration_seconds metrics keda-docs#1734

Fixes #6600

snyk-io · 2026-04-13T01:30:41Z

✅ Snyk checks have passed. No issues have been found so far.

Status	Scan Engine	Critical	High	Medium	Low	Total (0)
✅	Open Source Security	0	0	0	0	0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

github-actions · 2026-04-13T01:30:47Z

Thank you for your contribution! 🙏

Please understand that we will do our best to review your PR and give you feedback as soon as possible, but please bear with us if it takes a little longer as expected.

While you are waiting, make sure to:

Add an entry in our changelog in alphabetical order and link related issue
Update the documentation, if needed
Add unit & e2e tests for your changes
GitHub checks are passing
Is the DCO check failing? Here is how you can fix DCO issues

Once the initial tests are successful, a KEDA member will ensure that the e2e tests are run. Once the e2e tests have been successfully completed, the PR may be merged at a later date. Please be patient.

Learn more about our contribution guide.

Copilot

Pull request overview

Adds outbound HTTP client request metrics for scaler metric collection, enabling better observability of scaler-to-upstream HTTP success/error rates and latencies across both Prometheus and OpenTelemetry backends.

Changes:

Introduces an instrumented http.RoundTripper and wires it into shared HTTP client/transport helpers, with metric recording gated on scaler-context keys.
Injects scaler context into metric-collection request contexts (including retry/refresh path) so outbound requests can be labeled correctly.
Adds Prometheus + OTel metric instruments/recording paths, plus unit and sequential/e2e-style assertions.

Reviewed changes

Copilot reviewed 14 out of 14 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
tests/sequential/prometheus_metrics/prometheus_metrics_test.go	Adds sequential test assertions for new Prometheus HTTP client metrics.
tests/sequential/opentelemetry_metrics/opentelemetry_metrics_test.go	Adds sequential test assertions for OTel-exported HTTP client metrics.
pkg/util/http_roundtripper.go	New `InstrumentedRoundTripper` that records per-request count + duration when scaler context is present.
pkg/util/http_roundtripper_test.go	Unit tests for round-tripper wrapping behavior and helper instrumentation.
pkg/util/http.go	Wraps shared transports with instrumentation; changes helper return types to `http.RoundTripper`.
pkg/scaling/cache/scalers_cache.go	Injects scaler context keys into metric-collection context; updates retry path label sourcing.
pkg/scaling/cache/scalers_cache_test.go	Unit test verifying scaler request context key injection.
pkg/scalers/aws/aws_sigv4.go	Reuses a single transport in SigV4 RT to restore pooling and avoid repeated wrapping.
pkg/metricscollector/metricscollectors.go	Extends metrics collector interface with `RecordHTTPClientRequest` and shared status-code labeling.
pkg/metricscollector/prommetrics.go	Adds Prometheus counter + histogram and implements `RecordHTTPClientRequest`.
pkg/metricscollector/prommetrics_test.go	Unit tests for status code label logic and Prometheus recording path.
pkg/metricscollector/opentelemetry.go	Adds OTel instruments and implements `RecordHTTPClientRequest`; adjusts an error log message.
pkg/metricscollector/opentelemetry_test.go	Adds unit test coverage for OTel HTTP client request recording.
CHANGELOG.md	Documents addition of scaler HTTP request metrics.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

wozniakjan

Thanks, great addition! Just please see the copilot review above and my nits below

wozniakjan · 2026-04-22T09:33:34Z

/run-e2e
Update: You can check the progress here

JorTurFer · 2026-04-22T12:31:59Z

Hello
Even I like the idea, personally I don't like to support this metric by our own but just instrument the HTTP client using any already existing solution like
https://pkg.go.dev/go.opentelemetry.io/contrib/instrumentation/net/http/otelhttp#pkg-examples

The potential cardinality that we can generate if we record metrics with all these labels is huge

durationSeconds float64, statusCode int, isError bool, scaler, triggerName, metricName, namespace, scaledResource string)

wozniakjan · 2026-04-22T14:28:26Z

worth discussing, with third-party tools we would lose granularity and per SO / trigger information but perhaps that is acceptable? There are imho two points where we currently lack consensus:

A) gauge vs. histogram

gauge pros
- the metric samples are few, by default mostly exactly one per scraping period
- smaller memory footprint
- KEDA convention
histogram pros
- when aggregated across SOs, allows advanced analysis
- prometheus convention for latency metrics

B) custom HTTP metrics vs. third-party package

custom pros
- fine granularity with KEDA-relevant labels
third-party package pros
- industry standard and users' familiarity with it

any other arguments worth adding?

The potential cardinality that we can generate if we record metrics with all these labels is huge

theoretical cardinality yes, also amplified by buckets for sure. But practically, I think the estimates might be less dramatic and frequently similar to other KEDA metrics

statusCode - prometheus typically for queries responds - 200, 400, 422, 503, other scalers for query API are likely also bounded by some reasonable status count
scaler, triggerName, metricName - these are not orthogonal, there will be correlation, so it's not a cartesian product but rather the max of the three and other two serve the purpose of UX and discoverability
namespace, scaledResource - other metrics such as keda_scaler_metrics_value already have these labels so the metrics included here will grow proportionally to the size of the environment, similarly to some already existing metrics
imho the biggest question mark remains histogram vs gauge and how much it affects metric growth

aliaqel-stripe · 2026-04-23T20:42:39Z

@JorTurFer @wozniakjan ready for second review

rickbrouwer · 2026-05-07T05:44:46Z

…bel precedence, and histogram recording Signed-off-by: Ali Aqel <[email protected]>

…ey-value pairs Signed-off-by: Ali Aqel <[email protected]>

…g metric fetches Signed-off-by: Ali Aqel <[email protected]>

…on failures Signed-off-by: Ali Aqel <[email protected]>

Signed-off-by: Ali Aqel <[email protected]>

The duration histogram keda_scaler_http_request_duration_seconds previously carried 6 label dimensions (namespace, scaled_resource, scaler, trigger_name, metric_name, status_code), creating high MTS cardinality. Latency by scaler type is what matters, so the histogram is reduced to 2 labels: scaler and status_code. The counter retains all 6 labels. Signed-off-by: Ali Aqel <[email protected]>

Signed-off-by: Ali Aqel <[email protected]>

- Rename CreateHTTPTransport/CreateHTTPTransportWithTLSConfig to CreateRT/CreateRTWithTLSConfig to reflect they return http.RoundTripper - Fix OTel metric description: "labeled by status class" → "labeled by HTTP status code" - Fix doc comments on RecordHTTPClientRequest to accurately describe that context keys are extracted by InstrumentedRoundTripper, not the collector - Fix test comments and rename TestInstrumentedRoundTripper_ScalerContextKey to TestInstrumentedRoundTripper_AllContextKeys with all five required context keys set Signed-off-by: Ali Aqel <[email protected]>

The return type change (*http.Transport -> http.RoundTripper) is non-breaking for all callers since http.Client.Transport and all struct fields receiving the value are already typed as http.RoundTripper. No caller accesses *http.Transport-specific fields, so the rename to CreateRT* is unnecessary churn. Signed-off-by: Ali Aqel <[email protected]>

…ipper return type CreateHTTPTransport and CreateHTTPTransportWithTLSConfig now return http.RoundTripper (wrapping InstrumentedRoundTripper), so the *Transport suffix is misleading. Rename to CreateRT / CreateRTWithTLSConfig. All callers assign to http.Client.Transport or struct fields typed as http.RoundTripper, so the return type change is non-breaking. Signed-off-by: Ali Aqel <[email protected]>

Signed-off-by: Ali Aqel <[email protected]>

rickbrouwer · 2026-05-12T16:38:38Z

aliaqel-stripe · 2026-05-13T15:36:37Z

@rickbrouwer is this one ready to go now?

The metrics InstrumentedRoundTripper (added in kedacore#7644) wraps the scaler HTTP transport but only implemented RoundTrip. http.Client.CloseIdleConnections() only reaches the underlying transport when the client's Transport itself implements CloseIdleConnections(), so wrapping the *http.Transport turned every httpClient.CloseIdleConnections() call into a silent no-op. As a result, scaler Close()/refreshScaler stopped releasing idle keep-alive connections. With the default transport (keep-alive on, IdleConnTimeout == 0) those connections never expire, so on any scaler error/rebuild the old pool's idle connections leak and accumulate against the upstream (e.g. Prometheus), eventually overwhelming it. This regressed v2.19.0 behaviour, where the client's Transport was a bare *http.Transport that implements CloseIdleConnections(). Forward CloseIdleConnections() to the wrapped RoundTripper so idle-connection cleanup works through the wrapper again, for every scaler that goes through kedautil.CreateHTTPClient. Add regression tests. Fixes kedacore#7898 Signed-off-by: Ahmad Al-Masry <[email protected]>

Since kedacore#7644 kedautil.CreateHTTPClient wraps the scaler HTTP transport in metricscollector.InstrumentedRoundTripper, which only implements RoundTrip. http.Client.CloseIdleConnections() forwards to the transport only when it implements CloseIdleConnections(), so every scaler cleanup call became a silent no-op and idle keep-alive connections accumulated against the upstream (e.g. Prometheus) on every scaler Close() / refresh. Add a CloseIdleConnections() passthrough on InstrumentedRoundTripper using the same interface assertion the standard library uses, restoring the pre-v2.20 cleanup behaviour. Fixes kedacore#7898 Signed-off-by: Mahesh Sadupalli <[email protected]>

The metrics InstrumentedRoundTripper (added in kedacore#7644) wraps the scaler HTTP transport but only implemented RoundTrip. http.Client.CloseIdleConnections() only reaches the underlying transport when the client's Transport itself implements CloseIdleConnections(), so wrapping the *http.Transport turned every httpClient.CloseIdleConnections() call into a silent no-op. As a result, scaler Close()/refreshScaler stopped releasing idle keep-alive connections. With the default transport (keep-alive on, IdleConnTimeout == 0) those connections never expire, so on any scaler error/rebuild the old pool's idle connections leak and accumulate against the upstream (e.g. Prometheus), eventually overwhelming it. This regressed v2.19.0 behaviour, where the client's Transport was a bare *http.Transport that implements CloseIdleConnections(). Forward CloseIdleConnections() to the wrapped RoundTripper so idle-connection cleanup works through the wrapper again, for every scaler that goes through kedautil.CreateHTTPClient. Add regression tests. Fixes kedacore#7898 Signed-off-by: Ahmad Al-Masry <[email protected]>

aliaqel-stripe requested a review from a team as a code owner April 13, 2026 01:30

keda-automation requested a review from a team April 13, 2026 01:30

aliaqel-stripe force-pushed the feat/http-client-metrics branch from 77741df to 82d1237 Compare April 13, 2026 01:37

aliaqel-stripe changed the title ~~feat: add HTTP client request metrics for scaler metric fetches~~ add HTTP client request metrics for scaler metric requests Apr 13, 2026

aliaqel-stripe mentioned this pull request Apr 13, 2026

add keda_scaler_http_requests_total and keda_scaler_http_request_duration_seconds metrics kedacore/keda-docs#1734

Merged

1 task

aliaqel-stripe force-pushed the feat/http-client-metrics branch from fdb37a6 to 9c79fce Compare April 13, 2026 16:05

wozniakjan requested a review from Copilot April 21, 2026 13:29

Copilot started reviewing on behalf of wozniakjan April 21, 2026 13:30 View session

Copilot AI reviewed Apr 21, 2026

View reviewed changes

wozniakjan reviewed Apr 21, 2026

View reviewed changes

Comment thread pkg/util/http.go Outdated

Comment thread pkg/util/http.go Outdated

keda-automation requested a review from a team April 22, 2026 04:01

wozniakjan approved these changes Apr 22, 2026

View reviewed changes

wozniakjan added the Awaiting/2nd-approval This PR needs one more approval review label Apr 22, 2026

wozniakjan added needs-discussion and removed Awaiting/2nd-approval This PR needs one more approval review labels Apr 22, 2026

keda-automation requested a review from a team April 23, 2026 04:58

JorTurFer approved these changes Apr 26, 2026

View reviewed changes

JorTurFer reviewed Apr 26, 2026

View reviewed changes

Comment thread pkg/metricscollector/opentelemetry.go

JorTurFer approved these changes Apr 26, 2026

View reviewed changes

aliaqel-stripe mentioned this pull request Apr 26, 2026

feat(metrics): add histograms for scaler and internal-scale-loop latency #7682

Open

keda-automation requested a review from a team May 6, 2026 22:27

rickbrouwer added ok-to-merge This PR can be merged waiting-for-e2e labels May 7, 2026

aliaqel-stripe added 18 commits May 12, 2026 15:18

test: add missing tests for transport instrumentation, status code la…

1847695

…bel precedence, and histogram recording Signed-off-by: Ali Aqel <[email protected]>

test(scaling): verify buildScalerRequestCtx injects correct context k…

493dc9c

…ey-value pairs Signed-off-by: Ali Aqel <[email protected]>

refactor(metrics): rename to keda_scaler_http_* and only record durin…

d612545

…g metric fetches Signed-off-by: Ali Aqel <[email protected]>

fix(metrics): use accurate log message for OTel instrument registrati…

2f574dd

…on failures Signed-off-by: Ali Aqel <[email protected]>

chore: update copyright year to 2026 on new files

f6b6532

Signed-off-by: Ali Aqel <[email protected]>

chore: use "scaler metric collection" instead of "scaler metric fetch"

5056e1b

Signed-off-by: Ali Aqel <[email protected]>

fix: gofmt and CHANGELOG sort order for static checks

d11840d

Signed-off-by: Ali Aqel <[email protected]>

ci: retrigger static checks

b7d7393

Signed-off-by: Ali Aqel <[email protected]>

docs: update RecordHTTPClientRequest comment per Copilot suggestion

936637e

Signed-off-by: Ali Aqel <[email protected]>

refactor: move HTTP client metrics helpers into metricscollector

8e498ba

Signed-off-by: Ali Aqel <[email protected]>

refactor: colocate HTTP client metrics helpers

c4bb784

Signed-off-by: Ali Aqel <[email protected]>

Fix http roundtripper test bodyclose issues

aa321da

Signed-off-by: Ali Aqel <[email protected]>

Rename roundtripper locals to rt

4d194a3

Signed-off-by: Ali Aqel <[email protected]>

Rename HTTP client transport local to rt

ccc3155

Signed-off-by: Ali Aqel <[email protected]>

aliaqel-stripe force-pushed the feat/http-client-metrics branch from 0c742ca to ccc3155 Compare May 12, 2026 15:25

aliaqel-stripe mentioned this pull request May 12, 2026

Add flag to enable high cardinality metrics labels #7731

Open

wozniakjan merged commit d6740bc into kedacore:main May 20, 2026
23 checks passed

rickbrouwer mentioned this pull request May 25, 2026

Add OpenSearch scaler #7540

Merged

8 tasks

AhmadMasry mentioned this pull request Jul 1, 2026

KEDA is killing Prometheus: HTTP scalers leak idle connections since v2.20 (InstrumentedRoundTripper drops CloseIdleConnections, #7644) #7898

Open

mahesh-sadupalli mentioned this pull request Jul 3, 2026

fix: forward CloseIdleConnections through InstrumentedRoundTripper #7901

Closed

8 tasks

AhmadMasry mentioned this pull request Jul 3, 2026

fix: forward CloseIdleConnections through InstrumentedRoundTripper #7902

Open

8 tasks

Uh oh!

Conversation

aliaqel-stripe commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Changes

Checklist

Uh oh!

snyk-io Bot commented Apr 13, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Snyk checks have passed. No issues have been found so far.

Uh oh!

github-actions Bot commented Apr 13, 2026

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

wozniakjan left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

wozniakjan commented Apr 22, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

JorTurFer commented Apr 22, 2026

Uh oh!

wozniakjan commented Apr 22, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

A) gauge vs. histogram

B) custom HTTP metrics vs. third-party package

Uh oh!

aliaqel-stripe commented Apr 23, 2026

Uh oh!

Uh oh!

rickbrouwer commented May 7, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

rickbrouwer commented May 12, 2026 • edited by github-actions Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

aliaqel-stripe commented May 13, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

aliaqel-stripe commented Apr 13, 2026 •

edited

Loading

snyk-io Bot commented Apr 13, 2026 •

edited

Loading

wozniakjan commented Apr 22, 2026 •

edited by github-actions Bot

Loading

wozniakjan commented Apr 22, 2026 •

edited

Loading

rickbrouwer commented May 7, 2026 •

edited by github-actions Bot

Loading

rickbrouwer commented May 12, 2026 •

edited by github-actions Bot

Loading