diff --git a/.deepsource.toml b/.deepsource.toml index b77a5238..e175aaa7 100644 --- a/.deepsource.toml +++ b/.deepsource.toml @@ -1,9 +1,10 @@ version = 1 -# Trees that are generated or vendored rather than hand-written source. -# Built module artifacts under any `pkg/`, the int-gen outputs under `generated/`, and the scenario `verification/` -# fixtures. -exclude_patterns = ["**/pkg/**", "generated/**", "verification/**"] +# `pkg/` is deliberately NOT excluded from DeepSource analysis. +# It holds hand-written module loaders (e.g. the Pyodide shim et_ws_pydata1.js) alongside build output. Only the +# int-gen outputs under `generated/` and the scenario `verification/` fixtures are excluded, since those are fully +# generated; if genuinely-generated glue under `pkg/` turns up as a finding, add a narrow per-file exclude then. +exclude_patterns = ["generated/**", "verification/**"] # Repo convention: tests live in a `tests/` directory or in source files prefixed `test_`. test_patterns = ["**/test_*.py", "**/tests/**"] diff --git a/.mise/config.coverage.toml b/.mise/config.coverage.toml index f2be82d7..0f7c3485 100644 --- a/.mise/config.coverage.toml +++ b/.mise/config.coverage.toml @@ -102,7 +102,17 @@ for profraw in "$covdir"/*.profraw; do "$bin/llvm-profdata" merge -sparse -o "$pd" "$profraw" "$bin/llvm-cov" export --format=lcov --instr-profile "$pd" "$obj" >> "$covdir/wasi.lcov" done -coreutils cat "$covdir/wasi.lcov" >> lcov.info +# The wasm covmap records reference every source each module linked. +# Dependency crates under ~/.cargo/registry and toolchain std under ~/.rustup are not in VCS -- DeepSource +# flags them and they skew the aggregate metric -- so keep only workspace records (dropping .cargo/.rustup/rustc +# blocks) before merging the wasm lcov into lcov.info. +keep="$covdir/keep.awk" +coreutils cat > "$keep" <<'AWK' +{ buf = buf $0 ORS } +/^SF:/ { p = substr($0, 4); drop = (index(p, "/.cargo/") || index(p, "/.rustup/") || index(p, "/rustc/")) } +/^end_of_record$/ { if (!drop) printf "%s", buf; buf = ""; drop = 0 } +AWK +goawk -f "$keep" "$covdir/wasi.lcov" >> lcov.info """ shell = "bash -euo pipefail -c" diff --git a/.mise/config.toml b/.mise/config.toml index 8981aa3c..effec65b 100644 --- a/.mise/config.toml +++ b/.mise/config.toml @@ -849,7 +849,10 @@ run = "typos --config config/typos.toml --write-changes" # URLs from .rs comments and string literals too. [tasks.link-check] description = "Check that URLs in .md and .rs files are reachable (network)" -run = "lychee --config config/lychee.toml '**/*.md' '**/*.rs'" +# The .rs glob is scoped to the source dirs, not a bare `**/*.rs`. +# A recursive `**/*.rs` walks target/'s churning rustc temp files, and lychee expands globs before applying +# `exclude_path`, so it aborts with a GlobError when one of those temps vanishes mid-iteration. +run = "lychee --config config/lychee.toml '**/*.md' 'libs/**/*.rs' 'services/**/*.rs' 'utilities/**/*.rs'" [tasks.ryl-check] description = "Lint YAML with ryl (a yamllint-compatible Rust linter)" diff --git a/Cargo.lock b/Cargo.lock index 4b5aa7cc..69eb3324 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -4664,10 +4664,13 @@ dependencies = [ "actix-web", "edge-toolkit", "et-modules-service", + "et-otlp", "et-storage-service", "et-test-helpers", "et-ws-service", "futures-util", + "int-otlp-mock", + "retry", "serde_json", "tempfile", "tokio", @@ -6185,7 +6188,9 @@ dependencies = [ "et-test-helpers", "opentelemetry-proto 0.31.0", "prost", + "reqwest 0.13.4", "serde_json", + "tokio", ] [[package]] diff --git a/Cargo.toml b/Cargo.toml index 4b2347a7..4d8dcb81 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -105,11 +105,13 @@ opentelemetry-otlp = { version = "0.31", default-features = false, features = [ "http-json", "http-proto", "logs", + "metrics", "reqwest-blocking-client", "trace", ] } opentelemetry-proto = { version = "0.31", default-features = false, features = [ "gen-tonic-messages", + "metrics", "trace", "with-serde", ] } diff --git a/config/ast-grep/rules/no-mod-in-tests.yaml b/config/ast-grep/rules/no-mod-in-tests.yaml new file mode 100644 index 00000000..60d1ea45 --- /dev/null +++ b/config/ast-grep/rules/no-mod-in-tests.yaml @@ -0,0 +1,14 @@ +id: no-mod-in-tests +language: Rust +severity: error +message: | + `mod` is forbidden inside test files (`tests/**/*.rs` and `src/test_*.rs`). Keep shared test helpers in a + test-support library crate -- `et-test-helpers` for low-dependency helpers, or a dedicated crate such as + `et-ws-test-server` / `int-otlp-mock` -- and `use` them, rather than a `tests/common/mod.rs` include or an + inline `mod { ... }`. A library gives each helper one compiled home (so coverage and lints see it once) instead + of recompiling it per test binary, and it keeps integration-test files flat: one file per test binary. +rule: + kind: mod_item +files: + - "**/tests/**/*.rs" + - "**/test_*.rs" diff --git a/config/clippy.toml b/config/clippy.toml index 3031777f..645a9102 100644 --- a/config/clippy.toml +++ b/config/clippy.toml @@ -1,3 +1,23 @@ +allow-dbg-in-tests = true +allow-indexing-slicing-in-tests = true +allow-large-stack-frames-in-tests = true +allow-panic-in-tests = true +allow-print-in-tests = true +allow-unwrap-in-tests = true +allow-useless-vec-in-tests = true + +# Compile-time bans -- a second layer under the matching config/ast-grep/rules/*, which catch these at diff time. +# `.expect()` itself is NOT listed here. `disallowed_methods` has no test exemption and does not skip +# proc-macro-generated code, so it would flag the `.build().expect("Failed building the Runtime")` that the +# `#[tokio::test]` macro expands into -- breaking every async test, even though that `.expect()` isn't ours. The +# `clippy::expect_used` restriction lint (denied workspace-wide) is the right tool instead: it bans hand-written +# `.expect()` everywhere but skips macro-generated calls, so `#[tokio::test]` is unaffected. `disallowed-methods` +# covers `.expect_err()` (which `expect_used` misses -- use `.unwrap_err()`) and mirrors ast-grep's `no-current-dir`. +disallowed-methods = [ + { path = "std::env::current_dir", reason = "use get_project_root() / et_path::find_project_root()" }, + { path = "std::result::Result::expect_err", reason = "use .unwrap_err() -- no message string needed" }, +] + allowed-idents-below-min-chars = [ # clippy defaults "Eq", diff --git a/libs/edge-toolkit/src/config.rs b/libs/edge-toolkit/src/config.rs index cffb5624..1d4bcb3d 100644 --- a/libs/edge-toolkit/src/config.rs +++ b/libs/edge-toolkit/src/config.rs @@ -81,7 +81,12 @@ where /// Helper to find repository root. /// /// This is the one sanctioned `current_dir()`. -#[expect(clippy::missing_panics_doc, clippy::unwrap_used)] +#[expect( + clippy::disallowed_methods, + clippy::missing_panics_doc, + clippy::unwrap_used, + reason = "the one sanctioned current_dir() -- this helper is what the disallowed-methods ban points callers to" +)] #[must_use] pub fn get_project_root() -> PathBuf { et_path::find_project_root(&std::env::current_dir().unwrap()) diff --git a/libs/edge-toolkit/src/ws.rs b/libs/edge-toolkit/src/ws.rs index e1631a8e..6cebc992 100644 --- a/libs/edge-toolkit/src/ws.rs +++ b/libs/edge-toolkit/src/ws.rs @@ -6,15 +6,15 @@ use serde::{Deserialize, Serialize}; /// payload is described as "arbitrary JSON" without tripping the parser. #[cfg(feature = "schema-export")] #[expect( - clippy::expect_used, - reason = "static JSON literal -> Schema is infallible; surfacing it loudly if asyncapi-rust ever changes shape" + clippy::unwrap_used, + reason = "static JSON literal -> Schema conversion is infallible by construction" )] fn any_json_schema(_: &mut schemars::SchemaGenerator) -> schemars::Schema { serde_json::json!({ "description": "Arbitrary JSON value (opaque to the protocol)", }) .try_into() - .expect("any_json_schema is a valid object schema") + .unwrap() } /// Schema for `Vec` byte-array fields. schemars 1.x's default `Vec` @@ -24,8 +24,8 @@ fn any_json_schema(_: &mut schemars::SchemaGenerator) -> schemars::Schema { /// `list` representation. #[cfg(feature = "schema-export")] #[expect( - clippy::expect_used, - reason = "static JSON literal -> Schema is infallible; surfacing it loudly if asyncapi-rust ever changes shape" + clippy::unwrap_used, + reason = "static JSON literal -> Schema conversion is infallible by construction" )] fn byte_array_schema(_: &mut schemars::SchemaGenerator) -> schemars::Schema { serde_json::json!({ @@ -34,7 +34,7 @@ fn byte_array_schema(_: &mut schemars::SchemaGenerator) -> schemars::Schema { "description": "Byte array (uint8)", }) .try_into() - .expect("byte_array_schema is a valid array schema") + .unwrap() } #[expect( diff --git a/libs/edge-toolkit/src/ws_server.rs b/libs/edge-toolkit/src/ws_server.rs index b215a018..0789760e 100644 --- a/libs/edge-toolkit/src/ws_server.rs +++ b/libs/edge-toolkit/src/ws_server.rs @@ -207,14 +207,11 @@ impl AgentRegistry { summaries } - /// # Panics - /// Panics if `to_agent_id` is not present in the registry -- the caller is - /// expected to have validated that the recipient exists before queueing. + /// Queue a direct message for `to_agent_id`, returning the stored message and the recipient's session. + /// + /// Returns `None` when `to_agent_id` is not in the registry. The inner `Option` is the recipient's live + /// session -- `Some` when connected, `None` when the message was queued for a disconnected agent. #[must_use] - #[expect( - clippy::expect_used, - reason = "caller contract: to_agent_id must reference a known agent" - )] pub fn queue_direct( &self, message_id: String, @@ -222,11 +219,9 @@ impl AgentRegistry { to_agent_id: &str, server_received_at: String, message: serde_json::Value, - ) -> (PendingDirectMessage, Option) { + ) -> Option<(PendingDirectMessage, Option)> { let mut agents = lock_agents(&self.agents); - let recipient = agents - .get_mut(to_agent_id) - .expect("queue_direct called for unknown target agent"); + let recipient = agents.get_mut(to_agent_id)?; let pending = PendingDirectMessage { message_id, @@ -240,7 +235,7 @@ impl AgentRegistry { .insert(from_agent_id.to_string(), pending.clone()); drop(agents); - (pending, session) + Some((pending, session)) } #[must_use] diff --git a/libs/edge-toolkit/tests/config.rs b/libs/edge-toolkit/tests/config.rs index bbc40a31..d7515244 100644 --- a/libs/edge-toolkit/tests/config.rs +++ b/libs/edge-toolkit/tests/config.rs @@ -5,7 +5,6 @@ //! visible: `deserialize_optional::` and the `Duration` humantime //! variant share one sentinel (`none` / `off` / `disabled`). #![cfg(test)] -#![expect(clippy::expect_used, reason = "test code: expect panics surface the failure")] use std::time::Duration; @@ -15,12 +14,12 @@ use serde::de::value::{Error as ValueError, StrDeserializer}; fn optional_string(value: &str) -> Option { let deser: StrDeserializer<'_, ValueError> = value.into_deserializer(); - deserialize_optional::<_, String>(deser).expect("deserialize Option") + deserialize_optional::<_, String>(deser).unwrap() } fn optional_duration(value: &str) -> Option { let deser: StrDeserializer<'_, ValueError> = value.into_deserializer(); - deserialize_optional_humantime(deser).expect("deserialize Option") + deserialize_optional_humantime(deser).unwrap() } #[test] diff --git a/libs/edge-toolkit/tests/http_pyodide.rs b/libs/edge-toolkit/tests/http_pyodide.rs index ab3f9ba8..fc70b55f 100644 --- a/libs/edge-toolkit/tests/http_pyodide.rs +++ b/libs/edge-toolkit/tests/http_pyodide.rs @@ -11,11 +11,6 @@ //! silently passing. #![cfg(test)] -#![expect( - clippy::panic, - clippy::unwrap_used, - reason = "test code: missing install fails loudly with a hint" -)] use std::collections::HashSet; use std::path::PathBuf; diff --git a/libs/edge-toolkit/tests/no_mise.rs b/libs/edge-toolkit/tests/no_mise.rs index d65d31e0..fd1e9b31 100644 --- a/libs/edge-toolkit/tests/no_mise.rs +++ b/libs/edge-toolkit/tests/no_mise.rs @@ -5,7 +5,6 @@ //! warnings at startup. #![cfg(test)] -#![expect(clippy::unwrap_used, reason = "test code: failed tempdir setup should fail the test")] use std::path::PathBuf; diff --git a/libs/edge-toolkit/tests/npm_mod.rs b/libs/edge-toolkit/tests/npm_mod.rs index 4355299a..442eb33f 100644 --- a/libs/edge-toolkit/tests/npm_mod.rs +++ b/libs/edge-toolkit/tests/npm_mod.rs @@ -4,7 +4,6 @@ //! verifies the resolver picks the right `node_modules` directory. #![cfg(test)] -#![expect(clippy::unwrap_used, reason = "test code: failed tempdir setup should fail the test")] use edge_toolkit::config::find_npm_modules_path_in; use fs_err as fs; diff --git a/libs/edge-toolkit/tests/pipx_site_packages.rs b/libs/edge-toolkit/tests/pipx_site_packages.rs index 9436f460..f365291d 100644 --- a/libs/edge-toolkit/tests/pipx_site_packages.rs +++ b/libs/edge-toolkit/tests/pipx_site_packages.rs @@ -5,7 +5,6 @@ //! (`//Lib/site-packages`, no Python-version subdir). #![cfg(test)] -#![expect(clippy::unwrap_used, reason = "test code: failed tempdir setup should fail the test")] use edge_toolkit::config::find_site_packages_in; use fs_err as fs; diff --git a/libs/edge-toolkit/tests/ws.rs b/libs/edge-toolkit/tests/ws.rs index 85cc80fb..f4492c66 100644 --- a/libs/edge-toolkit/tests/ws.rs +++ b/libs/edge-toolkit/tests/ws.rs @@ -10,10 +10,8 @@ #![cfg(test)] #![expect( - clippy::expect_used, - clippy::panic, clippy::wildcard_enum_match_arm, - reason = "test code: assertion panics carry enough context for tests" + reason = "test code: wildcard enum match arms are intentional" )] use edge_toolkit::ws::{ClientMessage, ServerMessage}; @@ -30,93 +28,93 @@ fn client_expect_relay_text(msg: ClientMessage) -> String { #[test] fn client_relays_empty_string() { - let msg = ClientMessage::from_text_frame("").expect("relay must not error"); + let msg = ClientMessage::from_text_frame("").unwrap(); assert_eq!(client_expect_relay_text(msg), ""); } #[test] fn client_relays_plain_text() { - let msg = ClientMessage::from_text_frame("hello world").expect("relay must not error"); + let msg = ClientMessage::from_text_frame("hello world").unwrap(); assert_eq!(client_expect_relay_text(msg), "hello world"); } #[test] fn client_relays_malformed_json() { - let msg = ClientMessage::from_text_frame("{not json").expect("relay must not error"); + let msg = ClientMessage::from_text_frame("{not json").unwrap(); assert_eq!(client_expect_relay_text(msg), "{not json"); } #[test] fn client_relays_json_number() { - let msg = ClientMessage::from_text_frame("42").expect("relay must not error"); + let msg = ClientMessage::from_text_frame("42").unwrap(); assert_eq!(client_expect_relay_text(msg), "42"); } #[test] fn client_relays_json_string_literal() { let raw = "\"hello\""; - let msg = ClientMessage::from_text_frame(raw).expect("relay must not error"); + let msg = ClientMessage::from_text_frame(raw).unwrap(); assert_eq!(client_expect_relay_text(msg), raw); } #[test] fn client_relays_json_array() { - let msg = ClientMessage::from_text_frame("[1, 2, 3]").expect("relay must not error"); + let msg = ClientMessage::from_text_frame("[1, 2, 3]").unwrap(); assert_eq!(client_expect_relay_text(msg), "[1, 2, 3]"); } #[test] fn client_relays_json_null() { - let msg = ClientMessage::from_text_frame("null").expect("relay must not error"); + let msg = ClientMessage::from_text_frame("null").unwrap(); assert_eq!(client_expect_relay_text(msg), "null"); } #[test] fn client_relays_json_object_without_type() { let raw = r#"{"hello":"world"}"#; - let msg = ClientMessage::from_text_frame(raw).expect("relay must not error"); + let msg = ClientMessage::from_text_frame(raw).unwrap(); assert_eq!(client_expect_relay_text(msg), raw); } #[test] fn client_relays_json_object_with_non_string_type() { let raw = r#"{"type":42,"payload":true}"#; - let msg = ClientMessage::from_text_frame(raw).expect("relay must not error"); + let msg = ClientMessage::from_text_frame(raw).unwrap(); assert_eq!(client_expect_relay_text(msg), raw); } #[test] fn client_relays_json_object_with_non_et_type() { let raw = r#"{"type":"foo-bar","x":1}"#; - let msg = ClientMessage::from_text_frame(raw).expect("relay must not error"); + let msg = ClientMessage::from_text_frame(raw).unwrap(); assert_eq!(client_expect_relay_text(msg), raw); } #[test] fn client_relays_json_object_with_type_et_no_dash() { let raw = r#"{"type":"etwhatever"}"#; - let msg = ClientMessage::from_text_frame(raw).expect("relay must not error"); + let msg = ClientMessage::from_text_frame(raw).unwrap(); assert_eq!(client_expect_relay_text(msg), raw); } #[test] fn client_relays_json_object_with_capitalised_et_prefix() { let raw = r#"{"type":"Et-connect"}"#; - let msg = ClientMessage::from_text_frame(raw).expect("relay must not error"); + let msg = ClientMessage::from_text_frame(raw).unwrap(); assert_eq!(client_expect_relay_text(msg), raw); } #[test] fn client_relays_third_party_vendor_prefix() { let raw = r#"{"type":"vendor-x-event","seq":7}"#; - let msg = ClientMessage::from_text_frame(raw).expect("relay must not error"); + let msg = ClientMessage::from_text_frame(raw).unwrap(); assert_eq!(client_expect_relay_text(msg), raw); } #[test] fn client_typed_for_valid_et_message() { // `et-list-agents` has no payload fields; the bare envelope parses. - let msg = ClientMessage::from_text_frame(r#"{"type":"et-list-agents"}"#).expect("typed parse must succeed"); + let msg = ClientMessage::from_text_frame(r#"{"type":"et-list-agents"}"#).unwrap(); assert!( matches!(msg, ClientMessage::ListAgents), "expected ClientMessage::ListAgents, got {msg:?}" @@ -128,14 +126,13 @@ fn client_typed_for_server_only_variant_is_decode_error() { // `et-connect-ack` lives in ServerMessage, not ClientMessage. A client // claiming to send a ConnectAck must surface as a decode error -- that's // the type-level enforcement the split exists to provide. - let _err = ClientMessage::from_text_frame(r#"{"type":"et-connect-ack","agent_id":"a","status":"assigned"}"#) - .expect_err("server-side variant in client decoder must surface a decode error"); + let _err = + ClientMessage::from_text_frame(r#"{"type":"et-connect-ack","agent_id":"a","status":"assigned"}"#).unwrap_err(); } #[test] fn client_decode_error_for_unknown_variant() { - let _err = ClientMessage::from_text_frame(r#"{"type":"et-bogus-variant"}"#) - .expect_err("et-prefixed unknown variants must surface a decode error"); + let _err = ClientMessage::from_text_frame(r#"{"type":"et-bogus-variant"}"#).unwrap_err(); } #[test] @@ -158,21 +155,20 @@ fn server_expect_relay_text(msg: ServerMessage) -> String { #[test] fn server_relays_plain_text() { - let msg = ServerMessage::from_text_frame("hello").expect("relay must not error"); + let msg = ServerMessage::from_text_frame("hello").unwrap(); assert_eq!(server_expect_relay_text(msg), "hello"); } #[test] fn server_relays_json_object_with_non_et_type() { let raw = r#"{"type":"vendor-y-broadcast","seq":1}"#; - let msg = ServerMessage::from_text_frame(raw).expect("relay must not error"); + let msg = ServerMessage::from_text_frame(raw).unwrap(); assert_eq!(server_expect_relay_text(msg), raw); } #[test] fn server_typed_for_response_variant() { - let msg = - ServerMessage::from_text_frame(r#"{"type":"et-response","message":"hi"}"#).expect("typed parse must succeed"); + let msg = ServerMessage::from_text_frame(r#"{"type":"et-response","message":"hi"}"#).unwrap(); match msg { ServerMessage::Response { message } => assert_eq!(message, "hi"), other => panic!("expected ServerMessage::Response, got {other:?}"), @@ -183,8 +179,7 @@ fn server_typed_for_response_variant() { fn server_typed_for_client_only_variant_is_decode_error() { // `et-connect` lives in ClientMessage. A server claiming to send Connect // to a client must surface as a decode error. - let _err = ServerMessage::from_text_frame(r#"{"type":"et-connect"}"#) - .expect_err("client-side variant in server decoder must surface a decode error"); + let _err = ServerMessage::from_text_frame(r#"{"type":"et-connect"}"#).unwrap_err(); } #[test] diff --git a/libs/et-otlp/src/lib.rs b/libs/et-otlp/src/lib.rs index c2131727..ea4b3952 100644 --- a/libs/et-otlp/src/lib.rs +++ b/libs/et-otlp/src/lib.rs @@ -13,16 +13,12 @@ //! so batched spans/logs are flushed -- otherwise short-lived processes //! (e.g. the wasi-runner, which exits as soon as a module finishes) drop //! their tail-end spans. -#![expect( - clippy::expect_used, - reason = "init runs once at startup; exporter build / RUST_LOG / subscriber failures should crash early" -)] - use edge_toolkit::config::{OtlpConfig, OtlpProtocol}; use opentelemetry::{KeyValue, trace::TracerProvider as _}; use opentelemetry_appender_tracing::layer::OpenTelemetryTracingBridge; -use opentelemetry_otlp::{LogExporter, WithExportConfig as _, WithHttpConfig as _}; +use opentelemetry_otlp::{LogExporter, MetricExporter, WithExportConfig as _, WithHttpConfig as _}; use opentelemetry_sdk::logs::SdkLoggerProvider; +use opentelemetry_sdk::metrics::SdkMeterProvider; use opentelemetry_sdk::trace::SdkTracerProvider; use opentelemetry_sdk::{Resource, propagation::TraceContextPropagator}; use tracing::subscriber::set_global_default; @@ -31,7 +27,7 @@ use tracing_subscriber::{EnvFilter, Registry, layer::SubscriberExt as _}; pub const RUST_LOG: &str = "RUST_LOG"; -/// Handles for the spans + logs pipelines. +/// Handles for the spans + logs + metrics pipelines. /// /// Drop alone won't flush -- call [`OtelHandles::shutdown`] at the end of /// `main()` (or in a Drop guard). @@ -39,23 +35,30 @@ pub const RUST_LOG: &str = "RUST_LOG"; pub struct OtelHandles { pub tracer_provider: SdkTracerProvider, pub logger_provider: SdkLoggerProvider, + pub meter_provider: SdkMeterProvider, } impl OtelHandles { - /// Flush any buffered spans/logs and tear down the exporters. + /// Flush any buffered spans/logs/metrics and tear down the exporters. pub fn shutdown(self) { // Errors here are non-fatal -- the process is exiting anyway. drop(self.tracer_provider.shutdown()); drop(self.logger_provider.shutdown()); + drop(self.meter_provider.shutdown()); } } /// Initialise the global tracing subscriber + `OTel` pipeline against `config`. /// -/// Call exactly once per process; subsequent calls panic via -/// `set_global_default`. -#[must_use] -pub fn init(config: &OtlpConfig) -> OtelHandles { +/// Call exactly once per process; a second call returns an error from +/// `set_global_default`. Exporter-build and `RUST_LOG`-parse failures are +/// returned too, so `main` can surface them and exit non-zero. +/// +/// # Errors +/// +/// Returns an error if any OTLP exporter fails to build, `RUST_LOG` is +/// invalid, or the global subscriber is already set. +pub fn init(config: &OtlpConfig) -> Result> { // tracing_log forwards `log` crate records (used by transitive deps) // through the tracing subscriber. drop(tracing_log::LogTracer::init()); @@ -79,8 +82,7 @@ pub fn init(config: &OtlpConfig) -> OtelHandles { .with_protocol(protocol) .with_endpoint(trace_endpoint) .with_headers(headers.clone()) - .build() - .expect("build OTLP span exporter"); + .build()?; let mut service_descriptors = vec![KeyValue::new("service.version", env!("CARGO_PKG_VERSION").to_string())]; if let Some(hostname) = hostname::get().ok().and_then(|host| host.into_string().ok()) { @@ -95,19 +97,36 @@ pub fn init(config: &OtlpConfig) -> OtelHandles { .with_batch_exporter(span_exporter) .with_resource(resource.clone()) .build(); + // Set the global tracer provider so direct `global::tracer(...)` spans (e.g. the ws hub's `ws.connect`) + // export too -- not just the `tracing`-subscriber spans routed through the layer below. + opentelemetry::global::set_tracer_provider(tracer_provider.clone()); let otel_tracing_layer = OpenTelemetryLayer::new(tracer_provider.tracer(config.service_label.clone())); + // Metrics ride the same OTLP/HTTP transport as spans and logs, posting to `/metrics`. + // The periodic reader batches on its own interval; `OtelHandles::shutdown` forces a final flush on exit. + let metric_endpoint = format!("{}/metrics", config.collector_url); + let metric_exporter = MetricExporter::builder() + .with_http() + .with_protocol(protocol) + .with_endpoint(metric_endpoint) + .with_headers(headers.clone()) + .build()?; + let meter_provider = SdkMeterProvider::builder() + .with_periodic_exporter(metric_exporter) + .with_resource(resource.clone()) + .build(); + opentelemetry::global::set_meter_provider(meter_provider.clone()); + let log_directives = std::env::var(RUST_LOG).unwrap_or_else(|_| "info".to_string()); - let env_filter = EnvFilter::try_new(log_directives).expect("valid RUST_LOG"); + let env_filter = EnvFilter::try_new(log_directives)?; let log_exporter = LogExporter::builder() .with_http() .with_protocol(protocol) .with_endpoint(log_endpoint) .with_headers(headers) - .build() - .expect("build OTLP log exporter"); + .build()?; let logger_provider = SdkLoggerProvider::builder() .with_batch_exporter(log_exporter) @@ -123,10 +142,11 @@ pub fn init(config: &OtlpConfig) -> OtelHandles { .with(otel_tracing_layer) .with(otel_log_layer); - set_global_default(subscriber).expect("set tracing subscriber"); + set_global_default(subscriber)?; - OtelHandles { + Ok(OtelHandles { tracer_provider, logger_provider, - } + meter_provider, + }) } diff --git a/libs/otlp-mock/Cargo.toml b/libs/otlp-mock/Cargo.toml index f803daf9..0168b576 100644 --- a/libs/otlp-mock/Cargo.toml +++ b/libs/otlp-mock/Cargo.toml @@ -18,5 +18,9 @@ opentelemetry-proto.workspace = true prost.workspace = true serde_json.workspace = true +[dev-dependencies] +reqwest.workspace = true +tokio = { workspace = true, features = ["macros", "rt-multi-thread"] } + [lints] workspace = true diff --git a/libs/otlp-mock/src/lib.rs b/libs/otlp-mock/src/lib.rs index 9c60373e..ad4829b0 100644 --- a/libs/otlp-mock/src/lib.rs +++ b/libs/otlp-mock/src/lib.rs @@ -13,9 +13,10 @@ //! such as Vector's opentelemetry sink emits) or JSON (what `et-otlp` sends //! with `OTLP_PROTOCOL=JSON`). Both decode to the same `ExportTraceServiceRequest`. //! - `POST /logs` -- OTLP/HTTP-JSON log payloads. +//! - `POST /metrics` -- OTLP/HTTP metric payloads (protobuf or JSON, like `/traces`). //! -//! Read captured spans back via [`OtlpMock::flatten_spans`] and logs via -//! [`OtlpMock::logs`]. +//! Read captured spans back via [`OtlpMock::flatten_spans`], logs via +//! [`OtlpMock::logs`], and metrics via [`OtlpMock::flatten_metrics`]. #![expect( clippy::unwrap_used, clippy::panic, @@ -27,8 +28,10 @@ use std::sync::{Arc, Mutex}; use actix_web::http::header::ContentType; use actix_web::{App, HttpResponse, HttpServer, post, web}; +use opentelemetry_proto::tonic::collector::metrics::v1::ExportMetricsServiceRequest; use opentelemetry_proto::tonic::collector::trace::v1::ExportTraceServiceRequest; use opentelemetry_proto::tonic::common::v1::any_value; +use opentelemetry_proto::tonic::metrics::v1::{metric::Data, number_data_point}; use prost::Message as _; use serde_json::Value; @@ -36,6 +39,7 @@ use serde_json::Value; struct Captured { traces: Mutex>, logs: Mutex>, + metrics: Mutex>, } /// Handle to a running mock collector. @@ -104,6 +108,63 @@ impl OtlpMock { } out } + + /// Walk every metric across every captured request, pairing each with its `Resource`'s `service.name`. + /// `value` sums the numeric (Sum/Gauge) data points -- so a monotonic counter reads as its running total -- + /// while `data_points` counts them (histogram points are counted but don't contribute to `value`). + #[must_use] + pub fn flatten_metrics(&self) -> Vec { + let mut out = Vec::new(); + for req in self.captured.metrics.lock().unwrap().iter() { + for resource_metric in &req.resource_metrics { + let service_name = resource_metric + .resource + .as_ref() + .and_then(|resource| { + resource + .attributes + .iter() + .filter(|attr| attr.key == "service.name") + .find_map(|attr| { + let any_value::Value::StringValue(value) = attr.value.as_ref()?.value.as_ref()? else { + return None; + }; + Some(value.clone()) + }) + }) + .unwrap_or_default(); + for scope_metric in &resource_metric.scope_metrics { + for metric in &scope_metric.metrics { + let (value, data_points) = match &metric.data { + Some(Data::Sum(sum)) => sum_number_points(&sum.data_points), + Some(Data::Gauge(gauge)) => sum_number_points(&gauge.data_points), + Some(Data::Histogram(histogram)) => (0, histogram.data_points.len()), + _ => (0, 0), + }; + out.push(FlatMetric { + service_name: service_name.clone(), + name: metric.name.clone(), + unit: metric.unit.clone(), + value, + data_points, + }); + } + } + } + } + out + } +} + +/// Sum the integer OTLP data points into `(total, count)`; float data points don't contribute to `total`. +fn sum_number_points(points: &[opentelemetry_proto::tonic::metrics::v1::NumberDataPoint]) -> (i64, usize) { + let mut total: i64 = 0; + for point in points { + if let Some(number_data_point::Value::AsInt(value)) = point.value { + total = total.saturating_add(value); + } + } + (total, points.len()) } /// Lowercase-hex-encode bytes -- used for the trace/span ids in [`FlatSpan`]. @@ -130,6 +191,20 @@ pub struct FlatSpan { pub name: String, } +/// Flattened metric view for assertions. +#[derive(Clone, Debug)] +#[non_exhaustive] +pub struct FlatMetric { + pub service_name: String, + pub name: String, + pub unit: String, + /// Sum of the metric's integer (Sum/Gauge) data points -- a monotonic `u64`/`i64` counter's running total. + /// Floating-point data points are ignored (the metrics emitted here are integer counters). + pub value: i64, + /// Number of data points seen for this metric (includes histogram points). + pub data_points: usize, +} + #[expect( clippy::single_call_fn, reason = "actix-web route handler; registered via the #[post] macro" @@ -175,6 +250,36 @@ async fn handle_logs(state: web::Data>, body: web::Json) -> HttpResponse::Ok().content_type("application/json").body("{}") } +#[expect( + clippy::single_call_fn, + reason = "actix-web route handler; registered via the #[post] macro" +)] +#[post("/metrics")] +async fn handle_metrics( + state: web::Data>, + content_type: web::Header, + body: web::Bytes, +) -> HttpResponse { + // Same dual-encoding decode as `/traces`: protobuf from a real relay, JSON from `et-otlp`'s JSON protocol. + let is_protobuf = content_type.0.subtype().as_str().contains("protobuf"); + let decoded = if is_protobuf { + ExportMetricsServiceRequest::decode(body.as_ref()).ok() + } else { + serde_json::from_slice::(&body).ok() + }; + let Some(metrics_request) = decoded else { + return HttpResponse::BadRequest().finish(); + }; + state.metrics.lock().unwrap().push(metrics_request); + if is_protobuf { + HttpResponse::Ok() + .content_type("application/x-protobuf") + .body(Vec::new()) + } else { + HttpResponse::Ok().content_type("application/json").body("{}") + } +} + /// Start the mock on a free port and return its handle. /// /// The HTTP server runs on its own thread + actix runtime; the test's @@ -206,6 +311,7 @@ pub fn start_on(port: u16) -> OtlpMock { .app_data(web::PayloadConfig::new(64 * 1024 * 1024)) .service(handle_traces) .service(handle_logs) + .service(handle_metrics) }) .bind(&addr) .unwrap() diff --git a/libs/otlp-mock/tests/metrics.rs b/libs/otlp-mock/tests/metrics.rs new file mode 100644 index 00000000..bb26c8b1 --- /dev/null +++ b/libs/otlp-mock/tests/metrics.rs @@ -0,0 +1,153 @@ +//! Exercises the mock's `/metrics` endpoint and `flatten_metrics` across every OTLP metric shape: +//! `Sum`, `Gauge`, `Histogram`, and a data-less metric, decoded from both JSON and protobuf bodies, plus a +//! non-string `service.name` and a malformed body. This is the direct-injection counterpart to the end-to-end +//! `et-ws-test-server` `OTel` test, which only drives the integer-counter path the hub actually emits. +#![cfg(test)] + +use opentelemetry_proto::tonic::collector::metrics::v1::ExportMetricsServiceRequest; +use opentelemetry_proto::tonic::common::v1::{AnyValue, KeyValue, any_value}; +use opentelemetry_proto::tonic::metrics::v1::{ + Gauge, Histogram, HistogramDataPoint, Metric, NumberDataPoint, ResourceMetrics, ScopeMetrics, Sum, metric::Data, + number_data_point, +}; +use opentelemetry_proto::tonic::resource::v1::Resource; +use prost::Message as _; + +fn int_point(value: i64) -> NumberDataPoint { + NumberDataPoint { + value: Some(number_data_point::Value::AsInt(value)), + ..Default::default() + } +} + +fn metric(name: &str, data: Option) -> Metric { + Metric { + name: name.to_owned(), + unit: "1".to_owned(), + data, + ..Default::default() + } +} + +/// One resource carrying a non-string `service.name` (so the `StringValue` guard falls through to the empty +/// default) and one metric of every `Data` variant the flattener branches on, including a data-less one. +#[expect( + clippy::single_call_fn, + reason = "distinct fixture builder for the metric-shape matrix; kept separate" +)] +fn sample_request() -> ExportMetricsServiceRequest { + let resource = Resource { + attributes: vec![KeyValue { + key: "service.name".to_owned(), + // A non-string value: the flattener's `StringValue` binding must fail and yield the empty default. + value: Some(AnyValue { + value: Some(any_value::Value::IntValue(7)), + }), + }], + ..Default::default() + }; + let metrics = vec![ + metric( + "sum.metric", + Some(Data::Sum(Sum { + data_points: vec![int_point(3), int_point(4)], + ..Default::default() + })), + ), + metric( + "gauge.metric", + Some(Data::Gauge(Gauge { + data_points: vec![int_point(9)], + })), + ), + metric( + "hist.metric", + Some(Data::Histogram(Histogram { + data_points: vec![HistogramDataPoint::default(), HistogramDataPoint::default()], + ..Default::default() + })), + ), + metric("none.metric", None), + ]; + ExportMetricsServiceRequest { + resource_metrics: vec![ResourceMetrics { + resource: Some(resource), + scope_metrics: vec![ScopeMetrics { + metrics, + ..Default::default() + }], + ..Default::default() + }], + } +} + +#[tokio::test] +async fn metrics_endpoint_decodes_json_protobuf_and_flattens_every_shape() { + let mock = int_otlp_mock::start(); + let url = format!("{}/metrics", mock.collector_url()); + let client = reqwest::Client::new(); + let request = sample_request(); + + // JSON body -- the encoding `et-otlp`'s JSON protocol uses; drives handle_metrics' serde_json branch. + let json_resp = client + .post(&url) + .header("content-type", "application/json") + .body(serde_json::to_vec(&request).unwrap()) + .send() + .await + .unwrap(); + assert!(json_resp.status().is_success(), "JSON /metrics POST should succeed"); + + // Protobuf body -- what a real OTLP relay sends; drives handle_metrics' prost branch. + let mut proto_body = Vec::new(); + request.encode(&mut proto_body).unwrap(); + let proto_resp = client + .post(&url) + .header("content-type", "application/x-protobuf") + .body(proto_body) + .send() + .await + .unwrap(); + assert!( + proto_resp.status().is_success(), + "protobuf /metrics POST should succeed" + ); + + // Malformed body -- neither JSON nor protobuf decodes, so the handler must answer 400. + let bad_resp = client + .post(&url) + .header("content-type", "application/json") + .body(b"this is not a metrics payload".to_vec()) + .send() + .await + .unwrap(); + assert_eq!(bad_resp.status().as_u16(), 400, "undecodable body must be rejected"); + + // Both good posts landed, so every metric appears twice. + let flat = mock.flatten_metrics(); + let count = |name: &str| flat.iter().filter(|rec| rec.name == name).count(); + assert_eq!(count("sum.metric"), 2); + assert_eq!(count("gauge.metric"), 2); + assert_eq!(count("hist.metric"), 2); + assert_eq!(count("none.metric"), 2); + + let sum = flat.iter().find(|rec| rec.name == "sum.metric").unwrap(); + assert_eq!(sum.value, 7, "Sum sums its integer data points"); + assert_eq!(sum.data_points, 2); + // The non-string service.name fell through to the empty default rather than being captured. + assert_eq!(sum.service_name, "", "non-string service.name yields the empty default"); + + let gauge = flat.iter().find(|rec| rec.name == "gauge.metric").unwrap(); + assert_eq!(gauge.value, 9, "Gauge sums its integer data points"); + + let hist = flat.iter().find(|rec| rec.name == "hist.metric").unwrap(); + assert_eq!(hist.value, 0, "Histogram contributes no summed value"); + assert_eq!(hist.data_points, 2, "Histogram reports its data-point count"); + + let none = flat.iter().find(|rec| rec.name == "none.metric").unwrap(); + assert_eq!( + (none.value, none.data_points), + (0, 0), + "a data-less metric flattens to zero" + ); +} diff --git a/libs/path/tests/find.rs b/libs/path/tests/find.rs index 8543cd4d..08a4f005 100644 --- a/libs/path/tests/find.rs +++ b/libs/path/tests/find.rs @@ -1,8 +1,4 @@ #![cfg(test)] -#![expect( - clippy::unwrap_used, - reason = "test code: failed tempdir/fs setup should fail the test" -)] use et_path::find_project_root; use fs_err as fs; diff --git a/libs/test-helpers/src/lib.rs b/libs/test-helpers/src/lib.rs index a767a641..6b45061f 100644 --- a/libs/test-helpers/src/lib.rs +++ b/libs/test-helpers/src/lib.rs @@ -4,7 +4,7 @@ //! domain-specific fixtures live in their own test-support crate instead -- e.g. `et-ws-test-server` //! (an in-process ws-server) or `int-otlp-mock` (a mock OTLP collector). #![expect( - clippy::expect_used, + clippy::unwrap_used, reason = "test helper: a missing free port or unpiped child stderr should fail the test loudly" )] @@ -21,7 +21,7 @@ use retry::retry; /// race (another process could grab the port before the caller binds it) that is acceptable in tests. #[must_use] pub fn reserve_port() -> u16 { - port_check::free_local_port().expect("no free local port") + port_check::free_local_port().unwrap() } /// Wait until `port` accepts a TCP connection, polling ~every 100ms for up to ~20s. @@ -70,13 +70,13 @@ impl Drop for ChildGuard { /// so read it after shutting the child down. The child must have been spawned with `Stdio::piped()`. #[must_use] pub fn drain_stderr(child: &mut Child) -> Arc> { - let stderr = child.stderr.take().expect("child stderr was not piped"); + let stderr = child.stderr.take().unwrap(); let log = Arc::new(Mutex::new(String::new())); let sink = Arc::clone(&log); drop(std::thread::spawn(move || { let mut buffer = String::new(); drop(std::io::BufReader::new(stderr).read_to_string(&mut buffer)); - *sink.lock().expect("stderr log mutex") = buffer; + *sink.lock().unwrap() = buffer; })); log } diff --git a/libs/ws-runner-common/tests/config.rs b/libs/ws-runner-common/tests/config.rs index b3b13ce7..8e3f64e9 100644 --- a/libs/ws-runner-common/tests/config.rs +++ b/libs/ws-runner-common/tests/config.rs @@ -4,9 +4,8 @@ //! variable is absent. #![cfg(test)] #![expect( - clippy::expect_used, clippy::duration_suboptimal_units, - reason = "test code: panics carry context, and exact second counts mirror the parsed inputs" + reason = "test code: exact second counts mirror the parsed inputs" )] use std::time::Duration; @@ -29,7 +28,7 @@ fn maps_runner_and_ws_env_vars() { ("RUNNER_TIMEOUT", "3m"), ("WS_SERVER_URL", "ws://example:9000/ws"), ]) - .expect("parse env"); + .unwrap(); assert_eq!(config.runner.module, "et-ws-data1"); assert_eq!(config.runner.timeout, Some(Duration::from_secs(180))); @@ -38,14 +37,14 @@ fn maps_runner_and_ws_env_vars() { #[test] fn humantime_seconds_suffix_parses() { - let config: Config = serde_env::from_iter([("RUNNER_MODULE", "m"), ("RUNNER_TIMEOUT", "120s")]).expect("parse env"); + let config: Config = serde_env::from_iter([("RUNNER_MODULE", "m"), ("RUNNER_TIMEOUT", "120s")]).unwrap(); assert_eq!(config.runner.timeout, Some(Duration::from_secs(120))); } #[test] fn absent_optionals_default() { - let config: Config = serde_env::from_iter([("RUNNER_MODULE", "m")]).expect("parse env"); + let config: Config = serde_env::from_iter([("RUNNER_MODULE", "m")]).unwrap(); assert_eq!(config.runner.timeout, None); assert!(config.ws.server_url.starts_with("ws://localhost:")); @@ -74,7 +73,7 @@ struct WsOnly { } fn load_ws() -> WsConfig { - serde_env::from_env::().expect("parse WsConfig from env").ws + serde_env::from_env::().unwrap().ws } #[test] diff --git a/services/modules/tests/symlinks.rs b/services/modules/tests/symlinks.rs index 8d175170..e536989a 100644 --- a/services/modules/tests/symlinks.rs +++ b/services/modules/tests/symlinks.rs @@ -12,10 +12,8 @@ #![cfg(test)] #![cfg(unix)] #![expect( - clippy::unwrap_used, - clippy::expect_used, clippy::deref_by_slicing, - reason = "test code: fixture setup failures should fail the test" + reason = "test code: slice-deref in fixture assertions is intentional" )] use std::os::unix::fs::symlink; @@ -78,9 +76,7 @@ async fn list_modules_follows_symlinks_to_package_dirs() { // symlinked stub root module -- should be discovered. let by_name: std::collections::HashMap<&str, &PathBuf> = found.iter().map(|(name, path)| (name.as_str(), path)).collect(); - let pkg_path = by_name - .get("onnxruntime-web") - .expect("symlinked onnxruntime-web should be discovered"); + let pkg_path = &by_name["onnxruntime-web"]; // The discovered path must resolve to the real package dir so that // `Files::new("/modules/onnxruntime-web", pkg_path)` can serve diff --git a/services/storage/tests/put.rs b/services/storage/tests/put.rs index 4e75f73d..e4949502 100644 --- a/services/storage/tests/put.rs +++ b/services/storage/tests/put.rs @@ -8,11 +8,6 @@ //! what wires the route into the test app. #![cfg(test)] -#![expect( - clippy::unwrap_used, - clippy::expect_used, - reason = "test code: setup and route invocation failures should fail the test" -)] use std::collections::BTreeMap; @@ -82,7 +77,7 @@ async fn rejects_multi_component_filename_with_400() { let result = put_file::<()>(http_req, payload, web::Data::new(registry), web::Data::new(config)).await; - let err = result.expect_err("multi-component filename must be rejected"); + let err = result.unwrap_err(); assert!(matches!(err, StorageError::InvalidFilename)); assert_eq!(err.status_code(), StatusCode::BAD_REQUEST); } diff --git a/services/ws-modules/face-detection/src/test_face_detection.rs b/services/ws-modules/face-detection/src/test_face_detection.rs index 682c45b7..887ee0fd 100644 --- a/services/ws-modules/face-detection/src/test_face_detection.rs +++ b/services/ws-modules/face-detection/src/test_face_detection.rs @@ -1,9 +1,8 @@ #![cfg(test)] #![expect( clippy::float_cmp, - clippy::indexing_slicing, clippy::default_numeric_fallback, - reason = "test code: exact float comparisons, slice indexing, and inline f64 fixtures are intentional" + reason = "test code: exact float comparisons and inline f64 fixtures are intentional" )] use super::*; diff --git a/services/ws-modules/face-detection/tests/web.rs b/services/ws-modules/face-detection/tests/web.rs index 77ba95d2..a0e16f96 100644 --- a/services/ws-modules/face-detection/tests/web.rs +++ b/services/ws-modules/face-detection/tests/web.rs @@ -15,7 +15,7 @@ fn init_can_be_called_more_than_once() { #[wasm_bindgen_test] fn stop_is_idempotent_when_runtime_has_not_started() { assert!(!is_running()); - stop().expect("stop should succeed when face detection is not running"); + stop().unwrap(); assert!(!is_running()); } @@ -26,7 +26,7 @@ async fn run_failure_leaves_runtime_stopped() { match result { Ok(()) => { assert!(is_running()); - stop().expect("stop should succeed after a successful run"); + stop().unwrap(); assert!(!is_running()); } Err(_) => { diff --git a/services/ws-modules/har1/src/test_har1.rs b/services/ws-modules/har1/src/test_har1.rs index dc4e7b73..2dd99834 100644 --- a/services/ws-modules/har1/src/test_har1.rs +++ b/services/ws-modules/har1/src/test_har1.rs @@ -1,9 +1,8 @@ #![cfg(test)] #![expect( clippy::float_cmp, - clippy::indexing_slicing, clippy::default_numeric_fallback, - reason = "test code: exact float comparisons, slice indexing, and inline f64 sensor fixtures are intentional" + reason = "test code: exact float comparisons and inline f64 sensor fixtures are intentional" )] use super::*; diff --git a/services/ws-modules/pydata1/pkg/et_ws_pydata1.js b/services/ws-modules/pydata1/pkg/et_ws_pydata1.js index 95abed66..a1278b43 100644 --- a/services/ws-modules/pydata1/pkg/et_ws_pydata1.js +++ b/services/ws-modules/pydata1/pkg/et_ws_pydata1.js @@ -126,6 +126,9 @@ export async function run() { pyodide.toPy(log), pyodide.toPy(() => {}), ); + } catch (err) { + log(`pydata1 run failed: ${String(err)}`); + throw err; } finally { if (globalThis.__etPyCov) await globalThis.__etPyCov.stop(pyodide, "pydata1"); client.disconnect(); diff --git a/services/ws-modules/pyface1/pkg/et_ws_pyface1.js b/services/ws-modules/pyface1/pkg/et_ws_pyface1.js index 2303e4df..688da912 100644 --- a/services/ws-modules/pyface1/pkg/et_ws_pyface1.js +++ b/services/ws-modules/pyface1/pkg/et_ws_pyface1.js @@ -108,6 +108,9 @@ export async function run() { pyodide.toPy(setStatus), pyodide.toPy(() => runtime !== state), ); + } catch (err) { + log(`pyface1 run failed: ${String(err)}`); + throw err; } finally { // Fires even when getUserMedia throws under the runner, so import + pre-camera lines still get credited. if (globalThis.__etPyCov) await globalThis.__etPyCov.stop(pyodide, "pyface1"); diff --git a/services/ws-modules/wasi-comm1/src/coverage.rs b/services/ws-modules/wasi-comm1/src/coverage.rs index e5232a22..822eae78 100644 --- a/services/ws-modules/wasi-comm1/src/coverage.rs +++ b/services/ws-modules/wasi-comm1/src/coverage.rs @@ -10,7 +10,7 @@ pub fn dump() { let mut coverage = Vec::new(); // SAFETY: single-threaded guest; capture_coverage reads the instrumented counters once at run() end. unsafe { - minicov::capture_coverage(&mut coverage).expect("minicov capture_coverage"); + minicov::capture_coverage(&mut coverage).unwrap(); } - fs_err::write("/cov/et_ws_wasi_comm1.profraw", coverage).expect("write /cov profraw"); + fs_err::write("/cov/et_ws_wasi_comm1.profraw", coverage).unwrap(); } diff --git a/services/ws-modules/wasi-data1/src/coverage.rs b/services/ws-modules/wasi-data1/src/coverage.rs index b597b7f1..86f43cac 100644 --- a/services/ws-modules/wasi-data1/src/coverage.rs +++ b/services/ws-modules/wasi-data1/src/coverage.rs @@ -10,7 +10,7 @@ pub fn dump() { let mut coverage = Vec::new(); // SAFETY: single-threaded guest; capture_coverage reads the instrumented counters once at run() end. unsafe { - minicov::capture_coverage(&mut coverage).expect("minicov capture_coverage"); + minicov::capture_coverage(&mut coverage).unwrap(); } - fs_err::write("/cov/et_ws_wasi_data1.profraw", coverage).expect("write /cov profraw"); + fs_err::write("/cov/et_ws_wasi_data1.profraw", coverage).unwrap(); } diff --git a/services/ws-pyo3-runner/tests/modules.rs b/services/ws-pyo3-runner/tests/modules.rs index 1ea1f142..57c7c9be 100644 --- a/services/ws-pyo3-runner/tests/modules.rs +++ b/services/ws-pyo3-runner/tests/modules.rs @@ -12,11 +12,9 @@ #![cfg(test)] #![expect( clippy::arithmetic_side_effects, - clippy::expect_used, clippy::float_arithmetic, - clippy::print_stderr, clippy::single_call_fn, - reason = "integration test: poll math, float tolerance check, spawn expects, skip notices, step helpers" + reason = "integration test: poll math, float tolerance check, step helpers" )] use std::error::Error; @@ -259,7 +257,7 @@ fn spawn_runner(module: &str, ws_url: &str) -> Child { .stdout(Stdio::inherit()) .stderr(Stdio::inherit()) .spawn() - .expect("failed to spawn et-ws-pyo3-runner") + .unwrap() } /// Open a control client and drive et-connect until we have an `agent_id`. diff --git a/services/ws-server/src/main.rs b/services/ws-server/src/main.rs index 120cac10..b30b2d91 100644 --- a/services/ws-server/src/main.rs +++ b/services/ws-server/src/main.rs @@ -29,20 +29,16 @@ struct Args { } #[actix_web::main] -async fn main() -> Result<(), std::io::Error> { +async fn main() -> Result<(), Box> { let args = Args::parse(); let env = serde_env::from_env::().unwrap(); eprintln!("Starting with env vars {env:#?}"); - #[expect( - clippy::option_if_let_else, - reason = "both branches log and configure distinct tracing subscribers; map_or_else hides the structure" - )] let otel_handles = if let Some(otlp_config) = &env.otlp { info!("OpenTelemetry configuration detected, initializing tracing..."); - Some(et_otlp::init(otlp_config)) + Some(et_otlp::init(otlp_config)?) } else { info!("No OpenTelemetry configuration detected, using default tracing settings..."); tracing_subscriber::registry() @@ -144,5 +140,6 @@ async fn main() -> Result<(), std::io::Error> { if let Some(handles) = otel_handles { handles.shutdown(); } - result + result?; + Ok(()) } diff --git a/services/ws-test-server/Cargo.toml b/services/ws-test-server/Cargo.toml index 6c3bf2e4..077808c6 100644 --- a/services/ws-test-server/Cargo.toml +++ b/services/ws-test-server/Cargo.toml @@ -13,21 +13,24 @@ doctest = false actix.workspace = true actix-rt.workspace = true actix-web.workspace = true +edge-toolkit.workspace = true et-modules-service.workspace = true et-storage-service.workspace = true et-test-helpers.workspace = true et-ws-service.workspace = true +futures-util.workspace = true +serde_json.workspace = true tempfile.workspace = true +tokio = { workspace = true, features = ["macros", "net", "rt", "time"] } +tokio-tungstenite = { workspace = true, features = ["connect"] } # Same TracingLogger setup as the real ws-server. # So tests that init OTLP in-process see server-side spans parented on the propagated traceparent. tracing-actix-web.workspace = true [dev-dependencies] -edge-toolkit.workspace = true -futures-util.workspace = true -serde_json.workspace = true -tokio = { workspace = true, features = ["macros", "rt", "time"] } -tokio-tungstenite = { workspace = true, features = ["connect"] } +et-otlp.workspace = true +int-otlp-mock.workspace = true +retry.workspace = true [lints] workspace = true diff --git a/services/ws-test-server/src/lib.rs b/services/ws-test-server/src/lib.rs index 3b96dd79..639498a8 100644 --- a/services/ws-test-server/src/lib.rs +++ b/services/ws-test-server/src/lib.rs @@ -1,15 +1,23 @@ #![expect( - clippy::unwrap_used, - clippy::expect_used, + clippy::arithmetic_side_effects, + clippy::needless_continue, clippy::panic, - reason = "in-process test ws-server; bind/startup failures should fail the test fast" + clippy::unwrap_used, + clippy::wildcard_enum_match_arm, + reason = "in-process test ws-server + ws client helpers; setup/protocol failures should fail the test fast" )] +use std::time::Duration; + use actix_web::{App, HttpServer, web}; +use edge_toolkit::ws::{ClientMessage, ServerMessage}; use et_modules_service::{ModulesConfig, configure as configure_modules}; use et_storage_service::{StorageConfig, configure as configure_storage}; use et_ws_service::{AgentSession, WsAgentRegistry, WsConfig, configure as configure_ws}; +use futures_util::{SinkExt as _, StreamExt as _}; use tempfile::TempDir; +use tokio_tungstenite::connect_async; +use tokio_tungstenite::tungstenite::Message; use tracing_actix_web::TracingLogger; /// A running test server. The temporary storage directory is cleaned up on drop. @@ -25,7 +33,7 @@ pub struct TestServer { /// Serves modules from the default module paths (same as production). #[must_use] pub fn start() -> TestServer { - let storage_dir = TempDir::new().expect("failed to create temp storage dir"); + let storage_dir = TempDir::new().unwrap(); let storage_path = storage_dir.path().to_path_buf(); let port = et_test_helpers::reserve_port(); @@ -68,7 +76,69 @@ pub fn start() -> TestServer { storage_dir, }; } - std::thread::sleep(std::time::Duration::from_millis(100)); + std::thread::sleep(Duration::from_millis(100)); } panic!("test ws-server did not start within 5 seconds on port {port}"); } + +/// Open a ws connection to `ws_url`, send `et-connect`, and return `(stream, agent_id)` once the +/// `et-connect-ack` has been observed. Lets a test drive the hub as a websocket client. +pub async fn connect_agent( + ws_url: &str, +) -> ( + tokio_tungstenite::WebSocketStream>, + String, +) { + let (mut stream, _) = connect_async(ws_url).await.unwrap(); + let connect_msg = serde_json::to_string(&ClientMessage::Connect { agent_id: None }).unwrap(); + stream.send(Message::text(connect_msg)).await.unwrap(); + + // Bound the ack wait: a server that accepts the socket but never sends `et-connect-ack` (and never closes) + // must fail the test fast rather than hang. Non-ack frames simply fall through and the loop reads the next. + let deadline = tokio::time::Instant::now() + Duration::from_secs(5); + while let Ok(Some(Ok(msg))) = tokio::time::timeout( + deadline.saturating_duration_since(tokio::time::Instant::now()), + stream.next(), + ) + .await + { + if let Message::Text(text) = &msg + && let Ok(ServerMessage::ConnectAck { agent_id, .. }) = serde_json::from_str::(text) + { + return (stream, agent_id); + } + } + panic!("never received et-connect-ack within 5s"); +} + +/// Pull the next frame from `stream`, skipping known protocol acks (`et-connect-ack`, +/// `et-message-status`, `et-response`) so callers see the next "real" payload. +pub async fn next_payload( + stream: &mut tokio_tungstenite::WebSocketStream>, +) -> Message { + let deadline = tokio::time::Instant::now() + Duration::from_secs(5); + loop { + let remaining = deadline.saturating_duration_since(tokio::time::Instant::now()); + let next = tokio::time::timeout(remaining, stream.next()).await.unwrap(); + let msg = next.unwrap().unwrap(); + match &msg { + Message::Text(text) => { + if serde_json::from_str::(text).is_ok_and(|parsed| { + matches!( + parsed, + ServerMessage::ConnectAck { .. } + | ServerMessage::MessageStatus { .. } + | ServerMessage::Response { .. } + ) + }) { + continue; + } + return msg; + } + Message::Binary(_) => return msg, + // Ping/pong and any other control frame: skip until a real payload arrives (or the deadline + // elapses / the stream closes, which then surfaces through the `.unwrap()` above). + _ => continue, + } + } +} diff --git a/services/ws-test-server/tests/helpers.rs b/services/ws-test-server/tests/helpers.rs new file mode 100644 index 00000000..98f51dc3 --- /dev/null +++ b/services/ws-test-server/tests/helpers.rs @@ -0,0 +1,71 @@ +//! Exercises the `connect_agent` / `next_payload` client helpers' fail-fast and frame-skipping paths that the +//! happy-path hub tests never reach: the ack-wait timeout, protocol-ack skipping, and control-frame skipping. +//! Each test drives the helper against a tiny scripted ws server that emits an exact frame sequence, so the +//! behaviour is deterministic rather than dependent on real-hub timing. +#![cfg(test)] + +use edge_toolkit::ws::{ConnectStatus, ServerMessage}; +use et_ws_test_server::{connect_agent, next_payload}; +use futures_util::SinkExt as _; +use tokio::net::TcpListener; +use tokio_tungstenite::tungstenite::Message; +use tokio_tungstenite::{accept_async, connect_async}; + +/// Start a ws server on a free port that accepts one connection, sends `frames` in order, then holds the +/// socket open. With an empty `frames` it simply accepts and stays silent -- a server that never acks. +async fn scripted_server(frames: Vec) -> String { + let listener = TcpListener::bind("127.0.0.1:0").await.unwrap(); + let port = listener.local_addr().unwrap().port(); + drop(tokio::spawn(async move { + let Ok((stream, _)) = listener.accept().await else { + return; + }; + let Ok(mut ws) = accept_async(stream).await else { + return; + }; + for frame in frames { + if ws.send(frame).await.is_err() { + return; + } + } + // Keep the connection open so the client can finish reading rather than seeing an early close. + std::future::pending::<()>().await; + })); + format!("ws://127.0.0.1:{port}") +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +#[should_panic(expected = "et-connect-ack")] +async fn connect_agent_times_out_when_server_never_acks() { + // The server accepts the socket but never sends `et-connect-ack`, so connect_agent must give up (panic) + // once its bound elapses rather than hang the test indefinitely. + let url = scripted_server(Vec::new()).await; + let _connected = connect_agent(&url).await; +} + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn next_payload_skips_control_frames_and_protocol_acks() { + let ack = serde_json::to_string(&ServerMessage::ConnectAck { + agent_id: "scripted".to_owned(), + status: ConnectStatus::Assigned, + }) + .unwrap(); + let frames = vec![ + // A control frame and a protocol ack both precede the real payload; next_payload must skip both. + Message::Ping(Vec::new()), + Message::text(ack), + Message::text("actual-payload"), + ]; + let url = scripted_server(frames).await; + + let (mut stream, _response) = connect_async(&url).await.unwrap(); + let payload = next_payload(&mut stream).await; + let Message::Text(text) = payload else { + panic!("expected the real text payload, got {payload:?}"); + }; + assert_eq!( + text.as_str(), + "actual-payload", + "next_payload should return the first non-ack payload" + ); +} diff --git a/services/ws-test-server/tests/hub_forwarding.rs b/services/ws-test-server/tests/hub_forwarding.rs index 20bad116..54d19b3e 100644 --- a/services/ws-test-server/tests/hub_forwarding.rs +++ b/services/ws-test-server/tests/hub_forwarding.rs @@ -4,80 +4,17 @@ #![cfg(test)] #![expect( - clippy::arithmetic_side_effects, - clippy::expect_used, - clippy::needless_continue, - clippy::panic, clippy::similar_names, - clippy::unwrap_used, - clippy::wildcard_enum_match_arm, - reason = "integration tests: panics/expects are how test failures surface; idiomatic test-time control flow" + reason = "integration tests: idiomatic test-time control flow" )] use std::time::Duration; -use edge_toolkit::ws::{ClientMessage, ServerMessage}; +use edge_toolkit::ws::ServerMessage; +use et_ws_test_server::{connect_agent, next_payload}; use futures_util::{SinkExt as _, StreamExt as _}; use tokio_tungstenite::{connect_async, tungstenite::Message}; -/// Open a ws connection, send `et-connect`, and return `(stream, agent_id)` -/// once `et-connect-ack` has been observed. -async fn connect_agent( - ws_url: &str, -) -> ( - tokio_tungstenite::WebSocketStream>, - String, -) { - let (mut stream, _) = connect_async(ws_url).await.expect("ws connect"); - let connect_msg = serde_json::to_string(&ClientMessage::Connect { agent_id: None }).unwrap(); - stream.send(Message::text(connect_msg)).await.expect("send connect"); - - while let Some(msg) = stream.next().await { - let msg = msg.expect("ws recv"); - let Message::Text(text) = msg else { - continue; - }; - if let Ok(ServerMessage::ConnectAck { agent_id, .. }) = serde_json::from_str::(&text) { - return (stream, agent_id); - } - } - panic!("never received et-connect-ack"); -} - -/// Pull the next frame from `stream`, ignoring known protocol acks -/// (`et-message-status`, `et-connect-ack`, etc.) so callers see the -/// next "real" payload. -async fn next_payload( - stream: &mut tokio_tungstenite::WebSocketStream>, -) -> Message { - let deadline = tokio::time::Instant::now() + Duration::from_secs(5); - loop { - let remaining = deadline.saturating_duration_since(tokio::time::Instant::now()); - let next = tokio::time::timeout(remaining, stream.next()) - .await - .expect("timed out waiting for ws frame"); - let msg = next.expect("ws stream closed").expect("ws recv"); - match &msg { - Message::Text(text) => { - if let Ok(parsed) = serde_json::from_str::(text) - && matches!( - parsed, - ServerMessage::ConnectAck { .. } - | ServerMessage::MessageStatus { .. } - | ServerMessage::Response { .. } - ) - { - continue; - } - return msg; - } - Message::Binary(_) => return msg, - Message::Ping(_) | Message::Pong(_) => continue, - other => panic!("unexpected control frame: {other:?}"), - } - } -} - #[tokio::test(flavor = "multi_thread", worker_threads = 2)] async fn unrecognised_text_is_broadcast_verbatim() { let server = et_ws_test_server::start(); @@ -88,7 +25,7 @@ async fn unrecognised_text_is_broadcast_verbatim() { // A frame the server can't parse as ClientMessage -- no `type` field, no // recognisable shape. The hub fallback should forward it verbatim. let raw = r#"{"hello":"world","nested":{"n":42}}"#; - sender.send(Message::text(raw)).await.expect("send unknown text"); + sender.send(Message::text(raw)).await.unwrap(); let received = next_payload(&mut receiver).await; let Message::Text(received_text) = received else { @@ -118,10 +55,7 @@ async fn unrecognised_binary_is_broadcast_verbatim() { // Arbitrary opaque bytes -- the server has no way to interpret these, // so the hub fallback must forward them as-is. let payload: Vec = vec![0x00, 0x01, 0x02, 0xff, 0xfe, 0xfd, b'a', b'b', b'c']; - sender - .send(Message::binary(payload.clone())) - .await - .expect("send binary"); + sender.send(Message::binary(payload.clone())).await.unwrap(); let received = next_payload(&mut receiver).await; let Message::Binary(received_bytes) = received else { @@ -142,14 +76,12 @@ async fn unconnected_client_is_auto_registered_and_relays_both_ways() { let (mut peer, _peer_id) = connect_agent(&server.ws_url).await; // A "dumb" client that never sends et-connect -- e.g. a demo frontend on // a raw `new WebSocket(url)`. - let (mut dumb, _) = connect_async(&server.ws_url).await.expect("ws connect"); + let (mut dumb, _) = connect_async(&server.ws_url).await.unwrap(); // The dumb client's first binary frame must be broadcast to the peer: // sending it auto-registers the dumb client as an agent. let activations: Vec = vec![0x10, 0x20, 0x30, 0x40]; - dumb.send(Message::binary(activations.clone())) - .await - .expect("dumb send binary"); + dumb.send(Message::binary(activations.clone())).await.unwrap(); let received = next_payload(&mut peer).await; let Message::Binary(received_bytes) = received else { @@ -164,9 +96,7 @@ async fn unconnected_client_is_auto_registered_and_relays_both_ways() { // Reverse direction: the peer's reply must reach the now auto-registered // dumb client -- it became a broadcast recipient on its first frame. let grads: Vec = vec![0xaa, 0xbb, 0xcc]; - peer.send(Message::binary(grads.clone())) - .await - .expect("peer send binary"); + peer.send(Message::binary(grads.clone())).await.unwrap(); let reply = next_payload(&mut dumb).await; let Message::Binary(reply_bytes) = reply else { @@ -178,3 +108,30 @@ async fn unconnected_client_is_auto_registered_and_relays_both_ways() { "auto-registered client must receive peer broadcasts" ); } + +#[tokio::test(flavor = "multi_thread", worker_threads = 2)] +async fn direct_message_to_unknown_agent_is_rejected() { + let server = et_ws_test_server::start(); + let (mut agent, _agent_id) = connect_agent(&server.ws_url).await; + + // No agent with this id is registered, so the hub answers Invalid via handle_send_direct's queue miss. + let send = serde_json::json!({ + "type": "et-send-agent-message", + "to_agent_id": "no-such-agent", + "message": {"hello": "world"}, + }); + agent.send(Message::text(send.to_string())).await.unwrap(); + + let reply = next_payload(&mut agent).await; + let Message::Text(text) = reply else { + panic!("expected an Invalid text frame, got {reply:?}"); + }; + let parsed = serde_json::from_str::(&text).unwrap(); + let ServerMessage::Invalid { detail, .. } = parsed else { + panic!("expected ServerMessage::Invalid, got {parsed:?}"); + }; + assert!( + detail.contains("unknown target agent") && detail.contains("no-such-agent"), + "unexpected invalid detail: {detail}" + ); +} diff --git a/services/ws-test-server/tests/otel.rs b/services/ws-test-server/tests/otel.rs new file mode 100644 index 00000000..0ad3d2b2 --- /dev/null +++ b/services/ws-test-server/tests/otel.rs @@ -0,0 +1,70 @@ +//! End-to-end `OTel` coverage for the ws hub across all three signals -- traces, +//! logs, and metrics -- captured by the in-process OTLP mock. +//! +//! Boots the hub via `et_ws_test_server`, points the process's OTLP exporters at +//! the mock (installed as the global tracer/logger/meter providers by +//! `et_otlp::init`), drives a websocket client so the hub emits a `ws.connect` +//! span, connection `info!` logs, and the connection/message metrics, then +//! flushes on `OtelHandles::shutdown` and asserts every signal arrived. +#![cfg(test)] + +use edge_toolkit::config::{OtlpConfig, OtlpProtocol}; +use et_ws_test_server::connect_agent; +use futures_util::SinkExt as _; +use retry::delay::Fixed; +use retry::retry; +use tokio_tungstenite::tungstenite::Message; + +#[tokio::test] +async fn hub_emits_traces_logs_and_metrics() { + let mock = int_otlp_mock::start(); + + // Point the in-process OTLP exporters at the mock and install the global providers. + let mut otlp = OtlpConfig::default(); + otlp.collector_url = mock.collector_url().to_owned(); + otlp.protocol = OtlpProtocol::JSON; + otlp.service_label = "et-ws-test".to_string(); + otlp.auth = None; + let handles = et_otlp::init(&otlp).unwrap(); + + let server = et_ws_test_server::start(); + + // Drive one client through connect + a relayed frame, then disconnect, so the hub emits a `ws.connect` + // span, connection `info!` logs, and the connection/message metrics. + { + let (mut stream, _agent_id) = connect_agent(&server.ws_url).await; + stream.send(Message::text("hello hub")).await.unwrap(); + stream.close(None).await.unwrap(); + } + + // Flush batched spans/logs/metrics to the mock. + handles.shutdown(); + + // Traces: a `ws.connect` span tagged with our service name. + retry(Fixed::from_millis(200).take(75), || { + mock.flatten_spans() + .iter() + .any(|span| span.service_name == "et-ws-test" && span.name == "ws.connect") + .then_some(()) + .ok_or(()) + }) + .unwrap(); + + // Logs: the hub's connection `info!` lines reached the collector. + assert!(!mock.logs().is_empty(), "expected hub info! logs at the collector"); + + // Metrics: the inbound-message counter (>= 1 after the connect + relayed frame). + let metrics = retry(Fixed::from_millis(200).take(75), || { + let metrics = mock.flatten_metrics(); + metrics + .iter() + .any(|metric| metric.name == "et_ws.messages.received" && metric.value >= 1) + .then_some(metrics) + .ok_or(()) + }) + .unwrap(); + assert!( + metrics.iter().any(|metric| metric.name == "et_ws.connections.active"), + "expected the et_ws.connections.active gauge, got {metrics:?}" + ); +} diff --git a/services/ws-wasi-runner/src/main.rs b/services/ws-wasi-runner/src/main.rs index 1f3b6c70..d21d2a41 100644 --- a/services/ws-wasi-runner/src/main.rs +++ b/services/ws-wasi-runner/src/main.rs @@ -3,15 +3,11 @@ use et_ws_wasi_runner::run_module; use tracing::info; #[tokio::main] -async fn main() -> Result<(), Box> { +async fn main() -> Result<(), Box> { let config = serde_env::from_env::()?; - #[expect( - clippy::option_if_let_else, - reason = "None branch installs an alternate tracing subscriber as a side effect; map_or_else hides it" - )] let otel_handles = if let Some(otlp_config) = &config.otlp { - Some(et_otlp::init(otlp_config)) + Some(et_otlp::init(otlp_config)?) } else { tracing_subscriber::fmt() .with_env_filter(tracing_subscriber::EnvFilter::try_from_default_env().unwrap_or_else(|_| "info".into())) diff --git a/services/ws-wasi-runner/tests/modules.rs b/services/ws-wasi-runner/tests/modules.rs index 7d693d26..fde7856e 100644 --- a/services/ws-wasi-runner/tests/modules.rs +++ b/services/ws-wasi-runner/tests/modules.rs @@ -4,7 +4,6 @@ //! components rather than browser-targeted JS. #![cfg(test)] -#![expect(clippy::expect_used, reason = "test code: process spawn failure fails the test")] use edge_toolkit::config::{Language, mise_env_includes}; use rstest::rstest; @@ -36,7 +35,7 @@ fn module_runs_successfully(#[case] module: &str, #[case] language: Language) { .env("WS_SERVER_URL", &server.ws_url) .env("ET_TEST_WS_WASI_RUNNER_FAST_EXIT", "1") .status() - .expect("failed to spawn et-ws-wasi-runner"); + .unwrap(); assert!(status.success(), "{module} exited with code {:?}", status.code()); } diff --git a/services/ws-wasi-runner/tests/otel_propagation.rs b/services/ws-wasi-runner/tests/otel_propagation.rs index 83bf88d0..f597870c 100644 --- a/services/ws-wasi-runner/tests/otel_propagation.rs +++ b/services/ws-wasi-runner/tests/otel_propagation.rs @@ -22,10 +22,9 @@ #![cfg(test)] #![expect( - clippy::expect_used, clippy::uninlined_format_args, clippy::needless_collect, - reason = "test code: expect failures fail the test; assertion-helper format/collect idioms" + reason = "test code: assertion-helper format/collect idioms" )] use std::collections::HashSet; @@ -61,7 +60,7 @@ fn trace_ids_propagate_between_runner_and_server() { server_otlp.protocol = OtlpProtocol::JSON; server_otlp.service_label = "et-ws-test".to_string(); server_otlp.auth = None; - let server_handles = et_otlp::init(&server_otlp); + let server_handles = et_otlp::init(&server_otlp).unwrap(); let server = et_ws_test_server::start(); @@ -76,7 +75,7 @@ fn trace_ids_propagate_between_runner_and_server() { .env("OTLP_PROTOCOL", "JSON") .env("OTLP_SERVICE_LABEL", "et-ws-wasi-runner") .status() - .expect("failed to spawn et-ws-wasi-runner"); + .unwrap(); assert!(status.success(), "runner exited with code {:?}", status.code()); diff --git a/services/ws-wasi-runner/tests/vector_otlp_relay.rs b/services/ws-wasi-runner/tests/vector_otlp_relay.rs index eaf68d93..d8461dd2 100644 --- a/services/ws-wasi-runner/tests/vector_otlp_relay.rs +++ b/services/ws-wasi-runner/tests/vector_otlp_relay.rs @@ -22,12 +22,7 @@ //! wire format both into and out of Vector is protobuf on the `/traces` path. #![cfg(test)] -#![expect( - clippy::expect_used, - clippy::panic, - clippy::single_call_fn, - reason = "test code: loud failures and named single-use step helpers" -)] +#![expect(clippy::single_call_fn, reason = "test code: named single-use step helpers")] use std::process::{Command, Stdio}; use std::sync::Mutex; @@ -60,7 +55,7 @@ fn vector_relays_buffered_otlp_after_backend_comes_online() { // The tempdir is Vector's buffer data_dir; it exists, so Vector just nests // its buffer directory inside it. - let tmp = tempfile::tempdir().expect("create tempdir"); + let tmp = tempfile::tempdir().unwrap(); let config_path = edge_toolkit::config::get_project_root().join("config/vector-otlp-relay.yaml"); // 2. Start Vector from the static config. Its sink target (mock_port) is @@ -79,7 +74,7 @@ fn vector_relays_buffered_otlp_after_backend_comes_online() { .stdout(Stdio::null()) .stderr(Stdio::piped()) .spawn() - .expect("spawn vector"); + .unwrap(); // Drain Vector's stderr into memory so it stays quiet on success but is // available for failure messages (a file-backed `Stdio` would need the // banned `std::fs::File`). @@ -100,7 +95,7 @@ fn vector_relays_buffered_otlp_after_backend_comes_online() { .header("content-type", "application/x-protobuf") .body(otlp_trace_request()) .send() - .expect("POST OTLP to vector source"); + .unwrap(); assert!( response.status().is_success(), "vector source rejected the OTLP push: {}", @@ -185,5 +180,5 @@ fn stop_and_read(vector: &mut ChildGuard, log: &Mutex) -> String { vector.shutdown(); // Give the drainer thread a moment to flush the final bytes. std::thread::sleep(Duration::from_millis(200)); - format!("--- vector stderr ---\n{}", log.lock().expect("log mutex")) + format!("--- vector stderr ---\n{}", log.lock().unwrap()) } diff --git a/services/ws-web-runner/Cargo.toml b/services/ws-web-runner/Cargo.toml index 96a63c75..a7904029 100644 --- a/services/ws-web-runner/Cargo.toml +++ b/services/ws-web-runner/Cargo.toml @@ -47,8 +47,9 @@ winapi.workspace = true [build-dependencies] # Compiles mingw-shim/ for the x86_64-pc-windows-gnu build (see build.rs). -# Unconditional so the build script itself compiles on every platform -- the shim branch is gated at -# runtime on the target cfg. +# Used only in build.rs's `#[cfg(windows)]` branch, so these go unreferenced on non-Windows hosts -- harmless, +# since that branch (and the functions it calls) don't compile there, which also keeps them out of the Linux +# coverage report. cc.workspace = true fs-err.workspace = true diff --git a/services/ws-web-runner/build.rs b/services/ws-web-runner/build.rs index 79c658e6..6fc209eb 100644 --- a/services/ws-web-runner/build.rs +++ b/services/ws-web-runner/build.rs @@ -14,11 +14,6 @@ //! rlib that carries the `rusty_v8` archive -- GNU linkers resolve archives left-to-right, so the same //! libs emitted as `rustc-link-lib` from this crate would precede the archive and satisfy nothing. -#![expect( - clippy::expect_used, - reason = "build-script code: a panic is the only failure channel cargo gives, and expect names the invariant" -)] - fn main() { // cc emits rerun-if-env-changed directives, which switches cargo off its rerun-on-any-file default -- // so the shim sources must be declared explicitly or edits to them silently don't rebuild. @@ -27,6 +22,19 @@ fn main() { println!("cargo:rerun-if-changed=mingw-shim/msvc_crt_locale.c"); println!("cargo:rerun-if-changed=mingw-shim/msvc_crt_alloc.c"); + // The shim is only linked for the windows-gnu target, which is only ever built on a Windows host, so the + // whole branch compiles there and nowhere else -- keeping it out of the Linux coverage build entirely. + #[cfg(windows)] + link_mingw_shim(); +} + +#[cfg(windows)] +#[expect( + clippy::single_call_fn, + clippy::unwrap_used, + reason = "windows-gnu link setup: single call site, and unwraps assert build invariants" +)] +fn link_mingw_shim() { let target_os = std::env::var("CARGO_CFG_TARGET_OS").unwrap_or_default(); let target_env = std::env::var("CARGO_CFG_TARGET_ENV").unwrap_or_default(); let target_abi = std::env::var("CARGO_CFG_TARGET_ABI").unwrap_or_default(); @@ -39,7 +47,7 @@ fn main() { .file("mingw-shim/msvc_crt_ops.s") .compile("msvc_crt_shim"); - let out_dir = std::env::var("OUT_DIR").expect("cargo always sets OUT_DIR"); + let out_dir = std::env::var("OUT_DIR").unwrap(); // msvc_crt_locale.c must be a standalone OBJECT on the link line, not an archive member: its strong // definitions have to intercept names that -lmsvcrt (earlier in the default-libs block) would @@ -60,7 +68,7 @@ fn main() { // equivalent -- the shim + import libs below stand in for it. Satisfy the directives with empty // archives ("!\n" is a valid zero-member ar file) in OUT_DIR, which cc put on the search path. for name in ["liblibcmt.a", "liboldnames.a"] { - fs_err::write(format!("{out_dir}/{name}"), b"!\n").expect("OUT_DIR is writable during build scripts"); + fs_err::write(format!("{out_dir}/{name}"), b"!\n").unwrap(); } // The archive's std::exception_ptr internals (__ExceptionPtr*) are exported by msvcp140.dll, which @@ -87,9 +95,12 @@ fn main() { println!("cargo:rustc-link-arg=-fuse-ld=lld"); } +#[cfg(windows)] +#[expect( + clippy::unwrap_used, + reason = "build script: a panic is cargo's only failure channel for a failed command" +)] fn run(command: &mut std::process::Command) { - let status = command - .status() - .expect("gendef/dlltool come from winlibs, on PATH in the mingw mise env"); + let status = command.status().unwrap(); assert!(status.success(), "{command:?} exited with {status}"); } diff --git a/services/ws-web-runner/tests/modules.rs b/services/ws-web-runner/tests/modules.rs index 9fb79fa2..919c7455 100644 --- a/services/ws-web-runner/tests/modules.rs +++ b/services/ws-web-runner/tests/modules.rs @@ -34,12 +34,6 @@ //! the `SharedArrayBuffer` transfer. #![cfg(test)] -#![expect( - clippy::expect_used, - clippy::panic, - clippy::print_stdout, - reason = "test code: process spawn failure or non-zero exit fails the test; module-skip log lines use println" -)] use edge_toolkit::config::{Language, mise_env_includes}; #[cfg(feature = "coverage")] @@ -197,7 +191,7 @@ fn run_runner(module: &str, ws_url: &str, timeout_secs: u32) -> std::process::Ou .env("WS_SERVER_URL", ws_url) .env("RUNNER_TIMEOUT", format!("{timeout_secs}s")) .output() - .expect("failed to spawn et-ws-web-runner") + .unwrap() } /// Collect the coverage a module PUT into the test server's storage. @@ -227,18 +221,15 @@ fn collect_module_coverage(server: &et_ws_test_server::TestServer) { match path.extension().and_then(|ext| ext.to_str()) { Some("coverage") => { let dest_dir = root.join("target/pycov"); - fs::create_dir_all(&dest_dir).expect("create target/pycov"); - let stem = path - .file_stem() - .expect("coverage data file has a stem") - .to_string_lossy(); - let _copied = fs::copy(&path, dest_dir.join(format!(".coverage.{stem}"))).expect("copy pycov"); + fs::create_dir_all(&dest_dir).unwrap(); + let stem = path.file_stem().unwrap().to_string_lossy(); + let _copied = fs::copy(&path, dest_dir.join(format!(".coverage.{stem}"))).unwrap(); } Some("profraw") => { let dest_dir = root.join("target/wasi-cov"); - fs::create_dir_all(&dest_dir).expect("create target/wasi-cov"); - let name = path.file_name().expect("profraw has a file name"); - let _copied = fs::copy(&path, dest_dir.join(name)).expect("copy wasmcov profraw"); + fs::create_dir_all(&dest_dir).unwrap(); + let name = path.file_name().unwrap(); + let _copied = fs::copy(&path, dest_dir.join(name)).unwrap(); } _ => {} } diff --git a/services/ws/src/lib.rs b/services/ws/src/lib.rs index af14ee4e..961e4701 100644 --- a/services/ws/src/lib.rs +++ b/services/ws/src/lib.rs @@ -1,4 +1,5 @@ use std::collections::BTreeMap; +use std::sync::LazyLock; use std::time::{Duration, Instant}; use actix_web::{Error, HttpRequest, HttpResponse, web}; @@ -10,6 +11,7 @@ use edge_toolkit::ws_server::{AgentRecord, AgentRegistry, PendingDirectMessage, use futures_util::StreamExt as _; use opentelemetry::{ global, + metrics::{Counter, UpDownCounter}, trace::{Span, Tracer as _}, }; use serde::Deserialize; @@ -31,6 +33,21 @@ pub const HEARTBEAT_INTERVAL: Duration = Duration::from_secs(1); /// `[ws] max_frame_size` to `WS_MAX_FRAME_SIZE`). pub const DEFAULT_MAX_FRAME_SIZE: usize = 64 * 1024 * 1024; +// Hub metrics, recorded through the global meter `et_otlp::init` installs (mirrors the `global::tracer` use above). +// Built lazily on first use -- by then the meter provider is set -- and cached for the process. +static MESSAGES_RECEIVED: LazyLock> = LazyLock::new(|| { + global::meter("ws-server") + .u64_counter("et_ws.messages.received") + .with_description("Inbound WebSocket frames the hub has handled") + .build() +}); +static ACTIVE_CONNECTIONS: LazyLock> = LazyLock::new(|| { + global::meter("ws-server") + .i64_up_down_counter("et_ws.connections.active") + .with_description("Currently-open WebSocket connections") + .build() +}); + /// Runtime knobs for the WebSocket hub. Populated by `serde-env` in /// `et-ws-server::main`, then handed to `configure`. #[serde_inline_default] @@ -279,6 +296,10 @@ impl Connection { } } + #[expect( + clippy::cognitive_complexity, + reason = "linear send/queue/unknown-recipient dispatch; splitting scatters the three status replies" + )] async fn handle_send_direct( &mut self, span: &mut impl Span, @@ -287,13 +308,19 @@ impl Connection { message: serde_json::Value, ) { let server_received_at = Utc::now().to_rfc3339(); - let (pending, recipient_session) = self.registry.queue_direct( + let Some((pending, recipient_session)) = self.registry.queue_direct( Uuid::now_v7().to_string(), &from_agent_id, &to_agent_id, server_received_at, message, - ); + ) else { + warn!("direct message target {to_agent_id} is not a connected agent"); + self.send_invalid(None, format!("unknown target agent {to_agent_id}")) + .await; + span.end(); + return; + }; let message_id = pending.message_id.clone(); if let Some(recipient) = recipient_session { @@ -364,7 +391,9 @@ impl Connection { clippy::too_many_lines, reason = "single dispatcher for inbound ClientMessage variants; splitting it scatters handlers into trivial fns" )] + // skipcq: RS-R1000 -- dispatcher cyclomatic complexity is inherent to the ClientMessage match; not splittable async fn handle_inbound(&mut self, msg: AggregatedMessage) -> bool { + MESSAGES_RECEIVED.add(1, &[]); match msg { AggregatedMessage::Ping(ping) => { self.mark_activity(); @@ -456,18 +485,8 @@ impl Connection { return true; } - if !self - .registry - .list_agents() - .iter() - .any(|agent| agent.agent_id == to_agent_id) - { - self.send_invalid(None, format!("unknown target agent {to_agent_id}")) - .await; - span.end(); - return true; - } - + // Unknown / departed recipients are handled by handle_send_direct's queue miss + // below -- a single place that answers Invalid -- so there is no pre-check here. self.handle_send_direct(&mut span, from_agent_id, to_agent_id, message) .await; return true; @@ -610,6 +629,7 @@ impl Connection { self.current_agent_id() ); connect_span.end(); + ACTIVE_CONNECTIONS.add(1, &[]); let mut heartbeat = tokio::time::interval(HEARTBEAT_INTERVAL); heartbeat.set_missed_tick_behavior(tokio::time::MissedTickBehavior::Delay); @@ -661,6 +681,7 @@ impl Connection { } } + ACTIVE_CONNECTIONS.add(-1, &[]); if let Some(agent_id) = self.agent_id.as_deref() { self.registry.mark_disconnected(agent_id); info!("Agent {} disconnected; last known IP {}", agent_id, self.client_ip); diff --git a/services/ws/tests/config.rs b/services/ws/tests/config.rs index 30142d90..5bff5ada 100644 --- a/services/ws/tests/config.rs +++ b/services/ws/tests/config.rs @@ -4,9 +4,8 @@ //! default when the variable is absent. #![cfg(test)] #![expect( - clippy::expect_used, clippy::decimal_literal_representation, - reason = "test code: expect panics carry context; byte sizes read clearer as decimal MiB math than hex" + reason = "test code: byte sizes read clearer as decimal MiB math than hex" )] use et_ws_service::WsConfig; @@ -20,7 +19,7 @@ struct Wrapper { } fn load() -> WsConfig { - serde_env::from_env::().expect("parse WsConfig from env").ws + serde_env::from_env::().unwrap().ws } #[test] diff --git a/utilities/cli/tests/module_package_json.rs b/utilities/cli/tests/module_package_json.rs index 721f55a4..c590865e 100644 --- a/utilities/cli/tests/module_package_json.rs +++ b/utilities/cli/tests/module_package_json.rs @@ -1,9 +1,4 @@ #![cfg(test)] -#![expect( - clippy::unwrap_used, - clippy::indexing_slicing, - reason = "test code: setup failures and missing JSON fields should fail the test" -)] use et_cli::generate_module_package_json; use fs_err as fs; diff --git a/utilities/cli/tests/scenario_generation.rs b/utilities/cli/tests/scenario_generation.rs index 21e1229c..a91d1de9 100644 --- a/utilities/cli/tests/scenario_generation.rs +++ b/utilities/cli/tests/scenario_generation.rs @@ -1,9 +1,4 @@ #![cfg(test)] -#![expect( - clippy::unwrap_used, - clippy::indexing_slicing, - reason = "test code: setup failures and missing JSON fields should fail the test" -)] use et_cli::{ docker_image_module_paths, generate_deployment, module_package_json, regenerate_verification, scenario_module_paths, diff --git a/utilities/int-gen/src/lib.rs b/utilities/int-gen/src/lib.rs index f91d4f89..c83dafac 100644 --- a/utilities/int-gen/src/lib.rs +++ b/utilities/int-gen/src/lib.rs @@ -122,7 +122,7 @@ pub fn generate() -> Result<(), Error> { /// `ws.kdl`/`*.schema.json`/`rest.yaml`), so this is the prerequisite step /// every per-language `gen:*` mise task depends on. #[expect( - clippy::expect_used, + clippy::unwrap_used, clippy::unwrap_in_result, reason = "pretty_yaml only fails on malformed YAML and serde output is always well-formed" )] @@ -141,14 +141,12 @@ pub fn generate_core() -> Result<(), Error> { let yaml = serde_yaml::to_string(&spec_value)?; // serde_yaml always emits well-formed YAML, so pretty_yaml's parse step // can't fail here -- the only error variant is a syntax error. - let yaml = pretty_yaml::format_text(&yaml, &pretty_yaml::config::FormatOptions::default()) - .expect("serde_yaml output should always be well-formed"); + let yaml = pretty_yaml::format_text(&yaml, &pretty_yaml::config::FormatOptions::default()).unwrap(); write_if_changed(&specs_dir.join("ws.yaml"), &yaml)?; // REST OpenAPI doc -- emitted from utoipa annotations on actual handlers. let rest_yaml = openapi::render_yaml(); - let rest_yaml = pretty_yaml::format_text(&rest_yaml, &pretty_yaml::config::FormatOptions::default()) - .expect("utoipa output should always be well-formed YAML"); + let rest_yaml = pretty_yaml::format_text(&rest_yaml, &pretty_yaml::config::FormatOptions::default()).unwrap(); write_if_changed(&specs_dir.join("rest.yaml"), &rest_yaml)?; // Build intermediates land in target/ -- datamodel-codegen reads the JSON diff --git a/utilities/int-gen/src/openapi.rs b/utilities/int-gen/src/openapi.rs index 048d01de..4b67ddef 100644 --- a/utilities/int-gen/src/openapi.rs +++ b/utilities/int-gen/src/openapi.rs @@ -45,27 +45,27 @@ struct ApiDoc; /// but progenitor 0.14 only accepts 3.0.x and rejects the `identifier` field -- /// downgrade those before serializing. #[expect( - clippy::expect_used, + clippy::unwrap_used, reason = "all conversions are between serde-derived types; the only way these expect calls fire is a serde_json bug" )] fn build_spec() -> openapiv3::OpenAPI { let mut doc = ApiDoc::openapi(); doc.info.license = None; - let mut value = serde_json::to_value(&doc).expect("OpenApi is always JSON-serializable"); + let mut value = serde_json::to_value(&doc).unwrap(); if let Some(obj) = value.as_object_mut() { let _previous = obj.insert("openapi".into(), serde_json::Value::String("3.0.3".into())); } - serde_json::from_value(value).expect("downgraded OpenApi is always openapiv3::OpenAPI-shaped") + serde_json::from_value(value).unwrap() } /// Serialize the `OpenAPI` document as YAML for `generated/specs/rest.yaml`. #[must_use] #[expect( - clippy::expect_used, + clippy::unwrap_used, reason = "openapiv3::OpenAPI is serde-derived and round-trips through serde_yaml unconditionally" )] pub fn render_yaml() -> String { - serde_yaml::to_string(&build_spec()).expect("openapiv3::OpenAPI is always YAML-serializable") + serde_yaml::to_string(&build_spec()).unwrap() } /// Serialize the `OpenAPI` document as JSON. @@ -74,11 +74,11 @@ pub fn render_yaml() -> String { /// v0.2.0). #[must_use] #[expect( - clippy::expect_used, + clippy::unwrap_used, reason = "openapiv3::OpenAPI is serde-derived and round-trips through serde_json unconditionally" )] pub fn render_json() -> String { - serde_json::to_string_pretty(&build_spec()).expect("openapiv3::OpenAPI is always JSON-serializable") + serde_json::to_string_pretty(&build_spec()).unwrap() } /// Generate the Rust REST client (`generated/rust-rest/src/lib.rs`) from the @@ -93,7 +93,7 @@ pub fn render_json() -> String { /// tracing works without each call site repeating the boilerplate the old /// `inject_traceparent` helper did. #[expect( - clippy::expect_used, + clippy::unwrap_used, clippy::unwrap_in_result, reason = "progenitor's emit feeds straight into syn::parse2; a parse failure means progenitor produced invalid Rust" )] @@ -131,7 +131,7 @@ pub fn render_rust_client() -> Result { let mut settings = progenitor::GenerationSettings::default(); let mut generator = progenitor::Generator::new(settings.with_pre_hook_async(trace_hook)); let tokens = generator.generate_tokens(&spec)?; - let ast = syn::parse2(tokens).expect("progenitor always emits valid Rust"); + let ast = syn::parse2(tokens).unwrap(); let body = prettyplease::unparse(&ast); let body = inject_wasm_baseurl_fallback(&body); let body = inject_retry_exec(&body); diff --git a/utilities/int-gen/src/wit/messages.rs b/utilities/int-gen/src/wit/messages.rs index 4147e8cf..ff4ccc99 100644 --- a/utilities/int-gen/src/wit/messages.rs +++ b/utilities/int-gen/src/wit/messages.rs @@ -39,7 +39,7 @@ fn to_kebab(input: &str) -> String { type EnumSet = HashSet; #[expect( - clippy::expect_used, + clippy::unwrap_used, clippy::unwrap_in_result, reason = "the semver literal is a compile-time constant; an Err means the literal was mistyped" )] @@ -62,7 +62,7 @@ pub fn render(client_schema: &Schema, server_schema: &Schema) -> Result Result<(), Error> { clippy::unnecessary_wraps, clippy::single_call_fn, clippy::unwrap_used, - clippy::expect_used, clippy::unwrap_in_result, reason = "Result lets caller use ? like fetch_* helpers; called once; wit-parser anyhow::Error, inputs literals" )] @@ -290,7 +289,7 @@ fn strip_webgpu(raw: &str) -> Result { let mut webgpu = wit_encoder::packages_from_parsed(&resolve) .into_iter() .find(|pkg| pkg.name().namespace() == "wasi" && pkg.name().name().raw_name() == "webgpu") - .expect("upstream webgpu.wit declared a non-`wasi:webgpu` package"); + .unwrap(); let keep: HashSet<&str> = WEBGPU_KEEP_NAMES.iter().copied().collect(); let drop_methods: HashSet<&str> = WEBGPU_DROP_METHODS.iter().copied().collect();