feat: track card-info lookup failures with Prometheus metrics and structured logs - #408
Open
GauravRawat369 wants to merge 9 commits into
Open
feat: track card-info lookup failures with Prometheus metrics and structured logs#408GauravRawat369 wants to merge 9 commits into
GauravRawat369 wants to merge 9 commits into
Conversation
Contributor
There was a problem hiding this comment.
🟡 Changes recommended
The new instrumentation currently risks persisting/logging sensitive data (Secret-typed fields and full URLs/BINs) and can read large non-2xx response bodies fully into memory before truncation.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
This PR adds failure instrumentation around the outbound card-info (BIN metadata) lookup, emitting analytics events that can be monitored in ClickHouse/Grafana, and slightly expands BIN enrichment debug logging in the decide-gateway flow.
Changes:
- Capture and record card-info lookup failures (timeouts, transport errors, non-2xx responses, parse errors) as domain analytics events, including latency and selected response metadata.
- Introduce a dedicated analytics
FlowType(card_info_lookup_error) andevent_stage(card_info_lookup) for alerting/query consistency. - Expand BIN enrichment debug logging to include
cardSwitchProvider.
File summaries
| File | Description |
|---|---|
| src/types/card/card_info_api.rs | Adds failure handling that records analytics events, including latency and selected response headers/body on non-2xx. |
| src/decider/gatewaydecider/flow_new.rs | Extends BIN enrichment debug log to include cardSwitchProvider. |
| src/analytics/service.rs | Adds a convenience emitter for card-info lookup failure events with a fixed route/flow context. |
| src/analytics/models.rs | Adds CARD_INFO_LOOKUP_STAGE constant used to tag these failure events. |
| src/analytics/flow.rs | Adds FlowType::CardInfoLookupError and its string mapping. |
| src/analytics/events.rs | Adds a new card_info_lookup_failure event constructor that sets stage/status/error fields. |
Review details
- Files reviewed: 6/6 changed files
- Comments generated: 3
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- don't log the Secret-typed cardSwitchProvider value; log only whether it was set - drop the BIN from the analytics details payload (BIN-free endpoint instead of the full URL, no duplicate bin key — card_is_in already carries it) and strip URLs from reqwest error messages via without_url() - read the non-2xx response body chunk-by-chunk capped at 2KB instead of buffering the whole body Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…ead of ClickHouse
Replace the ClickHouse domain-event emission with two Prometheus metrics that
VictoriaMetrics scrapes from /metrics:
- card_info_lookup_failures_total{error_code, upstream_code} — error_code is the
HTTP status or TIMEOUT/REQUEST_ERROR/PARSE_ERROR; upstream_code is the cards
API's own error code from the error body (e.g. IR_31), both bounded-cardinality
- card_info_lookup_failure_latency_seconds{error_code} histogram
The unbounded forensics (x-request-id, response headers, 2KB body snippet,
endpoint, latency) move onto the structured WARN log line. Removes the
CardInfoLookupError flow type and its analytics plumbing.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…rics card_info_lookup_failures_total gains an error_message label: the upstream error body's message for non-2xx, the reqwest description for transport/parse failures. Messages are sanitized to stay bounded-cardinality (serde's ' at line N column M' suffix stripped, 120-char cap); the unmodified message stays on the WARN log. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
…quest id Remove the Instant/latency capture (latency is no longer used anywhere since the histogram was dropped) and replace the response-header allowlist with just the upstream x-request-id. The decision engine's own request id is already recorded on the request tracing span, so the WARN line carries both ids for correlation. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
jagan-jaya
previously approved these changes
Sep 4, 2026
added 2 commits
September 8, 2026 14:22
… add_cards_info_to_clickhouse
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Tracks every failed outbound card-info (BIN metadata) lookup in
get_card_info_by_binwith Prometheus metrics (scraped by VictoriaMetrics from the existing/metricsendpoint) plus an information-loaded structured WARN log per failure. Successful (2xx) lookups intentionally emit nothing.Why
get_card_info_by_binfailures were only visible as bare WARN log lines. A degraded cards API (elevated 4xx/5xx, timeouts, throttling) silently disabled BIN enrichment with no way to chart or alert on it.How
Metrics (bounded-cardinality labels only):
card_info_lookup_failures_total{error_code, upstream_code}— counter.error_codeis the HTTP status ("400","503", …) orTIMEOUT/REQUEST_ERROR/PARSE_ERROR;upstream_codeis the cards API's own error code parsed from its error body (e.g.IR_31"IIN does not exist",IR_01"invalid API key"), empty when absent.card_info_lookup_failure_latency_seconds{error_code}— histogram (timeouts land at the configuredtimeout_mscap).error_code"400","503", …)card_info_service.timeout_ms)TIMEOUTREQUEST_ERRORPARSE_ERRORLogs carry the unbounded forensics that don't belong in metric labels: BIN-free endpoint, latency, upstream code, a 2KB response-body snippet (read chunk-by-chunk and capped, so large upstream error pages are never fully buffered), and an allowlist of useful response headers —
x-request-id(correlation with Hyperswitch logs),retry-after(throttling),x-envoy-upstream-service-time(origin vs. proxy latency), pluscontent-type,content-length,server,via,date,cf-ray.Sensitive-data hygiene: the BIN never appears in the endpoint field or metric labels, reqwest error messages are stripped of URLs via
without_url(), and thebinEnrichmentFromCardInfodebug log inflow_new.rslogs only whether theSecret-typedcardSwitchProviderwas set, never its value.No new infrastructure: the app already serves Prometheus text format on the metrics port; VictoriaMetrics/vmagent just scrapes it.
Grafana
Failure rate by error code (VictoriaMetrics datasource):
Alert condition (any failures in the last 5 minutes above threshold):
Latency p99 of failed calls:
Drill-down: the WARN log line (
tag: "cardInfoApi") withx-request-id, headers, and the upstream error body.Verification
Verified end-to-end locally:
POST /decide-gatewaywith a failing lookup producedcard_info_lookup_failures_total{error_code="401",upstream_code="IR_01"} 2and populated latency-histogram buckets on:9094/metrics, and the WARN log carried the endpoint, latency,x-request-id, full header set, and upstream error body.cargo buildclean; all 64 analytics unit tests pass.🤖 Generated with Claude Code