Description

Fixes two bugs in the router's diagnostics telemetry.

#7260: failure counts are wiped whenever a new error response appears

The guard in commitStatusList checked whether the error response key existed instead of the event key. Each new error response replaced the event's whole inner map, so failuresMetric[event] never held more than one entry and the reported count was always 1.

• The update now lives in trackFailureMetrics(event, errorResponse), next to trackRequestMetrics, and checks the event key.
• TestTrackFailureMetrics covers it. It fails on the old guard: router_failed ends up as {"timeout": 1} instead of {"connection refused": 1, "timeout": 3}.

#7261: requestsMetric is reset while holding only a read lock

collectMetrics set requestsMetric = nil under RLock. Each router runs a single collectMetrics goroutine today, so this doesn't race in practice, but it breaks the sync.RWMutex contract.

• takeRequestMetrics now swaps and resets the slice under Lock, and collectMetrics aggregates the local copy, so Diagnostics.Track is no longer called with a lock held and workers calling trackRequestMetrics don't wait on it.
• Both telemetry locks are now sync.Mutex: neither requestsMetricLock nor failureMetricLock is ever read-locked, so reintroducing the misuse would be a compile error.
• TestTakeRequestMetrics covers it: the swap hands over everything collected so far and leaves the next batch empty, and concurrent takers and writers neither lose nor double-count a metric. Testing collectMetrics itself would mean mutating the package-level diagnostics globals, which no router test does today, so the helper is the seam.

This supersedes #7265 and #7266 by @harsh4vardhan, which took the same approach and were closed by the stale bot.

Testing

utils/misc doesn't build on Windows, so I ran the checks on Linux in GitHub Actions on my fork: run 35045981108. The workflow lives on a throwaway branch and is not part of this PR; it mirrors verify.yml and the Package Unit [ router ] job in tests.yaml.

• TestTrackFailureMetrics with the old guard put back: fails

  expected: map[string]map[string]int{"router_aborted":map[string]int{"bad request":1}, "router_failed":map[string]int{"connection refused":1, "timeout":3}}
  actual  : map[string]map[string]int{"router_aborted":map[string]int{"bad request":1}, "router_failed":map[string]int{"timeout":1}}
  

• go test -race -run '^(TestTrackFailureMetrics|TestTakeRequestMetrics|TestUpdateRudderSourcesStats_.*)$' ./router/ with the fix: ok github.com/rudderlabs/rudder-server/router 1.130s
• go mod tidy, make mocks, make fmt: no diff
• golangci-lint v2.13.2: no issues
• make test exclude="router/batchrouter/asyncdestinationmanager/bing-ads" package=router: every package passes (1232 tests: 1229 passed, 3 skipped). The skips are TestStreamWriterIntegration and TestAPIIntegration, which need CI credentials, and TestUploadIntegration, which is skipped in the code. FORCE_RUN_INTEGRATION_TESTS was left unset since the fork has no CI secrets. Per-test results match the same job run on upstream master (80353d18) exactly, apart from the two tests this PR adds.
• Coverage of the touched functions: trackFailureMetrics 100%, takeRequestMetrics 100%, commitStatusList 90.3%. collectMetrics stays where it was (15%): it reads the package-level diagnostics globals, so the helpers are the testable seam.

Linear Ticket

N/A (external contribution). Closes #7260, closes #7261.

Security

• The code changed/added as part of this pull request won't create any security issues with how the software is being used.