From 522605a055264a60492e3f8147adb3d1659e252c Mon Sep 17 00:00:00 2001 From: Henry Guo Date: Sat, 23 May 2026 23:23:05 +0800 Subject: [PATCH] test(internode): cover adapter metrics validation (#3071) Co-authored-by: Henry Guo --- .../internode-transport-adapter-rfc.md | 4 + .../transport-metrics-and-baseline.md | 143 ++++++++++++++++++ .../src/rpc/internode_data_transport.rs | 15 ++ crates/io-metrics/src/internode_metrics.rs | 39 +++++ 4 files changed, 201 insertions(+) create mode 100644 crates/ecstore/docs/internode-transport/transport-metrics-and-baseline.md diff --git a/crates/ecstore/docs/internode-transport/internode-transport-adapter-rfc.md b/crates/ecstore/docs/internode-transport/internode-transport-adapter-rfc.md index 50083af2f..0f2d116a2 100644 --- a/crates/ecstore/docs/internode-transport/internode-transport-adapter-rfc.md +++ b/crates/ecstore/docs/internode-transport/internode-transport-adapter-rfc.md @@ -38,6 +38,7 @@ Related design notes in this directory: - `transport-buffer-lifecycle.md` - `transport-buffer-contract.md` - `transport-fallback-and-selection.md` +- `transport-metrics-and-baseline.md` ## Open-source Scope @@ -335,6 +336,9 @@ S3 PUT/GET matrix against `local` and `distributed` scenarios and export: - `internode_metric_deltas.csv` (operation-level internode metric deltas when `--metrics-url` is provided) +See `transport-metrics-and-baseline.md` for current metric names, labels, +operation values, baseline inputs, and baseline artifact fields. + ## Transport Abstraction Proposal ### Design principle diff --git a/crates/ecstore/docs/internode-transport/transport-metrics-and-baseline.md b/crates/ecstore/docs/internode-transport/transport-metrics-and-baseline.md new file mode 100644 index 000000000..8580bf082 --- /dev/null +++ b/crates/ecstore/docs/internode-transport/transport-metrics-and-baseline.md @@ -0,0 +1,143 @@ +# Internode Transport Metrics and Baseline + +Status: current OSS behavior. This document describes the metrics and baseline +runner used to observe the existing `tcp-http` internode transport adapter. It +does not add a backend, change runtime behavior, or make performance claims. + +## Scope + +The current OSS adapter scope is: + +- `tcp-http` is the default and only supported internode data transport backend; +- `tcp` is a legacy alias for `tcp-http`; +- unsupported backend values fail closed during transport construction; +- metrics and baseline artifacts are used to understand the current data-plane + behavior. + +## Operation Metrics + +Operation-level internode metrics are defined in +`crates/io-metrics/src/internode_metrics.rs`. + +| Metric | Labels | Meaning | +| --- | --- | --- | +| `rustfs_system_network_internode_operation_sent_bytes_total` | `operation`, `backend` | Bytes sent for a known internode operation. | +| `rustfs_system_network_internode_operation_recv_bytes_total` | `operation`, `backend` | Bytes received for a known internode operation. | +| `rustfs_system_network_internode_operation_requests_outgoing_total` | `operation`, `backend` | Outgoing requests opened for a known internode operation. | +| `rustfs_system_network_internode_operation_requests_incoming_total` | `operation`, `backend` | Incoming requests handled for a known internode operation. | +| `rustfs_system_network_internode_operation_errors_total` | `operation`, `backend` | Errors observed for a known internode operation. | + +Current operation label values are: + +| Operation | Transport path | Notes | +| --- | --- | --- | +| `read_file_stream` | HTTP `/rustfs/rpc/read_file_stream` | Remote disk read stream opened by `InternodeDataTransport::open_read`. | +| `put_file_stream` | HTTP `/rustfs/rpc/put_file_stream` | Remote disk write stream opened by `InternodeDataTransport::open_write`. | +| `walk_dir` | HTTP `/rustfs/rpc/walk_dir` | Remote walk-dir stream opened by `InternodeDataTransport::open_walk_dir`. | +| `grpc_read_all` | gRPC `ReadAll` | Unary bytes response outside the adapter. | +| `grpc_write_all` | gRPC `WriteAll` | Unary bytes request outside the adapter. | + +Current backend label values are: + +| Backend | Meaning | +| --- | --- | +| `tcp-http` | Current HTTP stream backend for adapter-routed operations. | +| `grpc` | Current gRPC path for `ReadAll` and `WriteAll`. | +| `unknown` | Aggregate internode metric path when an operation-specific backend is not available. | + +The `backend` label reflects the current instrumentation source, for example +the `tcp-http` stream path, gRPC path, or aggregate unknown path. It does not +imply additional supported transport backends. + +Metric labels must stay low cardinality. Do not add object names, full paths, +full URLs, peer-specific dynamic strings, request IDs, or other per-request +values as labels. + +## Current Instrumentation Points + +| Area | File | Current signal | +| --- | --- | --- | +| Outgoing HTTP stream requests | `crates/rio/src/http_reader.rs` | Outgoing request count, sent bytes for writer bodies, received bytes for reader bodies, and errors for known `/rustfs/rpc/` operations. | +| Incoming HTTP stream requests | `rustfs/src/storage/rpc/http_service.rs` | Incoming request count, sent bytes for read and walk streams, received bytes for put streams, and route errors. | +| gRPC `ReadAll` / `WriteAll` | `rustfs/src/storage/rpc/disk.rs` | Incoming request count, sent/received bytes, and errors with the `grpc` backend label. | + +Known gaps: + +- Operation-level metrics do not currently expose latency histograms. +- The baseline runner reads Prometheus text output only when `--metrics-url` is + provided. +- Metrics are useful for attribution, but they do not replace end-to-end + correctness tests or object benchmark output. + +## Baseline Runner + +Use `scripts/run_internode_transport_baseline.sh` to run the current S3 PUT/GET +matrix against one or more configured endpoints. + +Required inputs: + +- `--access-key` +- `--secret-key` + +Common optional inputs: + +- `--tool warp|s3bench` +- `--scenarios name=url,...` +- `--sizes 4KiB,1MiB,...` +- `--concurrencies 1,16,...` +- `--duration 90s` +- `--metrics-url http://host:port/metrics` +- `--out-dir target/bench/internode-transport/manual-run` +- `--dry-run` + +Dry-run example: + +```bash +scripts/run_internode_transport_baseline.sh \ + --access-key minioadmin \ + --secret-key minioadmin \ + --scenarios local=http://127.0.0.1:9000,distributed=http://127.0.0.1:9001 \ + --sizes 4KiB,1MiB \ + --concurrencies 1 \ + --duration 10s \ + --dry-run +``` + +Real baseline example with metrics: + +```bash +RUSTFS_INTERNODE_DATA_TRANSPORT=tcp-http \ +scripts/run_internode_transport_baseline.sh \ + --access-key "$RUSTFS_ACCESS_KEY" \ + --secret-key "$RUSTFS_SECRET_KEY" \ + --scenarios local=http://127.0.0.1:9000,distributed=http://127.0.0.1:9001 \ + --metrics-url http://127.0.0.1:9000/metrics \ + --out-dir target/bench/internode-transport/manual-run +``` + +The real baseline requires a running RustFS deployment and working benchmark +tool. Do not invent or copy synthetic benchmark numbers into PR descriptions. + +## Output Artifacts + +The baseline runner writes: + +| File | Created when | Contents | +| --- | --- | --- | +| `run_manifest.txt` | Always | Timestamp, git commit, dirty flag, Rust compiler version, kernel string, scenario matrix, tool, workload settings, metrics URL, and redacted credentials. | +| `summary.csv` | Always | `scenario`, `endpoint`, `workload`, `concurrency`, `size`, `status`, `throughput`, `requests_per_sec`, `avg_latency`, `error_count`, `log_file`, and `run_dir`. | +| `internode_metric_deltas.csv` | When `--metrics-url` is set | `scenario`, `workload`, `concurrency`, `size`, metric name, operation, backend, before value, after value, and delta. | + +In dry-run mode, the runner still creates the manifest and CSV headers, and the +underlying object benchmark command is printed with credentials redacted. + +## Interpretation + +Use baseline artifacts to record and inspect the current `tcp-http` adapter +behavior across commits, environments, and scenario matrices. A baseline run is +valid only for the exact environment and command recorded in +`run_manifest.txt`. + +Baseline artifacts should state whether metrics were collected. When +`internode_metric_deltas.csv` is absent, benchmark output cannot attribute +internode operation deltas from Prometheus metrics. diff --git a/crates/ecstore/src/rpc/internode_data_transport.rs b/crates/ecstore/src/rpc/internode_data_transport.rs index bc61d89f8..144d79c14 100644 --- a/crates/ecstore/src/rpc/internode_data_transport.rs +++ b/crates/ecstore/src/rpc/internode_data_transport.rs @@ -328,12 +328,27 @@ mod tests { } } + #[test] + fn transport_config_known_backends_are_current_oss_values() { + assert_eq!( + KNOWN_INTERNODE_DATA_TRANSPORT_BACKENDS, + &[DEFAULT_INTERNODE_DATA_TRANSPORT, INTERNODE_DATA_TRANSPORT_TCP] + ); + + for configured in KNOWN_INTERNODE_DATA_TRANSPORT_BACKENDS { + let transport = build_internode_data_transport(Some(configured)).unwrap(); + + assert_eq!(transport.name(), DEFAULT_INTERNODE_DATA_TRANSPORT); + } + } + #[test] fn transport_config_rejects_unknown_backend() { let err = build_internode_data_transport(Some("unsupported-backend")).expect_err("unknown backend should fail closed"); assert!(err.to_string().contains(ENV_RUSTFS_INTERNODE_DATA_TRANSPORT)); assert!(err.to_string().contains("unsupported-backend")); + assert!(err.to_string().contains("supported values: tcp-http, tcp")); } #[test] diff --git a/crates/io-metrics/src/internode_metrics.rs b/crates/io-metrics/src/internode_metrics.rs index c7c0a5b3d..bf26133c9 100644 --- a/crates/io-metrics/src/internode_metrics.rs +++ b/crates/io-metrics/src/internode_metrics.rs @@ -308,4 +308,43 @@ mod tests { assert_eq!(metric.labels, &[OPERATION_LABEL, BACKEND_LABEL]); } } + + #[test] + fn operation_metric_names_and_low_cardinality_values_are_stable() { + assert_eq!(INTERNODE_OPERATION_READ_FILE_STREAM, "read_file_stream"); + assert_eq!(INTERNODE_OPERATION_PUT_FILE_STREAM, "put_file_stream"); + assert_eq!(INTERNODE_OPERATION_WALK_DIR, "walk_dir"); + assert_eq!(INTERNODE_OPERATION_GRPC_READ_ALL, "grpc_read_all"); + assert_eq!(INTERNODE_OPERATION_GRPC_WRITE_ALL, "grpc_write_all"); + + assert_eq!(INTERNODE_TRANSPORT_BACKEND_TCP_HTTP, "tcp-http"); + assert_eq!(INTERNODE_TRANSPORT_BACKEND_GRPC, "grpc"); + assert_eq!(INTERNODE_TRANSPORT_BACKEND_UNKNOWN, "unknown"); + + assert_eq!( + INTERNODE_OPERATION_METRICS, + &[ + InternodeOperationMetricDescriptor { + name: "rustfs_system_network_internode_operation_sent_bytes_total", + labels: &[OPERATION_LABEL, BACKEND_LABEL], + }, + InternodeOperationMetricDescriptor { + name: "rustfs_system_network_internode_operation_recv_bytes_total", + labels: &[OPERATION_LABEL, BACKEND_LABEL], + }, + InternodeOperationMetricDescriptor { + name: "rustfs_system_network_internode_operation_requests_outgoing_total", + labels: &[OPERATION_LABEL, BACKEND_LABEL], + }, + InternodeOperationMetricDescriptor { + name: "rustfs_system_network_internode_operation_requests_incoming_total", + labels: &[OPERATION_LABEL, BACKEND_LABEL], + }, + InternodeOperationMetricDescriptor { + name: "rustfs_system_network_internode_operation_errors_total", + labels: &[OPERATION_LABEL, BACKEND_LABEL], + }, + ] + ); + } }