feat(redis_interface): redis metrics addition#12656
Conversation
Changed Files
|
XyneSpaces
left a comment
There was a problem hiding this comment.
Review Summary
Classification: Observability Enhancement — Redis Per-Operation Metrics
Risk Level: Low
Files Changed: 5 files (new metrics module, instrumentation across both Redis backends)
Findings Overview
| Severity | Count | Issue |
|---|---|---|
| 1 | Metric double-counting in multitenancy fallback path | |
| 💡 | 1 | Inefficient operation label allocation |
Tier 1 — Blocking Issues
None found. Core implementation is sound.
Tier 2 — Warning Issues
In fred/commands.rs, when multitenancy_fallback is enabled and the primary tenant-aware key fails, the fallback path performs a second Redis call with the same track_redis_call(RedisOperation::GetKey, ...) wrapper. This results in two metric increments for a single logical operation.
Consider adding a dedicated RedisOperation::GetKeyFallback variant or tracking fallbacks separately to preserve metric accuracy.
Tier 3 — Observations
💡 Operation label uses heap allocation per call
The format!("{operation:?}") in metrics.rs:64 allocates a String on every Redis operation. Since all variants are statically known, implement AsRef<str> or a name() method on RedisOperation that returns &'static str:
impl RedisOperation {
const fn as_str(&self) -> &'static str { ... }
}This avoids the allocation and Debug formatting overhead on the hot path.
Positive Notes
- Consistent instrumentation across both
fredandredis_rsbackends - Proper error context preservation with
.change_context() #[instrument]already present on instrumented functions- Clean separation of metrics logic into dedicated module
Verdict: Address the Tier 2 double-counting concern before merge.
XyneSpaces
left a comment
There was a problem hiding this comment.
Review Summary
Classification: Feature — Redis Metrics Addition
Risk Level: Low-Medium (infrastructure observability, affects all Redis operations)
Files Changed: 6 files (new metrics module, Cargo.toml updates, fred + redis_rs backends)
Baseline Review (B1–B7)
✅ B1. File coverage — All changed files reviewed.
✅ B2. Code integrity — No panics, unwraps, or debug remnants found.
✅ B3. Code correctness — Proper error_stack chain preservation throughout.
✅ B4. Companion sync — Metrics module properly exported in lib.rs.
✅ B5. Instrumentation — #[instrument] present on async functions.
Tier 1 — Blocking Issues
No blocking issues found.
Tier 2 — Medium Signal Observations
operation label
The RedisOperation enum has 38 variants. All operations will be tagged with the operation label derived from Debug format. Verify that your metrics backend can handle this cardinality comfortably, especially if additional operations are added frequently.
Fix consideration: Document the expected cardinality in metrics documentation.
// Current approach uses Debug formatting for operation name
let attributes = router_env::metric_attributes!(("operation", format!("{operation:?}")));Tier 3 — Nits
🔍 Debug format for metric label may have unintended capitalization
Using format!("{operation:?}") on a Debug derived enum produces PascalCase variant names (e.g., "SetKey", "GetMultipleKeys"). While consistent, verify this matches the naming convention expected by dashboards.
Consider adding a dedicated as_str() method for explicit control over metric label names:
impl RedisOperation {
fn as_str(&self) -> &'static str {
match self {
Self::SetKey => "set_key",
Self::GetMultipleKeys => "get_multiple_keys",
// ...
}
}
}🔍 Double-counting potential in multitenancy fallback paths
In get_key and similar functions, the track_redis_call is invoked separately for tenant-aware and tenant-unaware fallback calls. This means a single logical "get" may produce 2 metric increments during migration scenarios with multitenancy_fallback enabled.
Document this behavior so operators understand metric spikes during tenant migration periods.
Positive Notes
- Clean separation of metrics logic into dedicated module
- Comprehensive coverage of all Redis operations (38 variants)
- Proper wrapping pattern with
track_redis_call()— consistent across both backends - No disruption to existing error handling chains
- Uses
std::time::Instantfor monotonic timing
Verdict: ✅ Clean implementation — redis metrics are additive with no functional changes.
SanchithHegde
left a comment
There was a problem hiding this comment.
Other than that, looks good to me!
Type of Change
Description
Adds per-operation metrics to
redis_interface.Function
track_redis_call(operation, future)(times the call, logs op + duration) and a sharedRedisOperationenum added for every Redis command in both backends (redis_rs+fred) is routed through it.Two metrics are recorded on the
ROUTER_APImeter, labelled byoperation:REDIS_CALLS_COUNT— counter →router_REDIS_CALLS_COUNT_total{operation="..."}REDIS_CALL_TIME— latency histogram →router_REDIS_CALL_TIME_*{operation="..."}Additional Changes
Motivation and Context
Redis interface had no metrics — it was a blind spot from monitoring perspective. This adds per-operation visibility, enabling call-rate/latency metrics for observability.
How did you test it?
Logs
2026-06-09T09:26:57.536773Z DEBUG redis_interface::metrics: Redis operation executed, redis_operation: GetMultipleKeys, execution_time: 743.333µs at crates/redis_interface/src/metrics.rs:62 in redis_interface::module::redis_rs::commands::get_multiple_keys_with_mget with keys: [RedisKey("API_LOCK_merchant_1780945287_payments_pay_0e8x8tYElfG5sy3Des0b"), RedisKey("API_LOCK_merchant_1780945287_payments_wertyuiuhgfcvghjkjhgfdgyuijhgfcvbhj")] in redis_interface::module::redis_rs::commands::get_keys_by_mode with keys: [RedisKey("API_LOCK_merchant_1780945287_payments_pay_0e8x8tYElfG5sy3Des0b"), RedisKey("API_LOCK_merchant_1780945287_payments_wertyuiuhgfcvghjkjhgfdgyuijhgfcvbhj")] in redis_interface::module::redis_rs::commands::get_multiple_keys with keys: [RedisKey("API_LOCK_merchant_1780945287_payments_pay_0e8x8tYElfG5sy3Des0b"), RedisKey("API_LOCK_merchant_1780945287_payments_wertyuiuhgfcvghjkjhgfdgyuijhgfcvbhj")] in router::core::api_locking::free_lock_action in router::services::api::server_wrap_util with flow: PaymentsCreate, lock_action: HoldMultiple { inputs: [LockingInput { unique_locking_key: "pay_0e8x8tYElfG5sy3Des0b", api_identifier: Payments, override_lock_retries: None }, LockingInput { unique_locking_key: "wertyuiuhgfcvghjkjhgfdgyuijhgfcvbhj", api_identifier: Payments, override_lock_retries: None }] }, merchant_id: "merchant_1780945287" in router::services::api::server_wrap with flow: PaymentsCreate, lock_action: HoldMultiple { inputs: [LockingInput { unique_locking_key: "pay_0e8x8tYElfG5sy3Des0b", api_identifier: Payments, override_lock_retries: None }, LockingInput { unique_locking_key: "wertyuiuhgfcvghjkjhgfdgyuijhgfcvbhj", api_identifier: Payments, override_lock_retries: None }] }, request_method: "POST", request_url_path: "/payments" in router::routes::payments::payments_create with flow: PaymentsCreate, payment_id: "pay_0e8x8tYElfG5sy3Des0b" in router::middleware::ROOT_SPAN with flow: "UNKNOWN", tenant_id: "public", tenant_id: "public" in router_env::root_span::HTTP request with http.method: POST, http.route: /payments, http.flavor: 1.1, http.scheme: http, http.host: localhost:9080, http.client_ip: ::1, http.user_agent: PostmanRuntime/7.44.1, http.target: /payments, otel.name: POST /payments, otel.kind: "server", request_id: 019eabb4-d0ed-73d0-b1b7-129d393d784e, request_id: "019eabb4-d0ed-73d0-b1b7-1288d500a6a5", trace_id: 00000000000000000000000000000000 2026-06-09T09:26:57.537574Z DEBUG redis_interface::metrics: Redis operation executed, redis_operation: DeleteKey, execution_time: 550.542µs at crates/redis_interface/src/metrics.rs:62 in redis_interface::module::redis_rs::commands::delete_key with key: RedisKey("API_LOCK_merchant_1780945287_payments_pay_0e8x8tYElfG5sy3Des0b") in redis_interface::module::redis_rs::commands::delete_multiple_keys with keys: [RedisKey("API_LOCK_merchant_1780945287_payments_pay_0e8x8tYElfG5sy3Des0b"), RedisKey("API_LOCK_merchant_1780945287_payments_wertyuiuhgfcvghjkjhgfdgyuijhgfcvbhj")] in router::core::api_locking::free_lock_action in router::services::api::server_wrap_util with flow: PaymentsCreate, lock_action: HoldMultiple { inputs: [LockingInput { unique_locking_key: "pay_0e8x8tYElfG5sy3Des0b", api_identifier: Payments, override_lock_retries: None }, LockingInput { unique_locking_key: "wertyuiuhgfcvghjkjhgfdgyuijhgfcvbhj", api_identifier: Payments, override_lock_retries: None }] }, merchant_id: "merchant_1780945287" in router::services::api::server_wrap with flow: PaymentsCreate, lock_action: HoldMultiple { inputs: [LockingInput { unique_locking_key: "pay_0e8x8tYElfG5sy3Des0b", api_identifier: Payments, override_lock_retries: None }, LockingInput { unique_locking_key: "wertyuiuhgfcvghjkjhgfdgyuijhgfcvbhj", api_identifier: Payments, override_lock_retries: None }] }, request_method: "POST", request_url_path: "/payments" in router::routes::payments::payments_create with flow: PaymentsCreate, payment_id: "pay_0e8x8tYElfG5sy3Des0b" in router::middleware::ROOT_SPAN with flow: "UNKNOWN", tenant_id: "public", tenant_id: "public" in router_env::root_span::HTTP request with http.method: POST, http.route: /payments, http.flavor: 1.1, http.scheme: http, http.host: localhost:9080, http.client_ip: ::1, http.user_agent: PostmanRuntime/7.44.1, http.target: /payments, otel.name: POST /payments, otel.kind: "server", request_id: 019eabb4-d0ed-73d0-b1b7-129d393d784e, request_id: "019eabb4-d0ed-73d0-b1b7-1288d500a6a5", trace_id: 00000000000000000000000000000000 2026-06-09T09:26:57.537657Z DEBUG redis_interface::metrics: Redis operation executed, redis_operation: DeleteKey, execution_time: 599.959µs at crates/redis_interface/src/metrics.rs:62 in redis_interface::module::redis_rs::commands::delete_key with key: RedisKey("API_LOCK_merchant_1780945287_payments_wertyuiuhgfcvghjkjhgfdgyuijhgfcvbhj") in redis_interface::module::redis_rs::commands::delete_multiple_keys with keys: [RedisKey("API_LOCK_merchant_1780945287_payments_pay_0e8x8tYElfG5sy3Des0b"), RedisKey("API_LOCK_merchant_1780945287_payments_wertyuiuhgfcvghjkjhgfdgyuijhgfcvbhj")] in router::core::api_locking::free_lock_action in router::services::api::server_wrap_util with flow: PaymentsCreate, lock_action: HoldMultiple { inputs: [LockingInput { unique_locking_key: "pay_0e8x8tYElfG5sy3Des0b", api_identifier: Payments, override_lock_retries: None }, LockingInput { unique_locking_key: "wertyuiuhgfcvghjkjhgfdgyuijhgfcvbhj", api_identifier: Payments, override_lock_retries: None }] }, merchant_id: "merchant_1780945287" in router::services::api::server_wrap with flow: PaymentsCreate, lock_action: HoldMultiple { inputs: [LockingInput { unique_locking_key: "pay_0e8x8tYElfG5sy3Des0b", api_identifier: Payments, override_lock_retries: None }, LockingInput { unique_locking_key: "wertyuiuhgfcvghjkjhgfdgyuijhgfcvbhj", api_identifier: Payments, override_lock_retries: None }] }, request_method: "POST", request_url_path: "/payments" in router::routes::payments::payments_create with flow: PaymentsCreate, payment_id: "pay_0e8x8tYElfG5sy3Des0b" in router::middleware::ROOT_SPAN with flow: "UNKNOWN", tenant_id: "public", tenant_id: "public" in router_env::root_span::HTTP request with http.method: POST, http.route: /payments, http.flavor: 1.1, http.scheme: http, http.host: localhost:9080, http.client_ip: ::1, http.user_agent: PostmanRuntime/7.44.1, http.target: /payments, otel.name: POST /payments, otel.kind: "server", request_id: 019eabb4-d0ed-73d0-b1b7-129d393d784e, request_id: "019eabb4-d0ed-73d0-b1b7-1288d500a6a5", trace_id: 00000000000000000000000000000000Checklist
cargo +nightly fmt --allcargo clippy