[caroolcanelas2]

What is the purpose of this pull request?

Record the outbound HTTP verb on http_client_requests_total, so o11y Query metrics can read Attributes['method'] for calls emitted after this version of @vtex/api is deployed.

What problem is this solving?

XTNSNS-2594, split out of XTNSNS-2507. Attributes['method'] is an empty string because metricsMiddleware never writes the key. The Read API only projects the map already stored in ClickHouse. The other four Query metrics symptoms stay in the Read API subtasks and are not part of this change.

How should this be manually tested?

• yarn test src/HttpClient/middlewares/metrics.test.ts src/service/worker/runtime/http/middlewares/timings.test.ts --watchman=false
• After @vtex/api is published and an app runs it, Query metrics on http_client_requests_total should return get, post, put, patch, delete, or head in Attributes['method'].
• Samples written before the deploy still have no key and still read as an empty string.

Screenshots or example usage

No UI change.

Types of changes

• Bug fix (a non-breaking change which fixes an issue)
• New feature (a non-breaking change which adds functionality)
• Breaking change (fix or feature that would cause existing functionality to change)
• Requires change to documentation, which has been updated accordingly.

Summary

• US-1: metricsMiddleware adds method from ctx.config.method, falling back to get when the value is missing or empty. HttpClient.get does not set the field. The same map feeds the latency histogram, http_client_cache_total, and http_client_requests_retried_total. The metrics catalog lists method on those rows.
• US-2: timings.ts is unchanged. Its inbound base attributes still omit method, so a handler that received GET does not overwrite an outbound post.

Tests

• metricsMiddleware records post on success, put when the call fails, patch on the retry counter, and get when method is omitted or empty.
• timings locks the inbound base map and http_handler_requests_total so they do not carry method even when the incoming request method is GET.
• Full Jest suite: 15 suites passed, 236 tests passed, 24 skipped.

ClickHouse before the change

ClickHouse after the change

Assumptions

• The stored verb stays lowercase, matching HttpClient (post, put, …). The GET / POST strings from the local ClickHouse simulation were written by that simulation.
• A failed call still uses the existing status selection. The error scenario asserts method and status: error. It does not require status_code to switch to the rejected response when ctx.response.status is already set.
• Rows already in ClickHouse are not rewritten. This repo has no path that backfills them, so that scenario is not a unit test.

Follow-ups

• Query metrics only changes after this library is published and apps pick it up.
• method brings the client custom attribute map to 6 keys, and to 7 when cache_state or retry_count is also set. That is the current cap in DiagnosticsMetrics.

Spec

specs/http-client-requests-method.md

Made with Cursor

[Copilot]

🟢 Approval recommended

The narrow change matches the documented contract, preserves inbound behavior, stays within the attribute limit, and has focused regression coverage.

No open issues 🎉

What changed in this PR

Adds the outbound HTTP method to http_client_requests_total and related client metrics, enabling Query metrics to group and filter newly emitted samples by verb.

Changes:

• Records ctx.config.method, defaulting to get when missing or empty.
• Adds regression coverage for success, failure, retries, defaults, and exclusion of the inbound method.
• Updates the metrics catalog and documents the emission contract and rollout requirements.

File	Description
src/​service/​worker/​runtime/​http/​middlewares/​timings.test.ts	Verifies inbound metrics do not include method.
src/​HttpClient/​middlewares/​metrics.ts	Adds the outbound method to shared client attributes.
src/​HttpClient/​middlewares/​metrics.test.ts	Tests method recording and fallback behavior.
specs/​http-client-requests-method.md	Documents requirements, scope, and rollout behavior.
docs/​METRICS_CATALOG.md	Lists method on the four affected instruments.

_{Review effort: Balanced}

[dk-pr-review]

DK Review — Audit Summary

Verdict: ✅ CLEAN

Severity	Count
BLOCK	0
RESTRICT	0
SUGGEST	3

Only BLOCK and RESTRICT findings anchored to a changed line are commented inline on the diff. The 3 findings below open no review thread.

Scenarios evaluated: feature-flags, general-review, quality-ratchet
Scenarios skipped: dependency-governance (no matching files), pipeline-config (no matching files), agent-skills-review (no matching files)

---

📋 Findings (3)

SUGGEST

1. [general.broken-references] specs/http-client-requests-method.md:5 — The spec links numerous external Jira tickets (XTNSNS-2594, XTNSNS-2507, XTNSNS-1311, XTNSNS-2590/2591/2592/2593) and a GitHub repo (vtex/node-vtex-api) that cannot be verified to exist or to resolve to the content described.
   → Double-check that each Jira ticket ID and the GitHub repo link are correct and accessible before merging the spec.
2. [Functional.Check] src/HttpClient/middlewares/metrics.ts:128 — method: ctx.config.method || 'get' is used directly as a metric label without case normalization. If callers set config.method with inconsistent casing (e.g. 'GET' vs 'get' vs 'Get'), the same logical HTTP method will be split across multiple label values, fragmenting counters/histograms (http_client_requests_total, latency, http_client_cache_total, http_client_requests_retried_total) and breaking aggregation/alerting on these metrics.
   → Normalize the method value (e.g. (ctx.config.method || 'get').toLowerCase()) before using it as a metric attribute, and add a test case covering mixed-case input.
3. [Functional.LargerDefect] src/service/worker/runtime/http/middlewares/timings.test.ts:105 — This PR adds the method dimension to outbound HttpClient metrics but explicitly locks in (via a new test) that inbound request-handler metrics (http_handler_requests_total, base attributes) do NOT record method. This creates an instrumentation asymmetry: operators can break down outbound call latency/errors by HTTP method but not inbound handler latency/errors by method, limiting diagnosability of the inbound path.
   → Confirm this omission is an intentional, documented scope decision; if not, consider adding the inbound request method as a metric attribute for parity with the outbound change.

---

DK Review v1.0.0 | To dismiss a finding: reply /dk-review dismiss [reason] under its comment

[sonar-workflows]

Quality Gate failed

• 7 New Issues (is greater than 0)
• 33.30% Coverage on New Code (is less than 75.00%)

Project ID: node-vtex-api

View in SonarQube