From c32f7c6234911ab2ce3760cf0f11a7f73d922313 Mon Sep 17 00:00:00 2001 From: James Date: Fri, 3 Oct 2025 10:51:31 -0400 Subject: [PATCH 01/24] feat: enhance otel implementation by setting more span info on request receipt --- Cargo.toml | 2 +- src/axum.rs | 84 +++++++++++++++++++++----- src/lib.rs | 10 ++-- src/pubsub/mod.rs | 1 + src/pubsub/shared.rs | 79 ++++++++++++++++++++----- src/router.rs | 28 ++++++++- src/routes/ctx.rs | 133 +++++++++++++++++++++++++++++++----------- src/routes/handler.rs | 15 ++--- src/routes/mod.rs | 5 +- src/types/batch.rs | 4 ++ 10 files changed, 280 insertions(+), 81 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index 27d779b..7de86da 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -5,7 +5,7 @@ description = "Simple, modern, ergonomic JSON-RPC 2.0 router built with tower an keywords = ["json-rpc", "jsonrpc", "json"] categories = ["web-programming::http-server", "web-programming::websocket"] -version = "0.3.4" +version = "0.4.0" edition = "2021" rust-version = "1.81" authors = ["init4", "James Prestwich"] diff --git a/src/axum.rs b/src/axum.rs index 7bb337e..4fd1db1 100644 --- a/src/axum.rs +++ b/src/axum.rs @@ -1,6 +1,6 @@ use crate::{ types::{InboundData, Response}, - HandlerCtx, TaskSet, + HandlerCtx, TaskSet, TracingInfo, }; use axum::{ extract::FromRequest, @@ -8,8 +8,16 @@ use axum::{ response::IntoResponse, }; use bytes::Bytes; -use std::{future::Future, pin::Pin}; +use std::{ + future::Future, + pin::Pin, + sync::{ + atomic::{AtomicU32, Ordering}, + Arc, + }, +}; use tokio::runtime::Handle; +use tracing::{debug, debug_span}; /// A wrapper around an [`Router`] that implements the /// [`axum::handler::Handler`] trait. This struct is an implementation detail @@ -21,7 +29,13 @@ use tokio::runtime::Handle; #[derive(Debug, Clone)] pub(crate) struct IntoAxum { pub(crate) router: crate::Router, + pub(crate) task_set: TaskSet, + + /// Counter for OTEL messages received. + pub(crate) rx_msg_id: Arc, + /// Counter for OTEL messages sent. + pub(crate) tx_msg_id: Arc, } impl From> for IntoAxum { @@ -29,6 +43,8 @@ impl From> for IntoAxum { Self { router, task_set: Default::default(), + rx_msg_id: Arc::new(AtomicU32::new(0)), + tx_msg_id: Arc::new(AtomicU32::new(0)), } } } @@ -39,12 +55,35 @@ impl IntoAxum { Self { router, task_set: handle.into(), + rx_msg_id: Arc::new(AtomicU32::new(0)), + tx_msg_id: Arc::new(AtomicU32::new(0)), } } +} - /// Get a new context, built from the task set. +impl IntoAxum +where + S: Clone + Send + Sync + 'static, +{ fn ctx(&self) -> HandlerCtx { - self.task_set.clone().into() + let request_span = debug_span!( + "ajj.IntoAxum::call", + "otel.kind" = "server", + "rpc.system" = "jsonrpc", + "rpc.jsonrpc.version" = "2.0", + "rpc.service" = self.router.service_name(), + notifications_enabled = false, + params = tracing::field::Empty + ); + + HandlerCtx::new( + None, + self.task_set.clone(), + TracingInfo { + service: self.router.service_name(), + request_span, + }, + ) } } @@ -56,25 +95,44 @@ where fn call(self, req: axum::extract::Request, state: S) -> Self::Future { Box::pin(async move { + let ctx = self.ctx(); + let Ok(bytes) = Bytes::from_request(req, &state).await else { return Box::::from(Response::parse_error()).into_response(); }; - // If the inbound data is not currently parsable, we - // send an empty one it to the router, as the router enforces - // the specification. - let req = InboundData::try_from(bytes).unwrap_or_default(); + // https://github.com/open-telemetry/semantic-conventions/blob/main/docs/rpc/rpc-spans.md#message-event + let req = ctx.span().in_scope(|| { + debug!( + "rpc.message.id" = self.rx_msg_id.fetch_add(1, Ordering::Relaxed), + "rpc.message.type" = "received", + "rpc.message.uncompressed_size" = bytes.len(), + "Received request" + ); + + // If the inbound data is not currently parsable, we + // send an empty one it to the router, as the router enforces + // the specification. + InboundData::try_from(bytes).unwrap_or_default() + }); - if let Some(response) = self - .router - .call_batch_with_state(self.ctx(), req, state) - .await - { + let span = ctx.span().clone(); + if let Some(response) = self.router.call_batch_with_state(ctx, req, state).await { let headers = [( header::CONTENT_TYPE, HeaderValue::from_static(mime::APPLICATION_JSON.as_ref()), )]; + let body = Box::::from(response); + + span.in_scope(|| { + debug!( + "rpc.message.id" = self.tx_msg_id.fetch_add(1, Ordering::Relaxed), + "rpc.message.type" = "received", + "Received request" + ); + }); + (headers, body).into_response() } else { ().into_response() diff --git a/src/lib.rs b/src/lib.rs index a5da1e6..e84dbec 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -24,7 +24,7 @@ //! }) //! // Routes get a ctx, which can be used to send notifications. //! .route("notify", |ctx: HandlerCtx| async move { -//! if ctx.notifications().is_none() { +//! if !ctx.notifications_enabled() { //! // This error will appear in the ResponsePayload's `data` field. //! return Err("notifications are disabled"); //! } @@ -171,6 +171,7 @@ pub use pubsub::ReadJsonStream; mod routes; pub use routes::{ BatchFuture, Handler, HandlerArgs, HandlerCtx, NotifyError, Params, RouteFuture, State, + TracingInfo, }; pub(crate) use routes::{BoxedIntoRoute, ErasedIntoRoute, Method, Route}; @@ -206,7 +207,8 @@ pub(crate) mod test_utils { mod test { use crate::{ - router::RouterInner, routes::HandlerArgs, test_utils::assert_rv_eq, ResponsePayload, + router::RouterInner, routes::HandlerArgs, test_utils::assert_rv_eq, HandlerCtx, + ResponsePayload, }; use bytes::Bytes; use serde_json::value::RawValue; @@ -232,7 +234,7 @@ mod test { let res = router .call_with_state( HandlerArgs { - ctx: Default::default(), + ctx: HandlerCtx::mock(), req: req.try_into().unwrap(), }, (), @@ -251,7 +253,7 @@ mod test { let res2 = router .call_with_state( HandlerArgs { - ctx: Default::default(), + ctx: HandlerCtx::mock(), req: req2.try_into().unwrap(), }, (), diff --git a/src/pubsub/mod.rs b/src/pubsub/mod.rs index 64dfb79..51aa896 100644 --- a/src/pubsub/mod.rs +++ b/src/pubsub/mod.rs @@ -95,6 +95,7 @@ mod ipc; pub use ipc::ReadJsonStream; mod shared; +pub(crate) use shared::WriteItem; pub use shared::{ConnectionId, DEFAULT_NOTIFICATION_BUFFER_PER_CLIENT}; mod shutdown; diff --git a/src/pubsub/shared.rs b/src/pubsub/shared.rs index 74ed4b3..b11b91e 100644 --- a/src/pubsub/shared.rs +++ b/src/pubsub/shared.rs @@ -1,11 +1,14 @@ use crate::{ pubsub::{In, JsonSink, Listener, Out}, types::InboundData, - HandlerCtx, TaskSet, + HandlerCtx, TaskSet, TracingInfo, }; use core::fmt; use serde_json::value::RawValue; -use std::sync::{atomic::AtomicU64, Arc}; +use std::sync::{ + atomic::{AtomicU32, AtomicU64, Ordering}, + Arc, +}; use tokio::{pin, runtime::Handle, select, sync::mpsc, task::JoinHandle}; use tokio_stream::StreamExt; use tokio_util::sync::WaitForCancellationFutureOwned; @@ -105,8 +108,7 @@ impl ConnectionManager { /// Increment the connection ID counter and return an unused ID. fn next_id(&self) -> ConnectionId { - self.next_id - .fetch_add(1, std::sync::atomic::Ordering::Relaxed) + self.next_id.fetch_add(1, Ordering::Relaxed) } /// Get a clone of the router. @@ -131,13 +133,15 @@ impl ConnectionManager { write_task: tx, requests, tasks: tasks.clone(), + rx_msg_id: Arc::new(AtomicU32::new(0)), }; let wt = WriteTask { tasks, conn_id, - json: rx, + items: rx, connection, + tx_msg_id: Arc::new(AtomicU32::new(0)), }; (rt, wt) @@ -168,11 +172,14 @@ struct RouteTask { /// Connection ID for the connection serviced by this task. pub(crate) conn_id: ConnectionId, /// Sender to the write task. - pub(crate) write_task: mpsc::Sender>, + pub(crate) write_task: mpsc::Sender, /// Stream of requests. pub(crate) requests: In, /// The task set for this connection pub(crate) tasks: TaskSet, + + /// Counter for OTEL messages received. + pub(crate) rx_msg_id: Arc, } impl fmt::Debug for RouteTask { @@ -199,6 +206,7 @@ where mut requests, write_task, tasks, + rx_msg_id, .. } = self; @@ -224,6 +232,8 @@ where break; }; + let item_bytes = item.len(); + // If the inbound data is not currently parsable, we // send an empty one it to the router, as the router // enforces the specification. @@ -234,16 +244,38 @@ where // if the client stops accepting responses, we do not keep // handling inbound requests. let Ok(permit) = write_task.clone().reserve_owned().await else { - tracing::error!("write task dropped while waiting for permit"); + error!("write task dropped while waiting for permit"); break; }; + let tracing = TracingInfo { service: router.service_name(), request_span: debug_span!( + parent: None, + "ajj.pubsub.RouteTask::call", + "otel.kind" = "server", + "rpc.system" = "jsonrpc", + "rpc.jsonrpc.version" = "2.0", + "rpc.service" = router.service_name(), + notifications_enabled = true, + params = tracing::field::Empty + ) }; + let ctx = HandlerCtx::new( Some(write_task.clone()), children.clone(), + tracing, ); + let span = ctx.span().clone(); + span.in_scope(|| { + debug!( + "rpc.message.id" = rx_msg_id.fetch_add(1, Ordering::Relaxed), + "rpc.message.type" = "received", + "rpc.message.uncompressed_size" = item_bytes, + "Received request" + ); + }); + // Run the future in a new task. let fut = router.handle_request_batch(ctx, reqs); @@ -252,9 +284,9 @@ where // Send the response to the write task. // we don't care if the receiver has gone away, // as the task is done regardless. - if let Some(rv) = fut.await { + if let Some(json) = fut.await { let _ = permit.send( - rv + WriteItem { span, json } ); } } @@ -275,6 +307,13 @@ where } } +/// An item to be written to an outbound JSON pubsub stream. +#[derive(Debug, Clone)] +pub(crate) struct WriteItem { + pub(crate) span: tracing::Span, + pub(crate) json: Box, +} + /// The Write Task is responsible for writing JSON to the outbound connection. struct WriteTask { /// Task set @@ -287,10 +326,13 @@ struct WriteTask { /// /// Dropping this channel will cause the associated [`RouteTask`] to /// shutdown. - pub(crate) json: mpsc::Receiver>, + pub(crate) items: mpsc::Receiver, /// Outbound connections. pub(crate) connection: Out, + + /// Counter for OTEL messages sent. + pub(crate) tx_msg_id: Arc, } impl WriteTask { @@ -305,8 +347,9 @@ impl WriteTask { pub(crate) async fn task_future(self) { let WriteTask { tasks, - mut json, + mut items, mut connection, + tx_msg_id, .. } = self; @@ -318,12 +361,20 @@ impl WriteTask { debug!("Shutdown signal received"); break; } - json = json.recv() => { - let Some(json) = json else { + item = items.recv() => { + let Some(WriteItem { span, json }) = item else { tracing::error!("Json stream has closed"); break; }; - let span = debug_span!("WriteTask", conn_id = self.conn_id); + span.record("conn_id", self.conn_id); + span.in_scope(|| { + debug!( + "rpc.message.id" = tx_msg_id.fetch_add(1, Ordering::Relaxed), + "rpc.message.type" = "sent", + "Sending response" + ); + }); + if let Err(err) = connection.send_json(json).instrument(span).await { debug!(%err, conn_id = self.conn_id, "Failed to send json"); break; diff --git a/src/router.rs b/src/router.rs index a589896..2846a7d 100644 --- a/src/router.rs +++ b/src/router.rs @@ -95,6 +95,22 @@ where } } + /// Create a new, empty router with the specified OpenTelemetry service + /// name. + pub fn new_named(service_name: &'static str) -> Self { + Self { + inner: Arc::new(RouterInner { + service_name: Some(service_name), + ..RouterInner::new() + }), + } + } + + /// Get the OpenTelemetry service name for this router. + pub fn service_name(&self) -> &'static str { + self.inner.service_name.unwrap_or("ajj") + } + /// If this router is the only reference to its inner state, return the /// inner state. Otherwise, clone the inner state and return the clone. fn into_inner(self) -> RouterInner { @@ -104,6 +120,7 @@ where routes: arc.routes.clone(), last_id: arc.last_id, fallback: arc.fallback.clone(), + service_name: arc.service_name, name_to_id: arc.name_to_id.clone(), id_to_name: arc.id_to_name.clone(), }, @@ -301,7 +318,7 @@ where pub fn call_with_state(&self, args: HandlerArgs, state: S) -> RouteFuture { let id = args.req().id_owned(); let method = args.req().method(); - let span = debug_span!(parent: None, "Router::call_with_state", %method, ?id); + let span = debug_span!(parent: args.span(), "Router::call_with_state", %method, ?id); self.inner.call_with_state(args, state).with_span(span) } @@ -316,7 +333,7 @@ where let mut fut = BatchFuture::new_with_capacity(inbound.single(), inbound.len()); // According to spec, non-parsable requests should still receive a // response. - let span = debug_span!(parent: None, "BatchFuture::poll", reqs = inbound.len(), futs = tracing::field::Empty); + let span = debug_span!(parent: ctx.span(), "BatchFuture::poll", reqs = inbound.len(), futs = tracing::field::Empty); for (batch_idx, req) in inbound.iter().enumerate() { let req = req.map(|req| { @@ -473,6 +490,10 @@ pub(crate) struct RouterInner { /// The handler to call when no method is found. fallback: Method, + /// An optional service name for OpenTelemetry tracing. This is not + /// set by default. + service_name: Option<&'static str>, + // next 2 fields are used for reverse lookup of method names /// A map from method names to their IDs. name_to_id: BTreeMap, MethodId>, @@ -502,6 +523,8 @@ impl RouterInner { fallback: Method::Ready(Route::default_fallback()), + service_name: None, + name_to_id: BTreeMap::new(), id_to_name: BTreeMap::new(), } @@ -523,6 +546,7 @@ impl RouterInner { .collect(), fallback: self.fallback.with_state(state), last_id: self.last_id, + service_name: self.service_name, name_to_id: self.name_to_id, id_to_name: self.id_to_name, } diff --git a/src/routes/ctx.rs b/src/routes/ctx.rs index 3a1553a..30b1aa8 100644 --- a/src/routes/ctx.rs +++ b/src/routes/ctx.rs @@ -1,9 +1,12 @@ -use crate::{types::Request, RpcSend, TaskSet}; +use crate::{pubsub::WriteItem, types::Request, RpcSend, TaskSet}; use serde_json::value::RawValue; use std::future::Future; -use tokio::{runtime::Handle, sync::mpsc, task::JoinHandle}; +use tokio::{ + sync::mpsc::{self, error::SendError}, + task::JoinHandle, +}; use tokio_util::sync::WaitForCancellationFutureOwned; -use tracing::error; +use tracing::{enabled, error, Level}; /// Errors that can occur when sending notifications. #[derive(thiserror::Error, Debug)] @@ -13,7 +16,37 @@ pub enum NotifyError { Serde(#[from] serde_json::Error), /// The notification channel was closed. #[error("notification channel closed")] - Send(#[from] mpsc::error::SendError>), + Send(#[from] SendError>), +} + +impl From> for NotifyError { + fn from(value: SendError) -> Self { + SendError(value.0.json).into() + } +} + +/// Tracing information for OpenTelemetry. This struct is used to store +/// information about the current request that can be used for tracing. +#[derive(Debug, Clone)] +#[non_exhaustive] +pub struct TracingInfo { + /// The OpenTelemetry service name. + pub service: &'static str, + + /// The request span. + pub request_span: tracing::Span, +} + +impl TracingInfo { + /// Create a mock tracing info for testing. + #[cfg(test)] + pub fn mock() -> Self { + use tracing::debug_span; + Self { + service: "test", + request_span: debug_span!("test"), + } + } } /// A context for handler requests that allow the handler to send notifications @@ -25,52 +58,62 @@ pub enum NotifyError { /// - Sending notifications to pubsub clients via [`HandlerCtx::notify`]. /// Notifcations SHOULD be valid JSON-RPC objects, but this is /// not enforced by the type system. -#[derive(Debug, Clone, Default)] +#[derive(Debug, Clone)] pub struct HandlerCtx { - pub(crate) notifications: Option>>, + pub(crate) notifications: Option>, /// A task set on which to spawn tasks. This is used to coordinate pub(crate) tasks: TaskSet, -} -impl From for HandlerCtx { - fn from(tasks: TaskSet) -> Self { - Self { - notifications: None, - tasks, - } - } -} - -impl From for HandlerCtx { - fn from(handle: Handle) -> Self { - Self { - notifications: None, - tasks: handle.into(), - } - } + /// Tracing information for OpenTelemetry. + pub(crate) tracing: TracingInfo, } impl HandlerCtx { /// Create a new handler context. - #[allow(dead_code)] // used in pubsub and axum features pub(crate) const fn new( - notifications: Option>>, + notifications: Option>, tasks: TaskSet, + tracing: TracingInfo, ) -> Self { Self { notifications, tasks, + tracing, } } - /// Get a reference to the notification sender. This is used to - /// send notifications over pubsub transports. - pub const fn notifications(&self) -> Option<&mpsc::Sender>> { - self.notifications.as_ref() + /// Create a mock handler context for testing. + #[cfg(test)] + pub fn mock() -> Self { + Self { + notifications: None, + tasks: TaskSet::default(), + tracing: TracingInfo::mock(), + } } - /// Check if notiifcations can be sent to the client. This will be false + /// Get a reference to the tracing information for this handler context. + pub const fn tracing_info(&self) -> &TracingInfo { + &self.tracing + } + + /// Get the OpenTelemetry service name for this handler context. + pub const fn otel_service_name(&self) -> &'static str { + self.tracing.service + } + + /// Get a reference to the tracing span for this handler context. + pub const fn span(&self) -> &tracing::Span { + &self.tracing.request_span + } + + /// Set the tracing information for this handler context. + pub fn set_tracing_info(&mut self, tracing: TracingInfo) { + self.tracing = tracing; + } + + /// Check if notifications can be sent to the client. This will be false /// when either the transport does not support notifications, or the /// notification channel has been closed (due the the client going away). pub fn notifications_enabled(&self) -> bool { @@ -84,7 +127,12 @@ impl HandlerCtx { pub async fn notify(&self, t: &T) -> Result<(), NotifyError> { if let Some(notifications) = self.notifications.as_ref() { let rv = serde_json::value::to_raw_value(t)?; - notifications.send(rv).await?; + notifications + .send(WriteItem { + span: self.span().clone(), + json: rv, + }) + .await?; } Ok(()) @@ -218,8 +266,17 @@ pub struct HandlerArgs { impl HandlerArgs { /// Create new handler arguments. - pub const fn new(ctx: HandlerCtx, req: Request) -> Self { - Self { ctx, req } + pub fn new(ctx: HandlerCtx, req: Request) -> Self { + let this = Self { ctx, req }; + + let span = this.span(); + span.record("otel.name", this.otel_span_name()); + span.record("rpc.method", this.req.method()); + if enabled!(Level::TRACE) { + span.record("params", this.req.params()); + } + + this } /// Get a reference to the handler context. @@ -227,8 +284,18 @@ impl HandlerArgs { &self.ctx } + /// Get a reference to the tracing span for this handler invocation. + pub const fn span(&self) -> &tracing::Span { + self.ctx.span() + } + /// Get a reference to the JSON-RPC request. pub const fn req(&self) -> &Request { &self.req } + + /// Get the OpenTelemetry span name for this handler invocation. + pub fn otel_span_name(&self) -> String { + format!("{}/{}", self.ctx.otel_service_name(), self.req.method()) + } } diff --git a/src/routes/handler.rs b/src/routes/handler.rs index dda25da..980a919 100644 --- a/src/routes/handler.rs +++ b/src/routes/handler.rs @@ -3,7 +3,7 @@ use crate::{ }; use serde_json::value::RawValue; use std::{convert::Infallible, future::Future, marker::PhantomData, pin::Pin, task}; -use tracing::{debug_span, enabled, trace, Instrument, Level}; +use tracing::{trace, Instrument}; macro_rules! convert_result { ($res:expr) => {{ @@ -412,16 +412,9 @@ where fn call(&mut self, args: HandlerArgs) -> Self::Future { let this = self.clone(); Box::pin(async move { - let notifications_enabled = args.ctx.notifications_enabled(); - - let span = debug_span!( - "HandlerService::call", - notifications_enabled, - params = tracing::field::Empty - ); - if enabled!(Level::TRACE) { - span.record("params", args.req.params()); - } + // This span captures standard OpenTelemetry attributes for + // JSON-RPC according to OTEL semantic conventions. + let span = args.span().clone(); Ok(this .handler diff --git a/src/routes/mod.rs b/src/routes/mod.rs index ecafb38..2fee397 100644 --- a/src/routes/mod.rs +++ b/src/routes/mod.rs @@ -1,5 +1,5 @@ mod ctx; -pub use ctx::{HandlerArgs, HandlerCtx, NotifyError}; +pub use ctx::{HandlerArgs, HandlerCtx, NotifyError, TracingInfo}; mod erased; pub(crate) use erased::{BoxedIntoRoute, ErasedIntoRoute, MakeErasedHandler}; @@ -14,6 +14,7 @@ pub use handler::{Handler, Params, State}; mod method; pub(crate) use method::Method; +use crate::types::Response; use serde_json::value::RawValue; use std::{ convert::Infallible, @@ -22,8 +23,6 @@ use std::{ use tower::{util::BoxCloneSyncService, Service, ServiceExt}; use tracing::{debug_span, enabled, Level}; -use crate::types::Response; - /// A JSON-RPC handler for a specific method. /// /// A route is a [`BoxCloneSyncService`] that takes JSON parameters and may diff --git a/src/types/batch.rs b/src/types/batch.rs index 9ee745e..1b650ff 100644 --- a/src/types/batch.rs +++ b/src/types/batch.rs @@ -58,6 +58,10 @@ impl TryFrom for InboundData { if enabled!(Level::TRACE) { tracing::span::Span::current().record("bytes", format!("0x{:x}", bytes)); } + + // This event exists only so that people who use default console + // logging setups still see the span details. Without this event, the + // span would not show up in logs. debug!("Parsing inbound data"); // We set up the deserializer to read from the byte buffer. From f67601f4ece458af9db7947bddf6a6961b2d2825 Mon Sep 17 00:00:00 2001 From: James Date: Fri, 3 Oct 2025 12:05:48 -0400 Subject: [PATCH 02/24] refactor: simplify conn_id --- src/pubsub/shared.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/pubsub/shared.rs b/src/pubsub/shared.rs index b11b91e..830262e 100644 --- a/src/pubsub/shared.rs +++ b/src/pubsub/shared.rs @@ -255,6 +255,7 @@ where "rpc.system" = "jsonrpc", "rpc.jsonrpc.version" = "2.0", "rpc.service" = router.service_name(), + conn_id = self.conn_id, notifications_enabled = true, params = tracing::field::Empty ) }; @@ -366,7 +367,6 @@ impl WriteTask { tracing::error!("Json stream has closed"); break; }; - span.record("conn_id", self.conn_id); span.in_scope(|| { debug!( "rpc.message.id" = tx_msg_id.fetch_add(1, Ordering::Relaxed), From 5182f6b085947c3a0ef2a771539707ee986c7e76 Mon Sep 17 00:00:00 2001 From: James Date: Fri, 3 Oct 2025 12:07:20 -0400 Subject: [PATCH 03/24] fix: share counters --- src/pubsub/axum.rs | 2 ++ src/pubsub/shared.rs | 10 ++++++++-- 2 files changed, 10 insertions(+), 2 deletions(-) diff --git a/src/pubsub/axum.rs b/src/pubsub/axum.rs index 338b099..ad305d1 100644 --- a/src/pubsub/axum.rs +++ b/src/pubsub/axum.rs @@ -126,6 +126,8 @@ impl AxumWsCfg { next_id: arc.next_id.clone(), router: arc.router.clone(), notification_buffer_per_task: arc.notification_buffer_per_task, + tx_msg_id: arc.tx_msg_id.clone(), + rx_msg_id: arc.rx_msg_id.clone(), }, } } diff --git a/src/pubsub/shared.rs b/src/pubsub/shared.rs index 830262e..5140b1d 100644 --- a/src/pubsub/shared.rs +++ b/src/pubsub/shared.rs @@ -70,6 +70,10 @@ pub(crate) struct ConnectionManager { pub(crate) router: crate::Router<()>, pub(crate) notification_buffer_per_task: usize, + + // OTEL message counters + pub(crate) tx_msg_id: Arc, + pub(crate) rx_msg_id: Arc, } impl ConnectionManager { @@ -80,6 +84,8 @@ impl ConnectionManager { next_id: AtomicU64::new(0).into(), router, notification_buffer_per_task: DEFAULT_NOTIFICATION_BUFFER_PER_CLIENT, + tx_msg_id: Arc::new(AtomicU32::new(0)), + rx_msg_id: Arc::new(AtomicU32::new(0)), } } @@ -133,7 +139,7 @@ impl ConnectionManager { write_task: tx, requests, tasks: tasks.clone(), - rx_msg_id: Arc::new(AtomicU32::new(0)), + rx_msg_id: self.rx_msg_id.clone(), }; let wt = WriteTask { @@ -141,7 +147,7 @@ impl ConnectionManager { conn_id, items: rx, connection, - tx_msg_id: Arc::new(AtomicU32::new(0)), + tx_msg_id: self.tx_msg_id.clone(), }; (rt, wt) From 30ea8e7d61372d4bbd008ca5b7d38cab0d8c9fa4 Mon Sep 17 00:00:00 2001 From: James Date: Fri, 3 Oct 2025 12:21:59 -0400 Subject: [PATCH 04/24] refactor: make ctx private to prevent misuse --- src/lib.rs | 10 ++------- src/routes/ctx.rs | 23 +++++++++++++++++--- src/routes/handler.rs | 50 +++++++++++++++++++++---------------------- src/routes/mod.rs | 7 +++--- 4 files changed, 50 insertions(+), 40 deletions(-) diff --git a/src/lib.rs b/src/lib.rs index e84dbec..e83fd11 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -233,10 +233,7 @@ mod test { let res = router .call_with_state( - HandlerArgs { - ctx: HandlerCtx::mock(), - req: req.try_into().unwrap(), - }, + HandlerArgs::new(HandlerCtx::mock(), req.try_into().unwrap()), (), ) .await @@ -252,10 +249,7 @@ mod test { let res2 = router .call_with_state( - HandlerArgs { - ctx: HandlerCtx::mock(), - req: req2.try_into().unwrap(), - }, + HandlerArgs::new(HandlerCtx::mock(), req2.try_into().unwrap()), (), ) .await diff --git a/src/routes/ctx.rs b/src/routes/ctx.rs index 30b1aa8..c2d591a 100644 --- a/src/routes/ctx.rs +++ b/src/routes/ctx.rs @@ -259,15 +259,22 @@ impl HandlerCtx { #[derive(Debug, Clone)] pub struct HandlerArgs { /// The handler context. - pub(crate) ctx: HandlerCtx, + ctx: HandlerCtx, /// The JSON-RPC request. - pub(crate) req: Request, + req: Request, + + /// prevent instantation outside of this module + _seal: (), } impl HandlerArgs { /// Create new handler arguments. pub fn new(ctx: HandlerCtx, req: Request) -> Self { - let this = Self { ctx, req }; + let this = Self { + ctx, + req, + _seal: (), + }; let span = this.span(); span.record("otel.name", this.otel_span_name()); @@ -279,6 +286,11 @@ impl HandlerArgs { this } + /// Decompose the handler arguments into its parts. + pub fn into_parts(self) -> (HandlerCtx, Request) { + (self.ctx, self.req) + } + /// Get a reference to the handler context. pub const fn ctx(&self) -> &HandlerCtx { &self.ctx @@ -294,6 +306,11 @@ impl HandlerArgs { &self.req } + /// Get the ID of the JSON-RPC request, if any. + pub fn id_owned(&self) -> Option> { + self.req.id_owned() + } + /// Get the OpenTelemetry span name for this handler invocation. pub fn otel_span_name(&self) -> String { format!("{}/{}", self.ctx.otel_service_name(), self.req.method()) diff --git a/src/routes/handler.rs b/src/routes/handler.rs index 980a919..1761177 100644 --- a/src/routes/handler.rs +++ b/src/routes/handler.rs @@ -453,7 +453,7 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { - let id = args.req.id_owned(); + let id = args.id_owned(); Box::pin(async move { let payload = self().await; @@ -473,8 +473,8 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { - let id = args.req.id_owned(); - let ctx = args.ctx; + let id = args.id_owned(); + let (ctx, _) = args.into_parts(); Box::pin(async move { let payload = self(ctx).await; @@ -495,7 +495,7 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { - let HandlerArgs { req, .. } = args; + let (_, req) = args.into_parts(); Box::pin(async move { let id = req.id_owned(); let Ok(params) = req.deser_params() else { @@ -519,7 +519,7 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { - let HandlerArgs { req, .. } = args; + let (_, req) = args.into_parts(); Box::pin(async move { let id = req.id_owned(); let Ok(params) = req.deser_params() else { @@ -543,7 +543,7 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { - let id = args.req.id_owned(); + let id = args.id_owned(); Box::pin(async move { let payload = self(state).await; Response::maybe(id.as_deref(), &payload) @@ -562,7 +562,7 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { - let id = args.req.id_owned(); + let id = args.id_owned(); Box::pin(async move { let payload = self(State(state)).await; Response::maybe(id.as_deref(), &payload) @@ -583,7 +583,7 @@ where fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { Box::pin(async move { - let HandlerArgs { ctx, req } = args; + let (ctx, req) = args.into_parts(); let id = req.id_owned(); let Ok(params) = req.deser_params() else { @@ -611,7 +611,7 @@ where fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { Box::pin(async move { - let HandlerArgs { ctx, req } = args; + let (ctx, req) = args.into_parts(); let id = req.id_owned(); let Ok(params) = req.deser_params() else { @@ -639,7 +639,7 @@ where fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { Box::pin(async move { - let HandlerArgs { req, .. } = args; + let (_, req) = args.into_parts(); let id = req.id_owned(); let Ok(params) = req.deser_params() else { @@ -667,7 +667,7 @@ where fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { Box::pin(async move { - let HandlerArgs { ctx, req } = args; + let (ctx, req) = args.into_parts(); let id = req.id_owned(); @@ -692,7 +692,7 @@ where fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { Box::pin(async move { - let HandlerArgs { ctx, req } = args; + let (ctx, req) = args.into_parts(); let id = req.id_owned(); @@ -719,7 +719,7 @@ where fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { Box::pin(async move { - let HandlerArgs { ctx, req } = args; + let (ctx, req) = args.into_parts(); let id = req.id_owned(); let Ok(params) = req.deser_params() else { @@ -744,7 +744,7 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { - let id = args.req.id_owned(); + let id = args.id_owned(); drop(args); Box::pin(async move { let payload = self().await; @@ -763,7 +763,7 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { - let HandlerArgs { ctx, req } = args; + let (ctx, req) = args.into_parts(); let id = req.id_owned(); @@ -788,7 +788,7 @@ where fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { Box::pin(async move { - let HandlerArgs { req, .. } = args; + let (_, req) = args.into_parts(); let id = req.id_owned(); let Ok(params) = req.deser_params() else { @@ -816,7 +816,7 @@ where fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { Box::pin(async move { - let HandlerArgs { req, .. } = args; + let (_, req) = args.into_parts(); let id = req.id_owned(); let Ok(params) = req.deser_params() else { @@ -843,7 +843,7 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { - let id = args.req.id_owned(); + let id = args.id_owned(); Box::pin(async move { let payload = convert_result!(self(state).await); Response::maybe(id.as_deref(), &payload) @@ -862,7 +862,7 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { - let id = args.req.id_owned(); + let id = args.id_owned(); Box::pin(async move { let payload = convert_result!(self(State(state)).await); Response::maybe(id.as_deref(), &payload) @@ -883,7 +883,7 @@ where fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { Box::pin(async move { - let HandlerArgs { ctx, req } = args; + let (ctx, req) = args.into_parts(); let id = req.id_owned(); let Ok(params) = req.deser_params() else { @@ -911,7 +911,7 @@ where fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { Box::pin(async move { - let HandlerArgs { ctx, req } = args; + let (ctx, req) = args.into_parts(); let id = req.id_owned(); let Ok(params) = req.deser_params() else { @@ -940,7 +940,7 @@ where fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { Box::pin(async move { - let HandlerArgs { req, .. } = args; + let (_, req) = args.into_parts(); let id = req.id_owned(); let Ok(params) = req.deser_params() else { @@ -967,7 +967,7 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { - let HandlerArgs { ctx, req } = args; + let (ctx, req) = args.into_parts(); let id = req.id_owned(); @@ -992,7 +992,7 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { - let HandlerArgs { ctx, req } = args; + let (ctx, req) = args.into_parts(); let id = req.id_owned(); @@ -1019,7 +1019,7 @@ where fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { Box::pin(async move { - let HandlerArgs { ctx, req } = args; + let (ctx, req) = args.into_parts(); let id = req.id_owned(); let Ok(params) = req.deser_params() else { diff --git a/src/routes/mod.rs b/src/routes/mod.rs index 2fee397..07af8e8 100644 --- a/src/routes/mod.rs +++ b/src/routes/mod.rs @@ -51,9 +51,8 @@ impl Route { /// Create a default fallback route that returns a method not found error. pub(crate) fn default_fallback() -> Self { Self::new(tower::service_fn(|args: HandlerArgs| async { - let HandlerArgs { req, .. } = args; - let id = req.id_owned(); - drop(req); + let id = args.id_owned(); + drop(args); // no longer needed Ok(Response::maybe_method_not_found(id.as_deref())) })) @@ -101,7 +100,7 @@ impl Service for Route { params = tracing::field::Empty, ); if enabled!(Level::TRACE) { - span.record("params", args.req.params()); + span.record("params", args.req().params()); } self.oneshot_inner(args) } From 3a27abe3eb76be29443659b42b2a328fcb5c39a0 Mon Sep 17 00:00:00 2001 From: James Date: Fri, 3 Oct 2025 14:36:16 -0400 Subject: [PATCH 05/24] feat: comply with error_code requirement --- src/routes/handler.rs | 95 +++++++++++++++++++++++-------------------- src/types/resp.rs | 5 +++ 2 files changed, 57 insertions(+), 43 deletions(-) diff --git a/src/routes/handler.rs b/src/routes/handler.rs index 1761177..a370f01 100644 --- a/src/routes/handler.rs +++ b/src/routes/handler.rs @@ -457,8 +457,7 @@ where Box::pin(async move { let payload = self().await; - - Response::maybe(id.as_deref(), &payload) + Response::maybe(args.span(), id.as_deref(), &payload) }) } } @@ -475,10 +474,11 @@ where fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { let id = args.id_owned(); let (ctx, _) = args.into_parts(); + let span = ctx.span().clone(); Box::pin(async move { let payload = self(ctx).await; - Response::maybe(id.as_deref(), &payload) + Response::maybe(&span, id.as_deref(), &payload) }) } } @@ -495,7 +495,8 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { - let (_, req) = args.into_parts(); + let (ctx, req) = args.into_parts(); + let span = ctx.span().clone(); Box::pin(async move { let id = req.id_owned(); let Ok(params) = req.deser_params() else { @@ -503,7 +504,7 @@ where }; let payload = self(params).await; - Response::maybe(id.as_deref(), &payload) + Response::maybe(&span, id.as_deref(), &payload) }) } } @@ -519,7 +520,8 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { - let (_, req) = args.into_parts(); + let (ctx, req) = args.into_parts(); + let span = ctx.span().clone(); Box::pin(async move { let id = req.id_owned(); let Ok(params) = req.deser_params() else { @@ -527,7 +529,7 @@ where }; let payload = self(Params(params)).await; - Response::maybe(id.as_deref(), &payload) + Response::maybe(&span, id.as_deref(), &payload) }) } } @@ -546,7 +548,7 @@ where let id = args.id_owned(); Box::pin(async move { let payload = self(state).await; - Response::maybe(id.as_deref(), &payload) + Response::maybe(args.span(), id.as_deref(), &payload) }) } } @@ -565,7 +567,7 @@ where let id = args.id_owned(); Box::pin(async move { let payload = self(State(state)).await; - Response::maybe(id.as_deref(), &payload) + Response::maybe(args.span(), id.as_deref(), &payload) }) } } @@ -584,6 +586,7 @@ where fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { Box::pin(async move { let (ctx, req) = args.into_parts(); + let span = ctx.span().clone(); let id = req.id_owned(); let Ok(params) = req.deser_params() else { @@ -593,7 +596,7 @@ where drop(req); // deallocate explicitly. No funny business. let payload = self(ctx, params).await; - Response::maybe(id.as_deref(), &payload) + Response::maybe(&span, id.as_deref(), &payload) }) } } @@ -612,6 +615,7 @@ where fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { Box::pin(async move { let (ctx, req) = args.into_parts(); + let span = ctx.span().clone(); let id = req.id_owned(); let Ok(params) = req.deser_params() else { @@ -621,7 +625,7 @@ where drop(req); // deallocate explicitly. No funny business. let payload = self(ctx, Params(params)).await; - Response::maybe(id.as_deref(), &payload) + Response::maybe(&span, id.as_deref(), &payload) }) } } @@ -639,7 +643,8 @@ where fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { Box::pin(async move { - let (_, req) = args.into_parts(); + let (ctx, req) = args.into_parts(); + let span = ctx.span().clone(); let id = req.id_owned(); let Ok(params) = req.deser_params() else { @@ -649,7 +654,7 @@ where let payload = self(params, state).await; - Response::maybe(id.as_deref(), &payload) + Response::maybe(&span, id.as_deref(), &payload) }) } } @@ -668,14 +673,14 @@ where fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { Box::pin(async move { let (ctx, req) = args.into_parts(); - + let span = ctx.span().clone(); let id = req.id_owned(); drop(req); // deallocate explicitly. No funny business. let payload = self(ctx, state).await; - Response::maybe(id.as_deref(), &payload) + Response::maybe(&span, id.as_deref(), &payload) }) } } @@ -693,14 +698,14 @@ where fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { Box::pin(async move { let (ctx, req) = args.into_parts(); - + let span = ctx.span().clone(); let id = req.id_owned(); drop(req); // deallocate explicitly. No funny business. let payload = self(ctx, State(state)).await; - Response::maybe(id.as_deref(), &payload) + Response::maybe(&span, id.as_deref(), &payload) }) } } @@ -720,8 +725,9 @@ where fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { Box::pin(async move { let (ctx, req) = args.into_parts(); - + let span = ctx.span().clone(); let id = req.id_owned(); + let Ok(params) = req.deser_params() else { return Response::maybe_invalid_params(id.as_deref()); }; @@ -729,7 +735,7 @@ where drop(req); // deallocate explicitly. No funny business. let payload = self(ctx, params, state).await; - Response::maybe(id.as_deref(), &payload) + Response::maybe(&span, id.as_deref(), &payload) }) } } @@ -745,10 +751,11 @@ where fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { let id = args.id_owned(); + let span = args.span().clone(); drop(args); Box::pin(async move { let payload = self().await; - Response::maybe(id.as_deref(), &convert_result!(payload)) + Response::maybe(&span, id.as_deref(), &convert_result!(payload)) }) } } @@ -764,14 +771,14 @@ where fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { let (ctx, req) = args.into_parts(); - + let span = ctx.span().clone(); let id = req.id_owned(); drop(req); Box::pin(async move { let payload = convert_result!(self(ctx).await); - Response::maybe(id.as_deref(), &payload) + Response::maybe(&span, id.as_deref(), &payload) }) } } @@ -788,8 +795,8 @@ where fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { Box::pin(async move { - let (_, req) = args.into_parts(); - + let (ctx, req) = args.into_parts(); + let span = ctx.span().clone(); let id = req.id_owned(); let Ok(params) = req.deser_params() else { return Response::maybe_invalid_params(id.as_deref()); @@ -799,7 +806,7 @@ where let payload = convert_result!(self(params).await); - Response::maybe(id.as_deref(), &payload) + Response::maybe(&span, id.as_deref(), &payload) }) } } @@ -816,7 +823,8 @@ where fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { Box::pin(async move { - let (_, req) = args.into_parts(); + let (ctx, req) = args.into_parts(); + let span = ctx.span().clone(); let id = req.id_owned(); let Ok(params) = req.deser_params() else { @@ -827,7 +835,7 @@ where let payload = convert_result!(self(Params(params)).await); - Response::maybe(id.as_deref(), &payload) + Response::maybe(&span, id.as_deref(), &payload) }) } } @@ -846,7 +854,7 @@ where let id = args.id_owned(); Box::pin(async move { let payload = convert_result!(self(state).await); - Response::maybe(id.as_deref(), &payload) + Response::maybe(args.span(), id.as_deref(), &payload) }) } } @@ -865,7 +873,7 @@ where let id = args.id_owned(); Box::pin(async move { let payload = convert_result!(self(State(state)).await); - Response::maybe(id.as_deref(), &payload) + Response::maybe(args.span(), id.as_deref(), &payload) }) } } @@ -884,7 +892,7 @@ where fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { Box::pin(async move { let (ctx, req) = args.into_parts(); - + let span = ctx.span().clone(); let id = req.id_owned(); let Ok(params) = req.deser_params() else { return Response::maybe_invalid_params(id.as_deref()); @@ -894,7 +902,7 @@ where let payload = convert_result!(self(ctx, params).await); - Response::maybe(id.as_deref(), &payload) + Response::maybe(&span, id.as_deref(), &payload) }) } } @@ -912,7 +920,7 @@ where fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { Box::pin(async move { let (ctx, req) = args.into_parts(); - + let span = ctx.span().clone(); let id = req.id_owned(); let Ok(params) = req.deser_params() else { return Response::maybe_invalid_params(id.as_deref()); @@ -922,7 +930,7 @@ where let payload = convert_result!(self(ctx, Params(params)).await); - Response::maybe(id.as_deref(), &payload) + Response::maybe(&span, id.as_deref(), &payload) }) } } @@ -940,7 +948,8 @@ where fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { Box::pin(async move { - let (_, req) = args.into_parts(); + let (ctx, req) = args.into_parts(); + let span = ctx.span().clone(); let id = req.id_owned(); let Ok(params) = req.deser_params() else { @@ -951,7 +960,7 @@ where let payload = convert_result!(self(params, state).await); - Response::maybe(id.as_deref(), &payload) + Response::maybe(&span, id.as_deref(), &payload) }) } } @@ -968,15 +977,15 @@ where fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { let (ctx, req) = args.into_parts(); - + let span = ctx.span().clone(); let id = req.id_owned(); - drop(req); + drop(req); // deallocate explicitly. No funny business. Box::pin(async move { let payload = convert_result!(self(ctx, state).await); - Response::maybe(id.as_deref(), &payload) + Response::maybe(&span, id.as_deref(), &payload) }) } } @@ -993,15 +1002,15 @@ where fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { let (ctx, req) = args.into_parts(); - + let span = ctx.span().clone(); let id = req.id_owned(); - drop(req); + drop(req); // deallocate explicitly. No funny business. Box::pin(async move { let payload = convert_result!(self(ctx, State(state)).await); - Response::maybe(id.as_deref(), &payload) + Response::maybe(&span, id.as_deref(), &payload) }) } } @@ -1020,7 +1029,7 @@ where fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { Box::pin(async move { let (ctx, req) = args.into_parts(); - + let span = ctx.span().clone(); let id = req.id_owned(); let Ok(params) = req.deser_params() else { return Response::maybe_invalid_params(id.as_deref()); @@ -1030,7 +1039,7 @@ where let payload = convert_result!(self(ctx, params, state).await); - Response::maybe(id.as_deref(), &payload) + Response::maybe(&span, id.as_deref(), &payload) }) } } diff --git a/src/types/resp.rs b/src/types/resp.rs index 3b2fe4e..a021a62 100644 --- a/src/types/resp.rs +++ b/src/types/resp.rs @@ -73,9 +73,14 @@ where E: Serialize, { pub(crate) fn maybe( + span: &tracing::Span, id: Option<&'b RawValue>, payload: &'a ResponsePayload, ) -> Option> { + if let Some(err_code) = payload.as_error().map(|e| e.code) { + span.record("rpc.jsonrpc.error_code", err_code); + } + id.map(|id| Self { id, payload }.to_json()) } From e53097d110351efbe17bc0b0c7a607902a434555 Mon Sep 17 00:00:00 2001 From: James Date: Fri, 3 Oct 2025 14:45:09 -0400 Subject: [PATCH 06/24] feat: comply with request_id on server span --- src/axum.rs | 13 +++++++++---- src/pubsub/shared.rs | 14 ++++++++++---- src/routes/ctx.rs | 1 + 3 files changed, 20 insertions(+), 8 deletions(-) diff --git a/src/axum.rs b/src/axum.rs index 4fd1db1..ed1a834 100644 --- a/src/axum.rs +++ b/src/axum.rs @@ -66,6 +66,9 @@ where S: Clone + Send + Sync + 'static, { fn ctx(&self) -> HandlerCtx { + // This span is populated with as much detail as possible, and then + // given to the Handler ctx. It will be populated with request-specific + // details (e.g. method) during ctx instantiation. let request_span = debug_span!( "ajj.IntoAxum::call", "otel.kind" = "server", @@ -103,11 +106,12 @@ where // https://github.com/open-telemetry/semantic-conventions/blob/main/docs/rpc/rpc-spans.md#message-event let req = ctx.span().in_scope(|| { + //// https://github.com/open-telemetry/semantic-conventions/blob/d66109ff41e75f49587114e5bff9d101b87f40bd/docs/rpc/rpc-spans.md#events debug!( "rpc.message.id" = self.rx_msg_id.fetch_add(1, Ordering::Relaxed), - "rpc.message.type" = "received", + "rpc.message.type" = "RECEIVED", "rpc.message.uncompressed_size" = bytes.len(), - "Received request" + "rpc.message" ); // If the inbound data is not currently parsable, we @@ -126,10 +130,11 @@ where let body = Box::::from(response); span.in_scope(|| { + // https://github.com/open-telemetry/semantic-conventions/blob/d66109ff41e75f49587114e5bff9d101b87f40bd/docs/rpc/rpc-spans.md#events debug!( "rpc.message.id" = self.tx_msg_id.fetch_add(1, Ordering::Relaxed), - "rpc.message.type" = "received", - "Received request" + "rpc.message.type" = "SENT", + "rpc.message.uncompressed_size" = body.len(), ); }); diff --git a/src/pubsub/shared.rs b/src/pubsub/shared.rs index 5140b1d..d5c2ccf 100644 --- a/src/pubsub/shared.rs +++ b/src/pubsub/shared.rs @@ -254,6 +254,10 @@ where break; }; + // This span is populated with as much detail as + // possible, and then given to the Handler ctx. It + // will be populated with request-specific details + // (e.g. method) during ctx instantiation. let tracing = TracingInfo { service: router.service_name(), request_span: debug_span!( parent: None, "ajj.pubsub.RouteTask::call", @@ -275,11 +279,12 @@ where let span = ctx.span().clone(); span.in_scope(|| { + // https://github.com/open-telemetry/semantic-conventions/blob/d66109ff41e75f49587114e5bff9d101b87f40bd/docs/rpc/rpc-spans.md#events debug!( "rpc.message.id" = rx_msg_id.fetch_add(1, Ordering::Relaxed), - "rpc.message.type" = "received", + "rpc.message.type" = "RECEIVED", "rpc.message.uncompressed_size" = item_bytes, - "Received request" + "rpc.message" ); }); @@ -374,10 +379,11 @@ impl WriteTask { break; }; span.in_scope(|| { + // https://github.com/open-telemetry/semantic-conventions/blob/d66109ff41e75f49587114e5bff9d101b87f40bd/docs/rpc/rpc-spans.md#events debug!( "rpc.message.id" = tx_msg_id.fetch_add(1, Ordering::Relaxed), - "rpc.message.type" = "sent", - "Sending response" + "rpc.message.type" = "SENT", + "rpc.message" ); }); diff --git a/src/routes/ctx.rs b/src/routes/ctx.rs index c2d591a..9b0b4f6 100644 --- a/src/routes/ctx.rs +++ b/src/routes/ctx.rs @@ -279,6 +279,7 @@ impl HandlerArgs { let span = this.span(); span.record("otel.name", this.otel_span_name()); span.record("rpc.method", this.req.method()); + span.record("rpc.jsonrpc.request_id", this.req.id()); if enabled!(Level::TRACE) { span.record("params", this.req.params()); } From a3a622e683aee99995d1d3825baf9b2c97242baa Mon Sep 17 00:00:00 2001 From: James Date: Fri, 3 Oct 2025 14:51:21 -0400 Subject: [PATCH 07/24] chore: readme update --- README.md | 14 ++++++++++++++ 1 file changed, 14 insertions(+) diff --git a/README.md b/README.md index dfaa536..553deff 100644 --- a/README.md +++ b/README.md @@ -71,6 +71,17 @@ implementations. See the [crate documentation on docs.rs] for more detailed examples. +## Specification Complinace + +`ajj` aims to be fully compliant with the [JSON-RPC 2.0] specification. If any +issues are found, please [open an issue]! + +`ajj` produces [`tracing`] spans that meet the [OpenTelemetry semantic +conventions] for JSON-RPC servers with the following exception: + +- The `server.address` attribute is NOT set, as the server address is not always + known to the ajj system. + ## Note on code provenance Some code in this project has been reproduced or adapted from other projects. @@ -94,3 +105,6 @@ reproduced from the following projects, and we are grateful for their work: [`interprocess::local_socket::ListenerOptions`]: https://docs.rs/interprocess/latest/interprocess/local_socket/struct.ListenerOptions.html [std::net::SocketAddr]: https://doc.rust-lang.org/std/net/enum.SocketAddr.html [alloy]: https://docs.rs/alloy/latest/alloy/ +[open an issue]: https://github.com/init4tech/ajj/issues/new +[OpenTelemetry semantic conventions]: https://opentelemetry.io/docs/specs/semconv/rpc/json-rpc/ +[`tracing`]: https://docs.rs/tracing/latest/tracing/ From 18bc4735dbac30b3faf9d4f29452f65ab329ad3d Mon Sep 17 00:00:00 2001 From: James Date: Fri, 3 Oct 2025 14:53:54 -0400 Subject: [PATCH 08/24] fix: add the error_message prop as well --- README.md | 5 +++-- src/types/resp.rs | 5 +++-- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index 553deff..d6662e2 100644 --- a/README.md +++ b/README.md @@ -76,11 +76,12 @@ See the [crate documentation on docs.rs] for more detailed examples. `ajj` aims to be fully compliant with the [JSON-RPC 2.0] specification. If any issues are found, please [open an issue]! -`ajj` produces [`tracing`] spans that meet the [OpenTelemetry semantic -conventions] for JSON-RPC servers with the following exception: +`ajj` produces [`tracing`] spans and events that meet the [OpenTelemetry +semantic conventions] for JSON-RPC servers with the following exception: - The `server.address` attribute is NOT set, as the server address is not always known to the ajj system. +- ## Note on code provenance diff --git a/src/types/resp.rs b/src/types/resp.rs index a021a62..4ac36fd 100644 --- a/src/types/resp.rs +++ b/src/types/resp.rs @@ -77,8 +77,9 @@ where id: Option<&'b RawValue>, payload: &'a ResponsePayload, ) -> Option> { - if let Some(err_code) = payload.as_error().map(|e| e.code) { - span.record("rpc.jsonrpc.error_code", err_code); + if let Some(e) = payload.as_error() { + span.record("rpc.jsonrpc.error_code", e.code); + span.record("rpc.jsonrpc.error_message", &e.message.as_ref()); } id.map(|id| Self { id, payload }.to_json()) From 6e9afdb5972af85d98e2b375e670f43e6d3b33b6 Mon Sep 17 00:00:00 2001 From: James Date: Fri, 3 Oct 2025 15:05:27 -0400 Subject: [PATCH 09/24] lint: clippy --- src/types/resp.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/types/resp.rs b/src/types/resp.rs index 4ac36fd..4eadfa8 100644 --- a/src/types/resp.rs +++ b/src/types/resp.rs @@ -79,7 +79,7 @@ where ) -> Option> { if let Some(e) = payload.as_error() { span.record("rpc.jsonrpc.error_code", e.code); - span.record("rpc.jsonrpc.error_message", &e.message.as_ref()); + span.record("rpc.jsonrpc.error_message", e.message.as_ref()); } id.map(|id| Self { id, payload }.to_json()) From 5e9d055d06eebe189ac7665f6fc9cb424dbacd45 Mon Sep 17 00:00:00 2001 From: James Date: Mon, 6 Oct 2025 09:52:21 -0400 Subject: [PATCH 10/24] nit: formatting --- src/axum.rs | 7 +++++++ src/pubsub/shared.rs | 5 +++++ 2 files changed, 12 insertions(+) diff --git a/src/axum.rs b/src/axum.rs index ed1a834..5bd7385 100644 --- a/src/axum.rs +++ b/src/axum.rs @@ -70,12 +70,19 @@ where // given to the Handler ctx. It will be populated with request-specific // details (e.g. method) during ctx instantiation. let request_span = debug_span!( + // We could erase the parent here, however, axum or tower layers + // may be creating per-request spans that we want to be children of. "ajj.IntoAxum::call", "otel.kind" = "server", "rpc.system" = "jsonrpc", "rpc.jsonrpc.version" = "2.0", "rpc.service" = self.router.service_name(), notifications_enabled = false, + "otel.name" = tracing::field::Empty, + "rpc.jsonrpc.request_id" = tracing::field::Empty, + "rpc.jsonrpc.error_code" = tracing::field::Empty, + "rpc.jsonrpc.error_message" = tracing::field::Empty, + "rpc.method" = tracing::field::Empty, params = tracing::field::Empty ); diff --git a/src/pubsub/shared.rs b/src/pubsub/shared.rs index d5c2ccf..1530883 100644 --- a/src/pubsub/shared.rs +++ b/src/pubsub/shared.rs @@ -267,6 +267,11 @@ where "rpc.service" = router.service_name(), conn_id = self.conn_id, notifications_enabled = true, + "otel.name" = tracing::field::Empty, + "rpc.jsonrpc.request_id" = tracing::field::Empty, + "rpc.jsonrpc.error_code" = tracing::field::Empty, + "rpc.jsonrpc.error_message" = tracing::field::Empty, + "rpc.method" = tracing::field::Empty, params = tracing::field::Empty ) }; From ade71a0ec39d56ea7b15bcbdf1c4f2cb65f7341a Mon Sep 17 00:00:00 2001 From: James Date: Mon, 6 Oct 2025 10:22:53 -0400 Subject: [PATCH 11/24] feat: use traceparent via otel http header extractor --- Cargo.toml | 5 ++++- src/axum.rs | 6 ++++++ 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 7de86da..2073eb7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -15,6 +15,7 @@ repository = "https://github.com/init4tech/ajj" [dependencies] bytes = "1.9.0" +opentelemetry = "0.31.0" pin-project = "1.1.8" serde = { version = "1.0.217", features = ["derive"] } serde_json = { version = "1.0.135", features = ["raw_value"] } @@ -23,10 +24,12 @@ tokio = { version = "1.43.0", features = ["sync", "rt", "macros"] } tokio-util = { version = "0.7.13", features = ["io", "rt"] } tower = { version = "0.5.2", features = ["util"] } tracing = "0.1.41" +tracing-opentelemetry = "0.32.0" # axum axum = { version = "0.8.1", optional = true } mime = { version = "0.3.17", optional = true } +opentelemetry-http = { version = "0.31.0", optional = true } # pubsub tokio-stream = { version = "0.1.17", optional = true } @@ -51,7 +54,7 @@ eyre = "0.6.12" [features] default = ["axum", "ws", "ipc"] -axum = ["dep:axum", "dep:mime"] +axum = ["dep:axum", "dep:mime", "dep:opentelemetry-http"] pubsub = ["dep:tokio-stream", "axum?/ws"] ipc = ["pubsub", "dep:interprocess"] ws = ["pubsub", "dep:tokio-tungstenite", "dep:futures-util"] diff --git a/src/axum.rs b/src/axum.rs index 5bd7385..99c8448 100644 --- a/src/axum.rs +++ b/src/axum.rs @@ -18,6 +18,7 @@ use std::{ }; use tokio::runtime::Handle; use tracing::{debug, debug_span}; +use tracing_opentelemetry::OpenTelemetrySpanExt; /// A wrapper around an [`Router`] that implements the /// [`axum::handler::Handler`] trait. This struct is an implementation detail @@ -107,6 +108,11 @@ where Box::pin(async move { let ctx = self.ctx(); + let parent_context = opentelemetry::global::get_text_map_propagator(|propagator| { + propagator.extract(&opentelemetry_http::HeaderExtractor(req.headers())) + }); + ctx.span().set_parent(parent_context).unwrap(); + let Ok(bytes) = Bytes::from_request(req, &state).await else { return Box::::from(Response::parse_error()).into_response(); }; From 7620c7c9fcb25e04a91e60153ebf58de22079471 Mon Sep 17 00:00:00 2001 From: James Date: Mon, 6 Oct 2025 10:27:38 -0400 Subject: [PATCH 12/24] feat: set otel status during response construction --- src/types/resp.rs | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) diff --git a/src/types/resp.rs b/src/types/resp.rs index 4eadfa8..acf0e77 100644 --- a/src/types/resp.rs +++ b/src/types/resp.rs @@ -1,8 +1,10 @@ use crate::RpcSend; +use opentelemetry::trace::Status; use serde::{ser::SerializeMap, Serialize, Serializer}; use serde_json::value::{to_raw_value, RawValue}; use std::borrow::Cow; use std::fmt; +use tracing_opentelemetry::OpenTelemetrySpanExt; const INTERNAL_ERROR: Cow<'_, str> = Cow::Borrowed("Internal error"); @@ -80,9 +82,12 @@ where if let Some(e) = payload.as_error() { span.record("rpc.jsonrpc.error_code", e.code); span.record("rpc.jsonrpc.error_message", e.message.as_ref()); + span.set_status(Status::Error { + description: e.message.clone(), + }); } - id.map(|id| Self { id, payload }.to_json()) + id.map(move |id| Self { id, payload }.to_json()) } pub(crate) fn to_json(&self) -> Box { From 714832dce08c17147f4ec40da0affaef7ee1867d Mon Sep 17 00:00:00 2001 From: James Date: Mon, 6 Oct 2025 10:41:16 -0400 Subject: [PATCH 13/24] refactor: DRY with macros --- src/axum.rs | 41 +++++++---------------- src/macros.rs | 78 ++++++++++++++++++++++++++++++++++++++++++++ src/pubsub/shared.rs | 38 ++++++--------------- 3 files changed, 100 insertions(+), 57 deletions(-) diff --git a/src/axum.rs b/src/axum.rs index 99c8448..e635d09 100644 --- a/src/axum.rs +++ b/src/axum.rs @@ -11,13 +11,9 @@ use bytes::Bytes; use std::{ future::Future, pin::Pin, - sync::{ - atomic::{AtomicU32, Ordering}, - Arc, - }, + sync::{atomic::AtomicU32, Arc}, }; use tokio::runtime::Handle; -use tracing::{debug, debug_span}; use tracing_opentelemetry::OpenTelemetrySpanExt; /// A wrapper around an [`Router`] that implements the @@ -70,21 +66,11 @@ where // This span is populated with as much detail as possible, and then // given to the Handler ctx. It will be populated with request-specific // details (e.g. method) during ctx instantiation. - let request_span = debug_span!( + let request_span = request_span!( // We could erase the parent here, however, axum or tower layers // may be creating per-request spans that we want to be children of. - "ajj.IntoAxum::call", - "otel.kind" = "server", - "rpc.system" = "jsonrpc", - "rpc.jsonrpc.version" = "2.0", - "rpc.service" = self.router.service_name(), - notifications_enabled = false, - "otel.name" = tracing::field::Empty, - "rpc.jsonrpc.request_id" = tracing::field::Empty, - "rpc.jsonrpc.error_code" = tracing::field::Empty, - "rpc.jsonrpc.error_message" = tracing::field::Empty, - "rpc.method" = tracing::field::Empty, - params = tracing::field::Empty + name: "ajj.IntoAxum::call", + router: &self.router, ); HandlerCtx::new( @@ -119,12 +105,10 @@ where // https://github.com/open-telemetry/semantic-conventions/blob/main/docs/rpc/rpc-spans.md#message-event let req = ctx.span().in_scope(|| { - //// https://github.com/open-telemetry/semantic-conventions/blob/d66109ff41e75f49587114e5bff9d101b87f40bd/docs/rpc/rpc-spans.md#events - debug!( - "rpc.message.id" = self.rx_msg_id.fetch_add(1, Ordering::Relaxed), - "rpc.message.type" = "RECEIVED", - "rpc.message.uncompressed_size" = bytes.len(), - "rpc.message" + message_event!( + @received, + counter: &self.rx_msg_id, + bytes: bytes.len(), ); // If the inbound data is not currently parsable, we @@ -143,11 +127,10 @@ where let body = Box::::from(response); span.in_scope(|| { - // https://github.com/open-telemetry/semantic-conventions/blob/d66109ff41e75f49587114e5bff9d101b87f40bd/docs/rpc/rpc-spans.md#events - debug!( - "rpc.message.id" = self.tx_msg_id.fetch_add(1, Ordering::Relaxed), - "rpc.message.type" = "SENT", - "rpc.message.uncompressed_size" = body.len(), + message_event!( + @sent, + counter: &self.tx_msg_id, + bytes: body.len(), ); }); diff --git a/src/macros.rs b/src/macros.rs index 105eaa4..d18bfd8 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -54,6 +54,84 @@ macro_rules! unwrap_infallible { }; } +/// Set up an initial tracing span for a request handler. +macro_rules! request_span { + (@noparent, $name:literal, $router:expr, notifications: $notifications:literal,) => { + ::tracing::debug_span!( + parent: None, + $name, + "otel.kind" = "server", + "rpc.system" = "jsonrpc", + "rpc.jsonrpc.version" = "2.0", + "rpc.service" = $router.service_name(), + notifications_enabled = $notifications, + "otel.name" = ::tracing::field::Empty, + "rpc.jsonrpc.request_id" = ::tracing::field::Empty, + "rpc.jsonrpc.error_code" = ::tracing::field::Empty, + "rpc.jsonrpc.error_message" = ::tracing::field::Empty, + "rpc.method" = ::tracing::field::Empty, + params = ::tracing::field::Empty, + ) + }; + + (parent: $parent:expr, $name:literal, $router:expr, notifications: $notifications:literal,) => { + ::tracing::debug_span!( + parent: $parent, + $name, + "otel.kind" = "server", + "rpc.system" = "jsonrpc", + "rpc.jsonrpc.version" = "2.0", + "rpc.service" = $router.service_name(), + notifications_enabled = $notifications, + "otel.name" = ::tracing::field::Empty, + "rpc.jsonrpc.request_id" = ::tracing::field::Empty, + "rpc.jsonrpc.error_code" = ::tracing::field::Empty, + "rpc.jsonrpc.error_message" = ::tracing::field::Empty, + "rpc.method" = ::tracing::field::Empty, + params = ::tracing::field::Empty, + ) + }; + + + (parent: $parent:expr, name: $name:literal, router: $router:expr,) => { + request_span!($parent, $name, $router, notifications: false,) + }; + + (parent: $parent:expr, name: $name:literal, router: $router:expr, with_notifications) => { + request_span!(parent: $parent, $name, $router, notifications: true,) + }; + + (name: $name:literal, router: $router:expr,) => { + request_span!(@noparent, $name, $router, notifications: false,) + }; + + (name: $name:literal, router: $router:expr, with_notifications) => { + request_span!(@noparent, $name, $router, notifications: true,) + }; +} + +/// Log a message event to the current span. +/// +/// See +macro_rules! message_event { + ($type:literal, counter: $counter:expr, bytes: $bytes:expr,) => {{ + ::tracing::debug!( + "rpc.message.id" = $counter.fetch_add(1, ::std::sync::atomic::Ordering::Relaxed), + "rpc.message.type" = $type, + "rpc.message.uncompressed_size" = $bytes, + "rpc.message" + ); + }}; + + (@received, counter: $counter:expr, bytes: $bytes:expr, ) => { + message_event!("RECEIVED", counter: $counter, bytes: $bytes,); + }; + + (@sent, counter: $counter:expr, bytes: $bytes:expr, ) => { + message_event!("SENT", counter: $counter, bytes: $bytes,); + }; +} + // Some code is this file is reproduced under the terms of the MIT license. It // originates from the `axum` crate. The original source code can be found at // the following URL, and the original license is included below. diff --git a/src/pubsub/shared.rs b/src/pubsub/shared.rs index 1530883..3b0278f 100644 --- a/src/pubsub/shared.rs +++ b/src/pubsub/shared.rs @@ -12,7 +12,7 @@ use std::sync::{ use tokio::{pin, runtime::Handle, select, sync::mpsc, task::JoinHandle}; use tokio_stream::StreamExt; use tokio_util::sync::WaitForCancellationFutureOwned; -use tracing::{debug, debug_span, error, trace, Instrument}; +use tracing::{debug, error, trace, Instrument}; /// Default notification buffer size per task. pub const DEFAULT_NOTIFICATION_BUFFER_PER_CLIENT: usize = 16; @@ -258,22 +258,7 @@ where // possible, and then given to the Handler ctx. It // will be populated with request-specific details // (e.g. method) during ctx instantiation. - let tracing = TracingInfo { service: router.service_name(), request_span: debug_span!( - parent: None, - "ajj.pubsub.RouteTask::call", - "otel.kind" = "server", - "rpc.system" = "jsonrpc", - "rpc.jsonrpc.version" = "2.0", - "rpc.service" = router.service_name(), - conn_id = self.conn_id, - notifications_enabled = true, - "otel.name" = tracing::field::Empty, - "rpc.jsonrpc.request_id" = tracing::field::Empty, - "rpc.jsonrpc.error_code" = tracing::field::Empty, - "rpc.jsonrpc.error_message" = tracing::field::Empty, - "rpc.method" = tracing::field::Empty, - params = tracing::field::Empty - ) }; + let tracing = TracingInfo { service: router.service_name(), request_span: request_span!(name: "ajj.pubsub.RouteTask::call", router: &router, with_notifications) }; let ctx = HandlerCtx::new( @@ -284,12 +269,10 @@ where let span = ctx.span().clone(); span.in_scope(|| { - // https://github.com/open-telemetry/semantic-conventions/blob/d66109ff41e75f49587114e5bff9d101b87f40bd/docs/rpc/rpc-spans.md#events - debug!( - "rpc.message.id" = rx_msg_id.fetch_add(1, Ordering::Relaxed), - "rpc.message.type" = "RECEIVED", - "rpc.message.uncompressed_size" = item_bytes, - "rpc.message" + message_event!( + @received, + counter: &rx_msg_id, + bytes: item_bytes, ); }); @@ -384,11 +367,10 @@ impl WriteTask { break; }; span.in_scope(|| { - // https://github.com/open-telemetry/semantic-conventions/blob/d66109ff41e75f49587114e5bff9d101b87f40bd/docs/rpc/rpc-spans.md#events - debug!( - "rpc.message.id" = tx_msg_id.fetch_add(1, Ordering::Relaxed), - "rpc.message.type" = "SENT", - "rpc.message" + message_event!( + @sent, + counter: &tx_msg_id, + bytes: json.get().len(), ); }); From 0458f0cde5336707d628cb4ce0b31d949020e659 Mon Sep 17 00:00:00 2001 From: James Date: Mon, 6 Oct 2025 11:03:09 -0400 Subject: [PATCH 14/24] refactor: improve code quality and DRY --- src/axum.rs | 28 ++++++++++++++++++++-------- src/macros.rs | 30 +++++++----------------------- src/pubsub/shared.rs | 2 +- src/types/batch.rs | 4 ++-- 4 files changed, 30 insertions(+), 34 deletions(-) diff --git a/src/axum.rs b/src/axum.rs index e635d09..b3cf84a 100644 --- a/src/axum.rs +++ b/src/axum.rs @@ -8,12 +8,14 @@ use axum::{ response::IntoResponse, }; use bytes::Bytes; +use opentelemetry::trace::TraceContextExt; use std::{ future::Future, pin::Pin, sync::{atomic::AtomicU32, Arc}, }; use tokio::runtime::Handle; +use tracing::Span; use tracing_opentelemetry::OpenTelemetrySpanExt; /// A wrapper around an [`Router`] that implements the @@ -62,17 +64,32 @@ impl IntoAxum where S: Clone + Send + Sync + 'static, { - fn ctx(&self) -> HandlerCtx { + fn ctx(&self, req: &axum::extract::Request) -> HandlerCtx { // This span is populated with as much detail as possible, and then // given to the Handler ctx. It will be populated with request-specific // details (e.g. method) during ctx instantiation. let request_span = request_span!( + parent: Span::current(), // We could erase the parent here, however, axum or tower layers // may be creating per-request spans that we want to be children of. - name: "ajj.IntoAxum::call", + name: "ajj.IntoAxum::request", router: &self.router, ); + let parent_context = opentelemetry::global::get_text_map_propagator(|propagator| { + propagator.extract(&opentelemetry_http::HeaderExtractor(req.headers())) + }); + request_span.set_parent(parent_context).unwrap(); + request_span.record( + "trace_id", + request_span + .context() + .span() + .span_context() + .trace_id() + .to_string(), + ); + HandlerCtx::new( None, self.task_set.clone(), @@ -92,12 +109,7 @@ where fn call(self, req: axum::extract::Request, state: S) -> Self::Future { Box::pin(async move { - let ctx = self.ctx(); - - let parent_context = opentelemetry::global::get_text_map_propagator(|propagator| { - propagator.extract(&opentelemetry_http::HeaderExtractor(req.headers())) - }); - ctx.span().set_parent(parent_context).unwrap(); + let ctx = self.ctx(&req); let Ok(bytes) = Bytes::from_request(req, &state).await else { return Box::::from(Response::parse_error()).into_response(); diff --git a/src/macros.rs b/src/macros.rs index d18bfd8..a436337 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -56,24 +56,6 @@ macro_rules! unwrap_infallible { /// Set up an initial tracing span for a request handler. macro_rules! request_span { - (@noparent, $name:literal, $router:expr, notifications: $notifications:literal,) => { - ::tracing::debug_span!( - parent: None, - $name, - "otel.kind" = "server", - "rpc.system" = "jsonrpc", - "rpc.jsonrpc.version" = "2.0", - "rpc.service" = $router.service_name(), - notifications_enabled = $notifications, - "otel.name" = ::tracing::field::Empty, - "rpc.jsonrpc.request_id" = ::tracing::field::Empty, - "rpc.jsonrpc.error_code" = ::tracing::field::Empty, - "rpc.jsonrpc.error_message" = ::tracing::field::Empty, - "rpc.method" = ::tracing::field::Empty, - params = ::tracing::field::Empty, - ) - }; - (parent: $parent:expr, $name:literal, $router:expr, notifications: $notifications:literal,) => { ::tracing::debug_span!( parent: $parent, @@ -83,7 +65,9 @@ macro_rules! request_span { "rpc.jsonrpc.version" = "2.0", "rpc.service" = $router.service_name(), notifications_enabled = $notifications, + "trace_id" = ::tracing::field::Empty, "otel.name" = ::tracing::field::Empty, + "otel.status" = ::tracing::field::Empty, "rpc.jsonrpc.request_id" = ::tracing::field::Empty, "rpc.jsonrpc.error_code" = ::tracing::field::Empty, "rpc.jsonrpc.error_message" = ::tracing::field::Empty, @@ -94,19 +78,19 @@ macro_rules! request_span { (parent: $parent:expr, name: $name:literal, router: $router:expr,) => { - request_span!($parent, $name, $router, notifications: false,) + request_span!(parent: $parent, $name, $router, notifications: false,) }; (parent: $parent:expr, name: $name:literal, router: $router:expr, with_notifications) => { request_span!(parent: $parent, $name, $router, notifications: true,) }; - (name: $name:literal, router: $router:expr,) => { - request_span!(@noparent, $name, $router, notifications: false,) + (@noparent, name: $name:literal, router: $router:expr,) => { + request_span!(parent: None,, $name, $router, notifications: false,) }; - (name: $name:literal, router: $router:expr, with_notifications) => { - request_span!(@noparent, $name, $router, notifications: true,) + (@noparent, name: $name:literal, router: $router:expr, with_notifications) => { + request_span!(parent: None, $name, $router, notifications: true,) }; } diff --git a/src/pubsub/shared.rs b/src/pubsub/shared.rs index 3b0278f..b9d5690 100644 --- a/src/pubsub/shared.rs +++ b/src/pubsub/shared.rs @@ -258,7 +258,7 @@ where // possible, and then given to the Handler ctx. It // will be populated with request-specific details // (e.g. method) during ctx instantiation. - let tracing = TracingInfo { service: router.service_name(), request_span: request_span!(name: "ajj.pubsub.RouteTask::call", router: &router, with_notifications) }; + let tracing = TracingInfo { service: router.service_name(), request_span: request_span!(@noparent, name: "ajj.pubsub.RouteTask::call", router: &router, with_notifications) }; let ctx = HandlerCtx::new( diff --git a/src/types/batch.rs b/src/types/batch.rs index 1b650ff..cdbabc1 100644 --- a/src/types/batch.rs +++ b/src/types/batch.rs @@ -3,7 +3,7 @@ use bytes::Bytes; use serde::Deserialize; use serde_json::value::RawValue; use std::ops::Range; -use tracing::{debug, enabled, instrument, Level}; +use tracing::{debug, enabled, instrument, span::Span, Level}; /// UTF-8, partially deserialized JSON-RPC request batch. #[derive(Default)] @@ -56,7 +56,7 @@ impl TryFrom for InboundData { #[instrument(level = "debug", skip(bytes), fields(buf_len = bytes.len(), bytes = tracing::field::Empty))] fn try_from(bytes: Bytes) -> Result { if enabled!(Level::TRACE) { - tracing::span::Span::current().record("bytes", format!("0x{:x}", bytes)); + Span::current().record("bytes", format!("0x{:x}", bytes)); } // This event exists only so that people who use default console From a0d7bd897f51117a11194777483c34153010991a Mon Sep 17 00:00:00 2001 From: James Date: Mon, 6 Oct 2025 11:53:42 -0400 Subject: [PATCH 15/24] feat: appropiately associate spans with names --- README.md | 1 - src/axum.rs | 38 +++---------- src/macros.rs | 42 +------------- src/pubsub/shared.rs | 3 +- src/router.rs | 19 ++++--- src/routes/ctx.rs | 128 +++++++++++++++++++++++++++++++++++++++---- 6 files changed, 142 insertions(+), 89 deletions(-) diff --git a/README.md b/README.md index d6662e2..ca0a743 100644 --- a/README.md +++ b/README.md @@ -81,7 +81,6 @@ semantic conventions] for JSON-RPC servers with the following exception: - The `server.address` attribute is NOT set, as the server address is not always known to the ajj system. -- ## Note on code provenance diff --git a/src/axum.rs b/src/axum.rs index b3cf84a..e8d9671 100644 --- a/src/axum.rs +++ b/src/axum.rs @@ -8,15 +8,13 @@ use axum::{ response::IntoResponse, }; use bytes::Bytes; -use opentelemetry::trace::TraceContextExt; use std::{ future::Future, pin::Pin, sync::{atomic::AtomicU32, Arc}, }; use tokio::runtime::Handle; -use tracing::Span; -use tracing_opentelemetry::OpenTelemetrySpanExt; +use tracing::{Instrument, Span}; /// A wrapper around an [`Router`] that implements the /// [`axum::handler::Handler`] trait. This struct is an implementation detail @@ -65,38 +63,14 @@ where S: Clone + Send + Sync + 'static, { fn ctx(&self, req: &axum::extract::Request) -> HandlerCtx { - // This span is populated with as much detail as possible, and then - // given to the Handler ctx. It will be populated with request-specific - // details (e.g. method) during ctx instantiation. - let request_span = request_span!( - parent: Span::current(), - // We could erase the parent here, however, axum or tower layers - // may be creating per-request spans that we want to be children of. - name: "ajj.IntoAxum::request", - router: &self.router, - ); - let parent_context = opentelemetry::global::get_text_map_propagator(|propagator| { propagator.extract(&opentelemetry_http::HeaderExtractor(req.headers())) }); - request_span.set_parent(parent_context).unwrap(); - request_span.record( - "trace_id", - request_span - .context() - .span() - .span_context() - .trace_id() - .to_string(), - ); HandlerCtx::new( None, self.task_set.clone(), - TracingInfo { - service: self.router.service_name(), - request_span, - }, + TracingInfo::new_with_context(self.router.service_name(), parent_context), ) } } @@ -110,6 +84,7 @@ where fn call(self, req: axum::extract::Request, state: S) -> Self::Future { Box::pin(async move { let ctx = self.ctx(&req); + ctx.init_request_span(&self.router, Some(&Span::current())); let Ok(bytes) = Bytes::from_request(req, &state).await else { return Box::::from(Response::parse_error()).into_response(); @@ -130,7 +105,12 @@ where }); let span = ctx.span().clone(); - if let Some(response) = self.router.call_batch_with_state(ctx, req, state).await { + if let Some(response) = self + .router + .call_batch_with_state(ctx, req, state) + .instrument(span.clone()) + .await + { let headers = [( header::CONTENT_TYPE, HeaderValue::from_static(mime::APPLICATION_JSON.as_ref()), diff --git a/src/macros.rs b/src/macros.rs index a436337..db9cbd6 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -54,52 +54,12 @@ macro_rules! unwrap_infallible { }; } -/// Set up an initial tracing span for a request handler. -macro_rules! request_span { - (parent: $parent:expr, $name:literal, $router:expr, notifications: $notifications:literal,) => { - ::tracing::debug_span!( - parent: $parent, - $name, - "otel.kind" = "server", - "rpc.system" = "jsonrpc", - "rpc.jsonrpc.version" = "2.0", - "rpc.service" = $router.service_name(), - notifications_enabled = $notifications, - "trace_id" = ::tracing::field::Empty, - "otel.name" = ::tracing::field::Empty, - "otel.status" = ::tracing::field::Empty, - "rpc.jsonrpc.request_id" = ::tracing::field::Empty, - "rpc.jsonrpc.error_code" = ::tracing::field::Empty, - "rpc.jsonrpc.error_message" = ::tracing::field::Empty, - "rpc.method" = ::tracing::field::Empty, - params = ::tracing::field::Empty, - ) - }; - - - (parent: $parent:expr, name: $name:literal, router: $router:expr,) => { - request_span!(parent: $parent, $name, $router, notifications: false,) - }; - - (parent: $parent:expr, name: $name:literal, router: $router:expr, with_notifications) => { - request_span!(parent: $parent, $name, $router, notifications: true,) - }; - - (@noparent, name: $name:literal, router: $router:expr,) => { - request_span!(parent: None,, $name, $router, notifications: false,) - }; - - (@noparent, name: $name:literal, router: $router:expr, with_notifications) => { - request_span!(parent: None, $name, $router, notifications: true,) - }; -} - /// Log a message event to the current span. /// /// See macro_rules! message_event { ($type:literal, counter: $counter:expr, bytes: $bytes:expr,) => {{ - ::tracing::debug!( + ::tracing::info!( "rpc.message.id" = $counter.fetch_add(1, ::std::sync::atomic::Ordering::Relaxed), "rpc.message.type" = $type, "rpc.message.uncompressed_size" = $bytes, diff --git a/src/pubsub/shared.rs b/src/pubsub/shared.rs index b9d5690..a1dcb3f 100644 --- a/src/pubsub/shared.rs +++ b/src/pubsub/shared.rs @@ -258,7 +258,7 @@ where // possible, and then given to the Handler ctx. It // will be populated with request-specific details // (e.g. method) during ctx instantiation. - let tracing = TracingInfo { service: router.service_name(), request_span: request_span!(@noparent, name: "ajj.pubsub.RouteTask::call", router: &router, with_notifications) }; + let tracing = TracingInfo::new(router.service_name()); let ctx = HandlerCtx::new( @@ -266,6 +266,7 @@ where children.clone(), tracing, ); + ctx.init_request_span(&router, None); let span = ctx.span().clone(); span.in_scope(|| { diff --git a/src/router.rs b/src/router.rs index 2846a7d..98e6593 100644 --- a/src/router.rs +++ b/src/router.rs @@ -333,20 +333,25 @@ where let mut fut = BatchFuture::new_with_capacity(inbound.single(), inbound.len()); // According to spec, non-parsable requests should still receive a // response. - let span = debug_span!(parent: ctx.span(), "BatchFuture::poll", reqs = inbound.len(), futs = tracing::field::Empty); + let batch_span = debug_span!(parent: ctx.span(), "BatchFuture::poll", reqs = inbound.len(), futs = tracing::field::Empty); - for (batch_idx, req) in inbound.iter().enumerate() { + for req in inbound.iter() { let req = req.map(|req| { - let span = debug_span!(parent: &span, "RouteFuture::poll", batch_idx, method = req.method(), id = ?req.id()); - let args = HandlerArgs::new(ctx.clone(), req); + // Cloning here resets the `TracingInfo`, which means each + // ctx has a separate span with similar metadata. + let ctx = ctx.clone(); + let request_span = ctx.init_request_span(&self, Some(&batch_span)).clone(); + + // Several span fields are populated in `HandlerArgs::new`. + let args = HandlerArgs::new(ctx, req); self.inner .call_with_state(args, state.clone()) - .with_span(span) + .with_span(request_span) }); fut.push_parse_result(req); } - span.record("futs", fut.len()); - fut.with_span(span) + batch_span.record("futs", fut.len()); + fut.with_span(batch_span) } /// Nest this router into a new Axum router, with the specified path and the currently-running diff --git a/src/routes/ctx.rs b/src/routes/ctx.rs index 9b0b4f6..2df9bcb 100644 --- a/src/routes/ctx.rs +++ b/src/routes/ctx.rs @@ -1,12 +1,15 @@ -use crate::{pubsub::WriteItem, types::Request, RpcSend, TaskSet}; +use crate::{pubsub::WriteItem, types::Request, Router, RpcSend, TaskSet}; +use ::tracing::info_span; +use opentelemetry::trace::TraceContextExt; use serde_json::value::RawValue; -use std::future::Future; +use std::{future::Future, sync::OnceLock}; use tokio::{ sync::mpsc::{self, error::SendError}, task::JoinHandle, }; use tokio_util::sync::WaitForCancellationFutureOwned; use tracing::{enabled, error, Level}; +use tracing_opentelemetry::OpenTelemetrySpanExt; /// Errors that can occur when sending notifications. #[derive(thiserror::Error, Debug)] @@ -27,24 +30,115 @@ impl From> for NotifyError { /// Tracing information for OpenTelemetry. This struct is used to store /// information about the current request that can be used for tracing. -#[derive(Debug, Clone)] +#[derive(Debug)] #[non_exhaustive] pub struct TracingInfo { /// The OpenTelemetry service name. pub service: &'static str, - /// The request span. - pub request_span: tracing::Span, + /// The open telemetry Context, + pub context: Option, + + /// The tracing span for this request. + span: OnceLock, +} + +impl Clone for TracingInfo { + fn clone(&self) -> Self { + Self { + service: self.service, + context: self.context.clone(), + span: OnceLock::new(), + } + } } impl TracingInfo { + /// Create a new tracing info with the given service name and no context. + #[allow(dead_code)] // used in some features + pub fn new(service: &'static str) -> Self { + Self { + service, + context: None, + span: OnceLock::new(), + } + } + + /// Create a new tracing info with the given service name and context. + pub fn new_with_context( + service: &'static str, + context: opentelemetry::context::Context, + ) -> Self { + Self { + service, + context: Some(context), + span: OnceLock::new(), + } + } + + /// Create a request span for a handler invocation. + fn init_request_span( + &self, + router: &Router, + with_notifications: bool, + parent: Option<&tracing::Span>, + ) -> &tracing::Span + where + S: Clone + Send + Sync + 'static, + { + // This span is populated with as much detail as possible, and then + // given to the Request. It will be populated with request-specific + // details (e.g. method) during request setup. + self.span.get_or_init(|| { + let span = info_span!( + parent: parent.and_then(|p| p.id()), + "AjjRequest", + "otel.kind" = "server", + "rpc.system" = "jsonrpc", + "rpc.jsonrpc.version" = "2.0", + "rpc.service" = router.service_name(), + notifications_enabled = with_notifications, + "trace_id" = ::tracing::field::Empty, + "otel.name" = ::tracing::field::Empty, + "otel.status_code" = ::tracing::field::Empty, + "rpc.jsonrpc.request_id" = ::tracing::field::Empty, + "rpc.jsonrpc.error_code" = ::tracing::field::Empty, + "rpc.jsonrpc.error_message" = ::tracing::field::Empty, + "rpc.method" = ::tracing::field::Empty, + params = ::tracing::field::Empty, + ); + if let Some(context) = &self.context { + let _ = span.set_parent(context.clone()); + + span.record( + "trace_id", + context.span().span_context().trace_id().to_string(), + ); + } + + span + }) + } + + /// Get a reference to the tracing span for this request. + /// + /// ## Panics + /// + /// Panics if the span has not been initialized via + /// [`Self::init_request_span`]. + #[track_caller] + fn request_span(&self) -> &tracing::Span { + self.span.get().expect("span not initialized") + } + /// Create a mock tracing info for testing. #[cfg(test)] pub fn mock() -> Self { - use tracing::debug_span; + use tracing::info_span; Self { service: "test", - request_span: debug_span!("test"), + context: None, + span: OnceLock::new(), } } } @@ -104,8 +198,9 @@ impl HandlerCtx { } /// Get a reference to the tracing span for this handler context. - pub const fn span(&self) -> &tracing::Span { - &self.tracing.request_span + #[track_caller] + pub fn span(&self) -> &tracing::Span { + &self.tracing.request_span() } /// Set the tracing information for this handler context. @@ -123,6 +218,19 @@ impl HandlerCtx { .unwrap_or_default() } + /// Create a request span for a handler invocation. + pub fn init_request_span( + &self, + router: &Router, + parent: Option<&tracing::Span>, + ) -> &tracing::Span + where + S: Clone + Send + Sync + 'static, + { + self.tracing_info() + .init_request_span(router, self.notifications_enabled(), parent) + } + /// Notify a client of an event. pub async fn notify(&self, t: &T) -> Result<(), NotifyError> { if let Some(notifications) = self.notifications.as_ref() { @@ -298,7 +406,7 @@ impl HandlerArgs { } /// Get a reference to the tracing span for this handler invocation. - pub const fn span(&self) -> &tracing::Span { + pub fn span(&self) -> &tracing::Span { self.ctx.span() } From 3e00517b9470f1ecc7fbc850e467303086815420 Mon Sep 17 00:00:00 2001 From: James Date: Mon, 6 Oct 2025 11:54:12 -0400 Subject: [PATCH 16/24] lint: clippy --- src/router.rs | 2 +- src/routes/ctx.rs | 10 +++++----- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/router.rs b/src/router.rs index 98e6593..3939b1a 100644 --- a/src/router.rs +++ b/src/router.rs @@ -340,7 +340,7 @@ where // Cloning here resets the `TracingInfo`, which means each // ctx has a separate span with similar metadata. let ctx = ctx.clone(); - let request_span = ctx.init_request_span(&self, Some(&batch_span)).clone(); + let request_span = ctx.init_request_span(self, Some(&batch_span)).clone(); // Several span fields are populated in `HandlerArgs::new`. let args = HandlerArgs::new(ctx, req); diff --git a/src/routes/ctx.rs b/src/routes/ctx.rs index 2df9bcb..5be7762 100644 --- a/src/routes/ctx.rs +++ b/src/routes/ctx.rs @@ -56,7 +56,7 @@ impl Clone for TracingInfo { impl TracingInfo { /// Create a new tracing info with the given service name and no context. #[allow(dead_code)] // used in some features - pub fn new(service: &'static str) -> Self { + pub const fn new(service: &'static str) -> Self { Self { service, context: None, @@ -65,7 +65,7 @@ impl TracingInfo { } /// Create a new tracing info with the given service name and context. - pub fn new_with_context( + pub const fn new_with_context( service: &'static str, context: opentelemetry::context::Context, ) -> Self { @@ -133,8 +133,8 @@ impl TracingInfo { /// Create a mock tracing info for testing. #[cfg(test)] - pub fn mock() -> Self { - use tracing::info_span; + pub const fn mock() -> Self { + Self { service: "test", context: None, @@ -200,7 +200,7 @@ impl HandlerCtx { /// Get a reference to the tracing span for this handler context. #[track_caller] pub fn span(&self) -> &tracing::Span { - &self.tracing.request_span() + self.tracing.request_span() } /// Set the tracing information for this handler context. From 172c8ffe4dd6988d3142b22c9708a82221207bbc Mon Sep 17 00:00:00 2001 From: James Date: Mon, 6 Oct 2025 11:59:21 -0400 Subject: [PATCH 17/24] fix: start message counters at 1 --- src/axum.rs | 8 ++++---- src/pubsub/shared.rs | 4 ++-- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/src/axum.rs b/src/axum.rs index e8d9671..d1327e8 100644 --- a/src/axum.rs +++ b/src/axum.rs @@ -40,8 +40,8 @@ impl From> for IntoAxum { Self { router, task_set: Default::default(), - rx_msg_id: Arc::new(AtomicU32::new(0)), - tx_msg_id: Arc::new(AtomicU32::new(0)), + rx_msg_id: Arc::new(AtomicU32::new(1)), + tx_msg_id: Arc::new(AtomicU32::new(1)), } } } @@ -52,8 +52,8 @@ impl IntoAxum { Self { router, task_set: handle.into(), - rx_msg_id: Arc::new(AtomicU32::new(0)), - tx_msg_id: Arc::new(AtomicU32::new(0)), + rx_msg_id: Arc::new(AtomicU32::new(1)), + tx_msg_id: Arc::new(AtomicU32::new(1)), } } } diff --git a/src/pubsub/shared.rs b/src/pubsub/shared.rs index a1dcb3f..26c3900 100644 --- a/src/pubsub/shared.rs +++ b/src/pubsub/shared.rs @@ -84,8 +84,8 @@ impl ConnectionManager { next_id: AtomicU64::new(0).into(), router, notification_buffer_per_task: DEFAULT_NOTIFICATION_BUFFER_PER_CLIENT, - tx_msg_id: Arc::new(AtomicU32::new(0)), - rx_msg_id: Arc::new(AtomicU32::new(0)), + tx_msg_id: Arc::new(AtomicU32::new(1)), + rx_msg_id: Arc::new(AtomicU32::new(1)), } } From e538f822b2bd918940ed6388785f53fa252175ff Mon Sep 17 00:00:00 2001 From: James Date: Mon, 6 Oct 2025 12:46:22 -0400 Subject: [PATCH 18/24] chore: document non-compliance --- README.md | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index ca0a743..22ae42e 100644 --- a/README.md +++ b/README.md @@ -77,10 +77,16 @@ See the [crate documentation on docs.rs] for more detailed examples. issues are found, please [open an issue]! `ajj` produces [`tracing`] spans and events that meet the [OpenTelemetry -semantic conventions] for JSON-RPC servers with the following exception: +semantic conventions] for JSON-RPC servers with the following exceptions: - The `server.address` attribute is NOT set, as the server address is not always known to the ajj system. +- `rpc.message` events are included in AJJ system spans for the batch request, + which technically does not comply with semantic conventions. The semantic + conventions do not specify how to handle batch requests, and assume that each + message corresponds to a separate request. In AJJ, batch requests are a single + message, and result in a single `rpc.message` event at receipt and at + response. ## Note on code provenance From c72f7213c2358dcadd99489c598ed08239751236 Mon Sep 17 00:00:00 2001 From: James Date: Mon, 6 Oct 2025 13:12:29 -0400 Subject: [PATCH 19/24] fix: mock --- src/routes/ctx.rs | 17 ++++++++++++++--- 1 file changed, 14 insertions(+), 3 deletions(-) diff --git a/src/routes/ctx.rs b/src/routes/ctx.rs index 5be7762..d3733db 100644 --- a/src/routes/ctx.rs +++ b/src/routes/ctx.rs @@ -133,12 +133,11 @@ impl TracingInfo { /// Create a mock tracing info for testing. #[cfg(test)] - pub const fn mock() -> Self { - + pub fn mock() -> Self { Self { service: "test", context: None, - span: OnceLock::new(), + span: OnceLock::from(info_span!("")), } } } @@ -377,6 +376,12 @@ pub struct HandlerArgs { impl HandlerArgs { /// Create new handler arguments. + /// + /// ## Panics + /// + /// If the ctx tracing span has not been initialized via + /// [`HandlerCtx::init_request_span`]. + #[track_caller] pub fn new(ctx: HandlerCtx, req: Request) -> Self { let this = Self { ctx, @@ -406,6 +411,12 @@ impl HandlerArgs { } /// Get a reference to the tracing span for this handler invocation. + /// + /// ## Panics + /// + /// If the span has not been initialized via + /// [`HandlerCtx::init_request_span`]. + #[track_caller] pub fn span(&self) -> &tracing::Span { self.ctx.span() } From 2aa541621d06648608da4a889e9021084373061f Mon Sep 17 00:00:00 2001 From: James Date: Wed, 8 Oct 2025 12:16:09 -0400 Subject: [PATCH 20/24] fix: clone vs child --- src/router.rs | 6 +-- src/routes/ctx.rs | 111 ++++++++++++++++++++++++++++---------------- tests/common/mod.rs | 24 ++++++---- 3 files changed, 91 insertions(+), 50 deletions(-) diff --git a/src/router.rs b/src/router.rs index 3939b1a..4f7793c 100644 --- a/src/router.rs +++ b/src/router.rs @@ -337,10 +337,10 @@ where for req in inbound.iter() { let req = req.map(|req| { - // Cloning here resets the `TracingInfo`, which means each + // This resets the `TracingInfo`, which means each // ctx has a separate span with similar metadata. - let ctx = ctx.clone(); - let request_span = ctx.init_request_span(self, Some(&batch_span)).clone(); + let ctx = ctx.child_ctx(self, Some(&batch_span)); + let request_span = ctx.span().clone(); // Several span fields are populated in `HandlerArgs::new`. let args = HandlerArgs::new(ctx, req); diff --git a/src/routes/ctx.rs b/src/routes/ctx.rs index d3733db..821f875 100644 --- a/src/routes/ctx.rs +++ b/src/routes/ctx.rs @@ -30,7 +30,7 @@ impl From> for NotifyError { /// Tracing information for OpenTelemetry. This struct is used to store /// information about the current request that can be used for tracing. -#[derive(Debug)] +#[derive(Debug, Clone)] #[non_exhaustive] pub struct TracingInfo { /// The OpenTelemetry service name. @@ -43,16 +43,6 @@ pub struct TracingInfo { span: OnceLock, } -impl Clone for TracingInfo { - fn clone(&self) -> Self { - Self { - service: self.service, - context: self.context.clone(), - span: OnceLock::new(), - } - } -} - impl TracingInfo { /// Create a new tracing info with the given service name and no context. #[allow(dead_code)] // used in some features @@ -76,6 +66,43 @@ impl TracingInfo { } } + fn make_span( + &self, + router: &Router, + with_notifications: bool, + parent: Option<&tracing::Span>, + ) -> tracing::Span + where + S: Clone + Send + Sync + 'static, + { + let span = info_span!( + parent: parent.and_then(|p| p.id()), + "AjjRequest", + "otel.kind" = "server", + "rpc.system" = "jsonrpc", + "rpc.jsonrpc.version" = "2.0", + "rpc.service" = router.service_name(), + notifications_enabled = with_notifications, + "trace_id" = ::tracing::field::Empty, + "otel.name" = ::tracing::field::Empty, + "otel.status_code" = ::tracing::field::Empty, + "rpc.jsonrpc.request_id" = ::tracing::field::Empty, + "rpc.jsonrpc.error_code" = ::tracing::field::Empty, + "rpc.jsonrpc.error_message" = ::tracing::field::Empty, + "rpc.method" = ::tracing::field::Empty, + params = ::tracing::field::Empty, + ); + if let Some(context) = &self.context { + let _ = span.set_parent(context.clone()); + + span.record( + "trace_id", + context.span().span_context().trace_id().to_string(), + ); + } + span + } + /// Create a request span for a handler invocation. fn init_request_span( &self, @@ -89,35 +116,23 @@ impl TracingInfo { // This span is populated with as much detail as possible, and then // given to the Request. It will be populated with request-specific // details (e.g. method) during request setup. - self.span.get_or_init(|| { - let span = info_span!( - parent: parent.and_then(|p| p.id()), - "AjjRequest", - "otel.kind" = "server", - "rpc.system" = "jsonrpc", - "rpc.jsonrpc.version" = "2.0", - "rpc.service" = router.service_name(), - notifications_enabled = with_notifications, - "trace_id" = ::tracing::field::Empty, - "otel.name" = ::tracing::field::Empty, - "otel.status_code" = ::tracing::field::Empty, - "rpc.jsonrpc.request_id" = ::tracing::field::Empty, - "rpc.jsonrpc.error_code" = ::tracing::field::Empty, - "rpc.jsonrpc.error_message" = ::tracing::field::Empty, - "rpc.method" = ::tracing::field::Empty, - params = ::tracing::field::Empty, - ); - if let Some(context) = &self.context { - let _ = span.set_parent(context.clone()); - - span.record( - "trace_id", - context.span().span_context().trace_id().to_string(), - ); - } + self.span + .get_or_init(|| self.make_span(router, with_notifications, parent)) + } - span - }) + /// Create a child tracing info for a new handler context. + pub fn child( + &self, + router: &Router, + with_notifications: bool, + parent: Option<&tracing::Span>, + ) -> Self { + let span = self.make_span(router, with_notifications, parent); + Self { + service: self.service, + context: self.context.clone(), + span: OnceLock::from(span), + } } /// Get a reference to the tracing span for this request. @@ -186,6 +201,24 @@ impl HandlerCtx { } } + /// Create a child handler context for a new handler invocation. + /// + /// This is used when handling batch requests, to give each handler + /// its own context. + pub fn child_ctx( + &self, + router: &Router, + parent: Option<&tracing::Span>, + ) -> Self { + Self { + notifications: self.notifications.clone(), + tasks: self.tasks.clone(), + tracing: self + .tracing + .child(router, self.notifications_enabled(), parent), + } + } + /// Get a reference to the tracing information for this handler context. pub const fn tracing_info(&self) -> &TracingInfo { &self.tracing diff --git a/tests/common/mod.rs b/tests/common/mod.rs index b42212a..edad5f4 100644 --- a/tests/common/mod.rs +++ b/tests/common/mod.rs @@ -104,13 +104,17 @@ async fn test_ping(client: &mut T) { /// basic tests of the test router pub async fn basic_tests(client: &mut T) { - test_ping(client).await; + timeout(Duration::from_secs(10), async move { + test_ping(client).await; - test_double(client).await; + test_double(client).await; - test_call_me_later(client).await; + test_call_me_later(client).await; - test_notification(client).await; + test_notification(client).await; + }) + .await + .unwrap(); } /// Test when a full batch request fails to parse @@ -205,11 +209,15 @@ async fn batch_contains_only_notifications(client: &mut T) { /// Test of batch requests pub async fn batch_tests(client: &mut T) { - batch_parse_fail(client).await; + timeout(Duration::from_secs(10), async move { + batch_parse_fail(client).await; - batch_single_req_parse_fail(client).await; + batch_single_req_parse_fail(client).await; - batch_contains_notification(client).await; + batch_contains_notification(client).await; - batch_contains_only_notifications(client).await; + batch_contains_only_notifications(client).await; + }) + .await + .unwrap(); } From 6425abdb116569e62214f13880c5ffcefb435ebe Mon Sep 17 00:00:00 2001 From: James Date: Thu, 9 Oct 2025 11:15:15 -0400 Subject: [PATCH 21/24] refactor: add handler macro, implement metrics --- Cargo.toml | 1 + src/axum.rs | 1 + src/lib.rs | 2 + src/macros.rs | 173 ++++++++++++++ src/metrics.rs | 170 +++++++++++++ src/router.rs | 13 +- src/routes/ctx.rs | 4 +- src/routes/future.rs | 18 +- src/routes/handler.rs | 317 ++++--------------------- src/types/resp/mod.rs | 5 + src/types/{resp.rs => resp/payload.rs} | 214 +++-------------- src/types/resp/ser.rs | 138 +++++++++++ 12 files changed, 594 insertions(+), 462 deletions(-) create mode 100644 src/metrics.rs create mode 100644 src/types/resp/mod.rs rename src/types/{resp.rs => resp/payload.rs} (55%) create mode 100644 src/types/resp/ser.rs diff --git a/Cargo.toml b/Cargo.toml index 2073eb7..36f6fc8 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -40,6 +40,7 @@ interprocess = { version = "2.2.2", features = ["async", "tokio"], optional = tr # ws tokio-tungstenite = { version = "0.26.1", features = ["rustls-tls-webpki-roots"], optional = true } futures-util = { version = "0.3.31", optional = true } +metrics = "0.24.2" [dev-dependencies] ajj = { path = "./", features = ["axum", "ws", "ipc"] } diff --git a/src/axum.rs b/src/axum.rs index d1327e8..3e4e74c 100644 --- a/src/axum.rs +++ b/src/axum.rs @@ -87,6 +87,7 @@ where ctx.init_request_span(&self.router, Some(&Span::current())); let Ok(bytes) = Bytes::from_request(req, &state).await else { + crate::metrics::record_parse_error(self.router.service_name()); return Box::::from(Response::parse_error()).into_response(); }; diff --git a/src/lib.rs b/src/lib.rs index e83fd11..37c297e 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -159,6 +159,8 @@ mod axum; mod error; pub use error::RegistrationError; +pub(crate) mod metrics; + mod primitives; pub use primitives::{BorrowedRpcObject, MethodId, RpcBorrow, RpcObject, RpcRecv, RpcSend}; diff --git a/src/macros.rs b/src/macros.rs index db9cbd6..89529d3 100644 --- a/src/macros.rs +++ b/src/macros.rs @@ -76,6 +76,179 @@ macro_rules! message_event { }; } +/// Implement a `Handler` call, with metrics recording and response building. +macro_rules! impl_handler_call { + (@metrics, $success:expr, $id:expr, $service:expr, $method:expr) => {{ + // Record the metrics. + $crate::metrics::record_execution($success, $service, $method); + $crate::metrics::record_output($id.is_some(), $service, $method); + }}; + + (@record_span, $span:expr, $payload:expr) => { + if let Some(e) = $payload.as_error() { + use tracing_opentelemetry::OpenTelemetrySpanExt; + $span.record("rpc.jsonrpc.error_code", e.code); + $span.record("rpc.jsonrpc.error_message", e.message.as_ref()); + $span.set_status(::opentelemetry::trace::Status::Error { + description: e.message.clone(), + }); + } + }; + + // Hit the metrics and return the payload if any. + (@finish $span:expr, $id:expr, $service:expr, $method:expr, $payload:expr) => {{ + impl_handler_call!(@metrics, $payload.is_success(), $id, $service, $method); + impl_handler_call!(@record_span, $span, $payload); + return Response::build_response($id.as_deref(), $payload); + }}; + + (@unpack_params $span:expr, $id:expr, $service:expr, $method:expr, $req:expr) => {{ + let Ok(params) = $req.deser_params() else { + impl_handler_call!(@finish $span, $id, $service, $method, &ResponsePayload::<(), ()>::invalid_params()); + }; + drop($req); // no longer needed + params + }}; + + (@unpack_struct_params $span:expr, $id:expr, $service:expr, $method:expr, $req:expr) => {{ + let Ok(params) = $req.deser_params() else { + impl_handler_call!(@finish $span, $id, $service, $method, &ResponsePayload::<(), ()>::invalid_params()); + }; + drop($req); // no longer needed + params + }}; + + (@unpack $args:expr) => {{ + let id = $args.id_owned(); + let (ctx, req) = $args.into_parts(); + let inst = ctx.span().clone(); + let span = ctx.span().clone(); + let method = req.method().to_string(); + let service = ctx.service_name(); + + (id, ctx, inst, span, method, service, req) + }}; + + // NO ARGS + ($args:expr, $this:ident()) => {{ + let (id, ctx, inst, span, method, service, req) = impl_handler_call!(@unpack $args); + drop(ctx); // no longer needed + drop(req); // no longer needed + + Box::pin( + async move { + let payload: $crate::ResponsePayload<_, _> = $this().await.into(); + impl_handler_call!(@finish span, id, service, &method, &payload); + } + .instrument(inst), + ) + }}; + + // CTX only + ($args:expr, $this:ident(ctx)) => {{ + let (id, ctx, inst, span, method, service, req) = impl_handler_call!(@unpack $args); + drop(req); // no longer needed + + Box::pin( + async move { + let payload: $crate::ResponsePayload<_, _> = $this(ctx).await.into(); + impl_handler_call!(@finish span, id, service, &method, &payload); + } + .instrument(inst), + ) + }}; + + + // PARAMS only + ($args:expr, $this:ident(params: $params_ty:ty)) => {{ + let (id, ctx, inst, span, method, service, req) = impl_handler_call!(@unpack $args); + drop(ctx); // no longer needed + + Box::pin( + async move { + let params: $params_ty = impl_handler_call!(@unpack_params span, id, service, &method, req); + let payload: $crate::ResponsePayload<_, _> = $this(params.into()).await.into(); + impl_handler_call!(@finish span, id, service, &method, &payload); + } + .instrument(inst), + ) + }}; + + + // STATE only + ($args:expr, $this:ident($state:expr)) => {{ + let (id, ctx, inst, span, method, service, req) = impl_handler_call!(@unpack $args); + drop(ctx); // no longer needed + drop(req); // no longer needed + + Box::pin( + async move { + let payload: $crate::ResponsePayload<_, _> = $this($state).await.into(); + impl_handler_call!(@finish span, id, service, &method, &payload); + } + .instrument(inst), + ) + }}; + + + // CTX and PARAMS + ($args:expr, $this:ident(ctx, params: $params_ty:ty)) => {{ + let (id, ctx, inst, span, method, service, req) = impl_handler_call!(@unpack $args); + + Box::pin( + async move { + let params: $params_ty = impl_handler_call!(@unpack_params span, id, service, &method, req); + let payload: $crate::ResponsePayload<_, _> = $this(ctx, params.into()).await.into(); + impl_handler_call!(@finish span, id, service, &method, &payload); + } + .instrument(inst), + ) + }}; + + // CTX and STATE + ($args:expr, $this:ident(ctx, $state:expr)) => {{ + let (id, ctx, inst, span, method, service, req) = impl_handler_call!(@unpack $args); + drop(req); // no longer needed + + Box::pin( + async move { + let payload: $crate::ResponsePayload<_, _> = $this(ctx, $state).await.into(); + impl_handler_call!(@finish span, id, service, &method, &payload); + } + .instrument(inst), + ) + }}; + + // PARAMS and STATE + ($args:expr, $this:ident(params: $params_ty:ty, $state:expr)) => {{ + let (id, ctx, inst, span, method, service, req) = impl_handler_call!(@unpack $args); + drop(ctx); // no longer needed + + Box::pin( + async move { + let params: $params_ty = impl_handler_call!(@unpack_params span, id, service, &method, req); + let payload: $crate::ResponsePayload<_, _> = $this(params.into(), $state).await.into(); + impl_handler_call!(@finish span, id, service, &method, &payload); + } + .instrument(inst), + ) + }}; + + // CTX and PARAMS and STATE + ($args:expr, $this:ident(ctx, params: $params_ty:ty, $state:expr)) => {{ + let (id, ctx, inst, span, method, service, req) = impl_handler_call!(@unpack $args); + + Box::pin( + async move { + let params: $params_ty = impl_handler_call!(@unpack_params span, id, service, &method, req); + let payload: $crate::ResponsePayload<_, _> = $this(ctx, params.into(), $state).await.into(); + impl_handler_call!(@finish span, id, service, &method, &payload); + } + .instrument(inst), + ) + }}; +} + // Some code is this file is reproduced under the terms of the MIT license. It // originates from the `axum` crate. The original source code can be found at // the following URL, and the original license is included below. diff --git a/src/metrics.rs b/src/metrics.rs new file mode 100644 index 0000000..ce55b96 --- /dev/null +++ b/src/metrics.rs @@ -0,0 +1,170 @@ +use std::sync::LazyLock; + +use metrics::Counter; + +/// Metric name for counting router calls. +pub(crate) const ROUTER_CALLS: &str = "ajj.router.calls"; +pub(crate) const ROUTER_CALLS_HELP: &str = + "Number of calls to ajj router methods. Not all requests will result in a response."; + +/// Metric name for counting router error execution. +pub(crate) const ROUTER_ERRORS: &str = "ajj.router.errors"; +pub(crate) const ROUTER_ERRORS_HELP: &str = + "Number of errored executions by ajj router methods. This does NOT imply a response was sent."; + +// Metric name for counting router successful executions. +pub(crate) const ROUTER_SUCCESSES: &str = "ajj.router.successes"; +pub(crate) const ROUTER_SUCCESSES_HELP: &str = + "Number of successful executions by ajj router methods. This does NOT imply a response was sent."; + +/// Metric name for counting router responses. +pub(crate) const ROUTER_RESPONSES: &str = "ajj.router.responses"; +pub(crate) const ROUTER_RESPONSES_HELP: &str = + "Number of responses sent by ajj router methods. Not all requests will result in a response."; + +// Metric name for counting omitted notification responses. +pub(crate) const ROUTER_NOTIFICATION_RESPONSE_OMITTED: &str = + "ajj.router.notification_response_omitted"; +pub(crate) const ROUTER_NOTIFICATION_RESPONSE_OMITTED_HELP: &str = + "Number of times ajj router methods omitted a response to a notification"; + +// Metric for counting parse errors. +pub(crate) const ROUTER_PARSE_ERRORS: &str = "ajj.router.parse_errors"; +pub(crate) const ROUTER_PARSE_ERRORS_HELP: &str = + "Number of parse errors encountered by ajj router methods. This implies no response was sent."; + +static DESCRIBE: LazyLock<()> = LazyLock::new(|| { + metrics::describe_counter!(ROUTER_CALLS, metrics::Unit::Count, ROUTER_CALLS_HELP); + metrics::describe_counter!(ROUTER_ERRORS, metrics::Unit::Count, ROUTER_ERRORS_HELP); + metrics::describe_counter!( + ROUTER_SUCCESSES, + metrics::Unit::Count, + ROUTER_SUCCESSES_HELP + ); + metrics::describe_counter!( + ROUTER_RESPONSES, + metrics::Unit::Count, + ROUTER_RESPONSES_HELP + ); + metrics::describe_counter!( + ROUTER_NOTIFICATION_RESPONSE_OMITTED, + metrics::Unit::Count, + ROUTER_NOTIFICATION_RESPONSE_OMITTED_HELP + ); + metrics::describe_counter!( + ROUTER_PARSE_ERRORS, + metrics::Unit::Count, + ROUTER_PARSE_ERRORS_HELP + ); +}); + +/// Get or register a counter for calls to a specific service and method. +pub(crate) fn calls(service_name: &'static str, method: &str) -> Counter { + let _ = &DESCRIBE; + metrics::counter!( + ROUTER_CALLS, + "service" => service_name.to_string(), + "method" => method.to_string() + ) +} + +/// Record a call to a specific service and method. +pub(crate) fn record_call(service_name: &'static str, method: &str) { + let counter = calls(service_name, method); + counter.increment(1); +} + +/// Get or register a counter for errors from a specific service and method. +pub(crate) fn errors(service_name: &'static str, method: &str) -> Counter { + let _ = &DESCRIBE; + metrics::counter!( + ROUTER_ERRORS, + "service" => service_name.to_string(), + "method" => method.to_string() + ) +} + +/// Record an error from a specific service and method. +pub(crate) fn record_execution_error(service_name: &'static str, method: &str) { + let counter = errors(service_name, method); + counter.increment(1); +} + +/// Get or register a counter for successes from a specific service and method. +pub(crate) fn successes(service_name: &'static str, method: &str) -> Counter { + let _ = &DESCRIBE; + metrics::counter!( + ROUTER_SUCCESSES, + "service" => service_name.to_string(), + "method" => method.to_string() + ) +} + +/// Record a success from a specific service and method. +pub(crate) fn record_execution_success(service_name: &'static str, method: &str) { + let counter = successes(service_name, method); + counter.increment(1); +} + +/// Record a response from a specific service and method, incrementing either +/// the success or error counter. +pub(crate) fn record_execution(success: bool, service_name: &'static str, method: &str) { + if success { + record_execution_success(service_name, method); + } else { + record_execution_error(service_name, method); + } +} + +/// Get or register a counter for responses from a specific service and method. +pub(crate) fn responses(service_name: &'static str, method: &str) -> Counter { + let _ = &DESCRIBE; + metrics::counter!( + ROUTER_RESPONSES, + "service" => service_name.to_string(), + "method" => method.to_string() + ) +} + +/// Record a response from a specific service and method. +pub(crate) fn record_response(service_name: &'static str, method: &str) { + let counter = responses(service_name, method); + counter.increment(1); +} + +/// Get or register a counter for omitted notification responses from a specific service and method. +pub(crate) fn response_omitted(service_name: &'static str, method: &str) -> Counter { + let _ = &DESCRIBE; + metrics::counter!( + ROUTER_NOTIFICATION_RESPONSE_OMITTED, + "service" => service_name.to_string(), + "method" => method.to_string() + ) +} + +/// Record an omitted notification response from a specific service and method. +pub(crate) fn record_response_omitted(service_name: &'static str, method: &str) { + let counter = response_omitted(service_name, method); + counter.increment(1); +} + +/// Record either a response sent or an omitted notification response. +pub(crate) fn record_output(response_sent: bool, service_name: &'static str, method: &str) { + if response_sent { + record_response(service_name, method); + } else { + record_response_omitted(service_name, method); + } +} + +// Get or register a counter for parse errors. +pub(crate) fn parse_errors(service_name: &'static str) -> Counter { + let _ = &DESCRIBE; + metrics::counter!(ROUTER_PARSE_ERRORS, "service" => service_name.to_string()) +} + +/// Record a parse error. +pub(crate) fn record_parse_error(service_name: &'static str) { + let counter = parse_errors(service_name); + counter.increment(1); +} diff --git a/src/router.rs b/src/router.rs index 4f7793c..884066f 100644 --- a/src/router.rs +++ b/src/router.rs @@ -108,7 +108,7 @@ where /// Get the OpenTelemetry service name for this router. pub fn service_name(&self) -> &'static str { - self.inner.service_name.unwrap_or("ajj") + self.inner.service_name() } /// If this router is the only reference to its inner state, return the @@ -330,7 +330,8 @@ where inbound: InboundData, state: S, ) -> BatchFuture { - let mut fut = BatchFuture::new_with_capacity(inbound.single(), inbound.len()); + let mut fut = + BatchFuture::new_with_capacity(inbound.single(), self.service_name(), inbound.len()); // According to spec, non-parsable requests should still receive a // response. let batch_span = debug_span!(parent: ctx.span(), "BatchFuture::poll", reqs = inbound.len(), futs = tracing::field::Empty); @@ -557,6 +558,11 @@ impl RouterInner { } } + /// Get the OpenTelemetry service name for this router. + fn service_name(&self) -> &'static str { + self.service_name.unwrap_or("ajj") + } + /// Get the next available ID. fn get_id(&mut self) -> MethodId { self.last_id += 1; @@ -641,6 +647,9 @@ impl RouterInner { #[track_caller] pub(crate) fn call_with_state(&self, args: HandlerArgs, state: S) -> RouteFuture { let method = args.req().method(); + + crate::metrics::record_call(self.service_name(), method); + self.method_by_name(method) .unwrap_or(&self.fallback) .call_with_state(args, state) diff --git a/src/routes/ctx.rs b/src/routes/ctx.rs index 821f875..bf001bb 100644 --- a/src/routes/ctx.rs +++ b/src/routes/ctx.rs @@ -225,7 +225,7 @@ impl HandlerCtx { } /// Get the OpenTelemetry service name for this handler context. - pub const fn otel_service_name(&self) -> &'static str { + pub const fn service_name(&self) -> &'static str { self.tracing.service } @@ -466,6 +466,6 @@ impl HandlerArgs { /// Get the OpenTelemetry span name for this handler invocation. pub fn otel_span_name(&self) -> String { - format!("{}/{}", self.ctx.otel_service_name(), self.req.method()) + format!("{}/{}", self.ctx.service_name(), self.req.method()) } } diff --git a/src/routes/future.rs b/src/routes/future.rs index 35b9846..71a97ba 100644 --- a/src/routes/future.rs +++ b/src/routes/future.rs @@ -119,6 +119,9 @@ pub struct BatchFuture { /// Whether the batch was a single request. single: bool, + /// The service name, for tracing and metrics. + service_name: &'static str, + /// The span (if any). span: Option, } @@ -134,11 +137,16 @@ impl fmt::Debug for BatchFuture { impl BatchFuture { /// Create a new batch future with a capacity. - pub(crate) fn new_with_capacity(single: bool, capacity: usize) -> Self { + pub(crate) fn new_with_capacity( + single: bool, + service_name: &'static str, + capacity: usize, + ) -> Self { Self { futs: BatchFutureInner::Prepping(Vec::with_capacity(capacity)), resps: Vec::with_capacity(capacity), single, + service_name, span: None, } } @@ -166,6 +174,7 @@ impl BatchFuture { /// Push a parse error into the batch. pub(crate) fn push_parse_error(&mut self) { + crate::metrics::record_parse_error(self.service_name); self.push_resp(Response::parse_error()); } @@ -196,6 +205,7 @@ impl std::future::Future for BatchFuture { if matches!(self.futs, BatchFutureInner::Prepping(_)) { // SPEC: empty arrays are invalid if self.futs.is_empty() && self.resps.is_empty() { + crate::metrics::record_parse_error(self.service_name); return Poll::Ready(Some(Response::parse_error())); } self.futs.run(); @@ -227,7 +237,11 @@ impl std::future::Future for BatchFuture { // SPEC: single requests return a single response // Batch requests return an array of responses let resp = if *this.single { - this.resps.pop().unwrap_or_else(Response::parse_error) + this.resps.pop().unwrap_or_else(|| { + // this should never happen, but just in case... + crate::metrics::record_parse_error(this.service_name); + Response::parse_error() + }) } else { // otherwise, we have an array of responses serde_json::value::to_raw_value(&this.resps) diff --git a/src/routes/handler.rs b/src/routes/handler.rs index a370f01..ffa833c 100644 --- a/src/routes/handler.rs +++ b/src/routes/handler.rs @@ -5,15 +5,6 @@ use serde_json::value::RawValue; use std::{convert::Infallible, future::Future, marker::PhantomData, pin::Pin, task}; use tracing::{trace, Instrument}; -macro_rules! convert_result { - ($res:expr) => {{ - match $res { - Ok(val) => ResponsePayload::Success(val), - Err(err) => ResponsePayload::internal_error_with_obj(err), - } - }}; -} - /// Hint type for differentiating certain handler impls. See the [`Handler`] /// trait "Handler argument type inference" section for more information. #[derive(Debug)] @@ -26,6 +17,12 @@ pub struct State(pub S); #[repr(transparent)] pub struct Params(pub T); +impl From for Params { + fn from(value: T) -> Self { + Self(value) + } +} + /// Marker type used for differentiating certain handler impls. #[allow(missing_debug_implementations, unreachable_pub)] pub struct PhantomState(PhantomData); @@ -227,7 +224,7 @@ pub struct PhantomParams(PhantomData); /// let cant_infer_err = || async { Ok(2) }; /// /// // cannot infer type of the type parameter `ErrData` declared on the enum `ResponsePayload` -/// let cant_infer_failure = || async { ResponsePayload::Success(3) }; +/// let cant_infer_failure = || async { ResponsePayload(Ok(3)) }; /// /// // cannot infer type of the type parameter `ErrData` declared on the enum `ResponsePayload` /// let cant_infer_success = || async { ResponsePayload::internal_error_with_obj(4) }; @@ -245,7 +242,7 @@ pub struct PhantomParams(PhantomData); /// let handler_b = || async { Err::<(), _>(2) }; /// /// // specify the ErrData on your Success -/// let handler_c = || async { ResponsePayload::Success::<_, ()>(3) }; +/// let handler_c = || async { ResponsePayload::<_, ()>(Ok(3)) }; /// /// // specify the Payload on your Failure /// let handler_d = || async { ResponsePayload::<(), _>::internal_error_with_obj(4) }; @@ -443,6 +440,7 @@ pub struct OutputResponsePayload { _sealed: (), } +/// Takes nothing, returns ResponsePayload impl Handler<(OutputResponsePayload,), S> for F where F: FnOnce() -> Fut + Clone + Send + Sync + 'static, @@ -453,15 +451,11 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { - let id = args.id_owned(); - - Box::pin(async move { - let payload = self().await; - Response::maybe(args.span(), id.as_deref(), &payload) - }) + impl_handler_call!(args, self()) } } +/// Takes Ctx, returns ResponsePayload impl Handler<(OutputResponsePayload, HandlerCtx), S> for F where F: FnOnce(HandlerCtx) -> Fut + Clone + Send + Sync + 'static, @@ -472,17 +466,11 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { - let id = args.id_owned(); - let (ctx, _) = args.into_parts(); - let span = ctx.span().clone(); - - Box::pin(async move { - let payload = self(ctx).await; - Response::maybe(&span, id.as_deref(), &payload) - }) + impl_handler_call!(args, self(ctx)) } } +/// Takes Params, returns ResponsePayload impl Handler<(OutputResponsePayload, PhantomParams), S> for F where @@ -495,20 +483,11 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { - let (ctx, req) = args.into_parts(); - let span = ctx.span().clone(); - Box::pin(async move { - let id = req.id_owned(); - let Ok(params) = req.deser_params() else { - return Response::maybe_invalid_params(id.as_deref()); - }; - - let payload = self(params).await; - Response::maybe(&span, id.as_deref(), &payload) - }) + impl_handler_call!(args, self(params: Input)) } } +/// Takes Params, returns ResponsePayload impl Handler<(OutputResponsePayload, Params), S> for F where F: FnOnce(Params) -> Fut + Clone + Send + Sync + 'static, @@ -520,20 +499,11 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { - let (ctx, req) = args.into_parts(); - let span = ctx.span().clone(); - Box::pin(async move { - let id = req.id_owned(); - let Ok(params) = req.deser_params() else { - return Response::maybe_invalid_params(id.as_deref()); - }; - - let payload = self(Params(params)).await; - Response::maybe(&span, id.as_deref(), &payload) - }) + impl_handler_call!(args, self(params: Input)) } } +/// Takes State, returns ResponsePayload impl Handler<(OutputResponsePayload, PhantomState), S> for F where F: FnOnce(S) -> Fut + Clone + Send + Sync + 'static, @@ -545,14 +515,11 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { - let id = args.id_owned(); - Box::pin(async move { - let payload = self(state).await; - Response::maybe(args.span(), id.as_deref(), &payload) - }) + impl_handler_call!(args, self(state)) } } +/// Takes State, returns ResponsePayload impl Handler<(OutputResponsePayload, State), S> for F where F: FnOnce(State) -> Fut + Clone + Send + Sync + 'static, @@ -564,14 +531,11 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { - let id = args.id_owned(); - Box::pin(async move { - let payload = self(State(state)).await; - Response::maybe(args.span(), id.as_deref(), &payload) - }) + impl_handler_call!(args, self(State(state))) } } +/// Takes Ctx, Params, returns ResponsePayload impl Handler<(OutputResponsePayload, HandlerCtx, PhantomParams), S> for F where @@ -584,23 +548,11 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { - Box::pin(async move { - let (ctx, req) = args.into_parts(); - let span = ctx.span().clone(); - - let id = req.id_owned(); - let Ok(params) = req.deser_params() else { - return Response::maybe_invalid_params(id.as_deref()); - }; - - drop(req); // deallocate explicitly. No funny business. - - let payload = self(ctx, params).await; - Response::maybe(&span, id.as_deref(), &payload) - }) + impl_handler_call!(args, self(ctx, params: Input)) } } +/// Takes Ctx, Params, returns ResponsePayload impl Handler<(OutputResponsePayload, HandlerCtx, Params), S> for F where @@ -613,23 +565,11 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { - Box::pin(async move { - let (ctx, req) = args.into_parts(); - let span = ctx.span().clone(); - - let id = req.id_owned(); - let Ok(params) = req.deser_params() else { - return Response::maybe_invalid_params(id.as_deref()); - }; - - drop(req); // deallocate explicitly. No funny business. - - let payload = self(ctx, Params(params)).await; - Response::maybe(&span, id.as_deref(), &payload) - }) + impl_handler_call!(args, self(ctx, params: Input)) } } +/// Takes Params, State, returns ResponsePayload impl Handler<(OutputResponsePayload, Input, S), S> for F where F: FnOnce(Input, S) -> Fut + Clone + Send + Sync + 'static, @@ -642,23 +582,11 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { - Box::pin(async move { - let (ctx, req) = args.into_parts(); - let span = ctx.span().clone(); - - let id = req.id_owned(); - let Ok(params) = req.deser_params() else { - return Response::maybe_invalid_params(id.as_deref()); - }; - drop(req); // deallocate explicitly. No funny business. - - let payload = self(params, state).await; - - Response::maybe(&span, id.as_deref(), &payload) - }) + impl_handler_call!(args, self(params: Input, state)) } } +/// Takes Ctx, State, returns ResponsePayload impl Handler<(OutputResponsePayload, HandlerCtx, PhantomState), S> for F where @@ -671,20 +599,11 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { - Box::pin(async move { - let (ctx, req) = args.into_parts(); - let span = ctx.span().clone(); - let id = req.id_owned(); - - drop(req); // deallocate explicitly. No funny business. - - let payload = self(ctx, state).await; - - Response::maybe(&span, id.as_deref(), &payload) - }) + impl_handler_call!(args, self(ctx, state)) } } +/// Takes Ctx, State, returns ResponsePayload impl Handler<(OutputResponsePayload, HandlerCtx, State), S> for F where F: FnOnce(HandlerCtx, State) -> Fut + Clone + Send + Sync + 'static, @@ -696,20 +615,11 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { - Box::pin(async move { - let (ctx, req) = args.into_parts(); - let span = ctx.span().clone(); - let id = req.id_owned(); - - drop(req); // deallocate explicitly. No funny business. - - let payload = self(ctx, State(state)).await; - - Response::maybe(&span, id.as_deref(), &payload) - }) + impl_handler_call!(args, self(ctx, State(state))) } } +/// Takes Ctx, Params, State, returns ResponsePayload impl Handler<(OutputResponsePayload, HandlerCtx, Input, S), S> for F where @@ -723,23 +633,11 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { - Box::pin(async move { - let (ctx, req) = args.into_parts(); - let span = ctx.span().clone(); - let id = req.id_owned(); - - let Ok(params) = req.deser_params() else { - return Response::maybe_invalid_params(id.as_deref()); - }; - - drop(req); // deallocate explicitly. No funny business. - - let payload = self(ctx, params, state).await; - Response::maybe(&span, id.as_deref(), &payload) - }) + impl_handler_call!(args, self(ctx, params: Input, state)) } } +/// Takes nothing, returns Result impl Handler<(OutputResult,), S> for F where F: FnOnce() -> Fut + Clone + Send + Sync + 'static, @@ -750,13 +648,7 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { - let id = args.id_owned(); - let span = args.span().clone(); - drop(args); - Box::pin(async move { - let payload = self().await; - Response::maybe(&span, id.as_deref(), &convert_result!(payload)) - }) + impl_handler_call!(args, self()) } } @@ -770,16 +662,7 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { - let (ctx, req) = args.into_parts(); - let span = ctx.span().clone(); - let id = req.id_owned(); - - drop(req); - - Box::pin(async move { - let payload = convert_result!(self(ctx).await); - Response::maybe(&span, id.as_deref(), &payload) - }) + impl_handler_call!(args, self(ctx)) } } @@ -794,20 +677,7 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { - Box::pin(async move { - let (ctx, req) = args.into_parts(); - let span = ctx.span().clone(); - let id = req.id_owned(); - let Ok(params) = req.deser_params() else { - return Response::maybe_invalid_params(id.as_deref()); - }; - - drop(req); // deallocate explicitly. No funny business. - - let payload = convert_result!(self(params).await); - - Response::maybe(&span, id.as_deref(), &payload) - }) + impl_handler_call!(args, self(params: Input)) } } @@ -822,21 +692,7 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { - Box::pin(async move { - let (ctx, req) = args.into_parts(); - let span = ctx.span().clone(); - - let id = req.id_owned(); - let Ok(params) = req.deser_params() else { - return Response::maybe_invalid_params(id.as_deref()); - }; - - drop(req); // deallocate explicitly. No funny business. - - let payload = convert_result!(self(Params(params)).await); - - Response::maybe(&span, id.as_deref(), &payload) - }) + impl_handler_call!(args, self(params: Input)) } } @@ -851,11 +707,7 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { - let id = args.id_owned(); - Box::pin(async move { - let payload = convert_result!(self(state).await); - Response::maybe(args.span(), id.as_deref(), &payload) - }) + impl_handler_call!(args, self(state)) } } @@ -870,11 +722,7 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { - let id = args.id_owned(); - Box::pin(async move { - let payload = convert_result!(self(State(state)).await); - Response::maybe(args.span(), id.as_deref(), &payload) - }) + impl_handler_call!(args, self(State(state))) } } @@ -890,20 +738,7 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { - Box::pin(async move { - let (ctx, req) = args.into_parts(); - let span = ctx.span().clone(); - let id = req.id_owned(); - let Ok(params) = req.deser_params() else { - return Response::maybe_invalid_params(id.as_deref()); - }; - - drop(req); // deallocate explicitly. No funny business. - - let payload = convert_result!(self(ctx, params).await); - - Response::maybe(&span, id.as_deref(), &payload) - }) + impl_handler_call!(args, self(ctx, params: Input)) } } @@ -918,20 +753,7 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, _state: S) -> Self::Future { - Box::pin(async move { - let (ctx, req) = args.into_parts(); - let span = ctx.span().clone(); - let id = req.id_owned(); - let Ok(params) = req.deser_params() else { - return Response::maybe_invalid_params(id.as_deref()); - }; - - drop(req); // deallocate explicitly. No funny business. - - let payload = convert_result!(self(ctx, Params(params)).await); - - Response::maybe(&span, id.as_deref(), &payload) - }) + impl_handler_call!(args, self(ctx, params: Input)) } } @@ -947,21 +769,7 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { - Box::pin(async move { - let (ctx, req) = args.into_parts(); - let span = ctx.span().clone(); - - let id = req.id_owned(); - let Ok(params) = req.deser_params() else { - return Response::maybe_invalid_params(id.as_deref()); - }; - - drop(req); // deallocate explicitly. No funny business. - - let payload = convert_result!(self(params, state).await); - - Response::maybe(&span, id.as_deref(), &payload) - }) + impl_handler_call!(args, self(params: Input, state)) } } @@ -976,17 +784,7 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { - let (ctx, req) = args.into_parts(); - let span = ctx.span().clone(); - let id = req.id_owned(); - - drop(req); // deallocate explicitly. No funny business. - - Box::pin(async move { - let payload = convert_result!(self(ctx, state).await); - - Response::maybe(&span, id.as_deref(), &payload) - }) + impl_handler_call!(args, self(ctx, state)) } } @@ -1001,17 +799,7 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { - let (ctx, req) = args.into_parts(); - let span = ctx.span().clone(); - let id = req.id_owned(); - - drop(req); // deallocate explicitly. No funny business. - - Box::pin(async move { - let payload = convert_result!(self(ctx, State(state)).await); - - Response::maybe(&span, id.as_deref(), &payload) - }) + impl_handler_call!(args, self(ctx, State(state))) } } @@ -1027,20 +815,7 @@ where type Future = Pin>> + Send>>; fn call_with_state(self, args: HandlerArgs, state: S) -> Self::Future { - Box::pin(async move { - let (ctx, req) = args.into_parts(); - let span = ctx.span().clone(); - let id = req.id_owned(); - let Ok(params) = req.deser_params() else { - return Response::maybe_invalid_params(id.as_deref()); - }; - - drop(req); // deallocate explicitly. No funny business. - - let payload = convert_result!(self(ctx, params, state).await); - - Response::maybe(&span, id.as_deref(), &payload) - }) + impl_handler_call!(args, self(ctx, params: Input, state)) } } @@ -1053,7 +828,7 @@ mod test { struct NewType; async fn resp_ok() -> ResponsePayload<(), ()> { - ResponsePayload::Success(()) + ResponsePayload(Ok(())) } async fn ok() -> Result<(), ()> { diff --git a/src/types/resp/mod.rs b/src/types/resp/mod.rs new file mode 100644 index 0000000..b20095b --- /dev/null +++ b/src/types/resp/mod.rs @@ -0,0 +1,5 @@ +mod payload; +pub use payload::{ErrorPayload, ResponsePayload}; + +mod ser; +pub(crate) use ser::Response; diff --git a/src/types/resp.rs b/src/types/resp/payload.rs similarity index 55% rename from src/types/resp.rs rename to src/types/resp/payload.rs index acf0e77..58f92e0 100644 --- a/src/types/resp.rs +++ b/src/types/resp/payload.rs @@ -1,169 +1,61 @@ use crate::RpcSend; -use opentelemetry::trace::Status; -use serde::{ser::SerializeMap, Serialize, Serializer}; +use serde::Serialize; use serde_json::value::{to_raw_value, RawValue}; use std::borrow::Cow; use std::fmt; -use tracing_opentelemetry::OpenTelemetrySpanExt; const INTERNAL_ERROR: Cow<'_, str> = Cow::Borrowed("Internal error"); -/// Response struct. -#[derive(Debug, Clone)] -pub(crate) struct Response<'a, 'b, T, E> { - pub(crate) id: &'b RawValue, - pub(crate) payload: &'a ResponsePayload, -} - -impl Response<'_, '_, (), ()> { - /// Parse error response, used when the request is not valid JSON. - #[allow(dead_code)] // used in features - pub(crate) fn parse_error() -> Box { - Response::<(), ()> { - id: RawValue::NULL, - payload: &ResponsePayload::parse_error(), - } - .to_json() - } - - /// Invalid params response, used when the params field does not - /// deserialize into the expected type. - pub(crate) fn invalid_params(id: &RawValue) -> Box { - Response::<(), ()> { - id, - payload: &ResponsePayload::invalid_params(), - } - .to_json() - } - - /// Invalid params response, used when the params field does not - /// deserialize into the expected type. This function exists to simplify - /// notification responses, which should be omitted. - pub(crate) fn maybe_invalid_params(id: Option<&RawValue>) -> Option> { - id.map(Self::invalid_params) - } - - /// Method not found response, used in default fallback handler. - pub(crate) fn method_not_found(id: &RawValue) -> Box { - Response::<(), ()> { - id, - payload: &ResponsePayload::method_not_found(), - } - .to_json() - } - - /// Method not found response, used in default fallback handler. This - /// function exists to simplify notification responses, which should be - /// omitted. - pub(crate) fn maybe_method_not_found(id: Option<&RawValue>) -> Option> { - id.map(Self::method_not_found) - } - - /// Response failed to serialize - pub(crate) fn serialization_failure(id: &RawValue) -> Box { - RawValue::from_string(format!( - r#"{{"jsonrpc":"2.0","id":{},"error":{{"code":-32700,"message":"response serialization error"}}}}"#, - id.get() - )) - .expect("valid json") - } -} - -impl<'a, 'b, T, E> Response<'a, 'b, T, E> -where - T: Serialize, - E: Serialize, -{ - pub(crate) fn maybe( - span: &tracing::Span, - id: Option<&'b RawValue>, - payload: &'a ResponsePayload, - ) -> Option> { - if let Some(e) = payload.as_error() { - span.record("rpc.jsonrpc.error_code", e.code); - span.record("rpc.jsonrpc.error_message", e.message.as_ref()); - span.set_status(Status::Error { - description: e.message.clone(), - }); - } - - id.map(move |id| Self { id, payload }.to_json()) - } - - pub(crate) fn to_json(&self) -> Box { - serde_json::value::to_raw_value(self).unwrap_or_else(|err| { - tracing::debug!(%err, id = ?self.id, "failed to serialize response"); - Response::serialization_failure(self.id) - }) - } -} - -impl Serialize for Response<'_, '_, T, E> -where - T: Serialize, - E: Serialize, -{ - fn serialize(&self, serializer: S) -> Result - where - S: Serializer, - { - let mut map = serializer.serialize_map(Some(3))?; - map.serialize_entry("jsonrpc", "2.0")?; - map.serialize_entry("id", &self.id)?; - match &self.payload { - ResponsePayload::Success(result) => { - map.serialize_entry("result", result)?; - } - ResponsePayload::Failure(error) => { - map.serialize_entry("error", error)?; - } - } - map.end() - } -} - /// A JSON-RPC 2.0 response payload. /// /// This enum covers both the success and error cases of a JSON-RPC 2.0 /// response. It is used to represent the `result` and `error` fields of a /// response object. #[derive(Clone, Debug, PartialEq, Eq)] -pub enum ResponsePayload { - /// A successful response payload. - Success(Payload), - /// An error response payload. - Failure(ErrorPayload), +#[repr(transparent)] +pub struct ResponsePayload(pub Result>); + +impl From> for ResponsePayload +where + E: RpcSend, +{ + fn from(res: Result) -> Self { + match res { + Ok(payload) => Self(Ok(payload)), + Err(err) => Self(Err(ErrorPayload::internal_error_with_obj(err))), + } + } } impl ResponsePayload { /// Create a new error payload for a parse error. pub const fn parse_error() -> Self { - Self::Failure(ErrorPayload::parse_error()) + Self(Err(ErrorPayload::parse_error())) } /// Create a new error payload for an invalid request. pub const fn invalid_request() -> Self { - Self::Failure(ErrorPayload::invalid_request()) + Self(Err(ErrorPayload::invalid_request())) } /// Create a new error payload for a method not found error. pub const fn method_not_found() -> Self { - Self::Failure(ErrorPayload::method_not_found()) + Self(Err(ErrorPayload::method_not_found())) } /// Create a new error payload for an invalid params error. pub const fn invalid_params() -> Self { - Self::Failure(ErrorPayload::invalid_params()) + Self(Err(ErrorPayload::invalid_params())) } /// Create a new error payload for an internal error. pub const fn internal_error() -> Self { - Self::Failure(ErrorPayload::internal_error()) + Self(Err(ErrorPayload::internal_error())) } /// Create a new error payload for an internal error with a custom message. pub const fn internal_error_message(message: Cow<'static, str>) -> Self { - Self::Failure(ErrorPayload::internal_error_message(message)) + Self(Err(ErrorPayload::internal_error_message(message))) } /// Create a new error payload for an internal error with a custom message @@ -172,7 +64,7 @@ impl ResponsePayload { where ErrData: RpcSend, { - Self::Failure(ErrorPayload::internal_error_with_obj(data)) + Self(Err(ErrorPayload::internal_error_with_obj(data))) } /// Create a new error payload for an internal error with a custom message @@ -184,15 +76,15 @@ impl ResponsePayload { where ErrData: RpcSend, { - Self::Failure(ErrorPayload::internal_error_with_message_and_obj( + Self(Err(ErrorPayload::internal_error_with_message_and_obj( message, data, - )) + ))) } /// Fallible conversion to the successful payload. pub const fn as_success(&self) -> Option<&Payload> { match self { - Self::Success(payload) => Some(payload), + Self(Ok(payload)) => Some(payload), _ => None, } } @@ -200,19 +92,19 @@ impl ResponsePayload { /// Fallible conversion to the error object. pub const fn as_error(&self) -> Option<&ErrorPayload> { match self { - Self::Failure(payload) => Some(payload), + Self(Err(payload)) => Some(payload), _ => None, } } /// Returns `true` if the response payload is a success. pub const fn is_success(&self) -> bool { - matches!(self, Self::Success(_)) + matches!(self, Self(Ok(_))) } /// Returns `true` if the response payload is an error. pub const fn is_error(&self) -> bool { - matches!(self, Self::Failure(_)) + matches!(self, Self(Err(_))) } /// Convert a result into a response payload, by converting the error into @@ -222,8 +114,8 @@ impl ResponsePayload { T: Into>, { match res { - Ok(payload) => Self::Success(payload), - Err(err) => Self::Failure(ErrorPayload::internal_error_message(err.into())), + Ok(payload) => Self(Ok(payload)), + Err(err) => Self(Err(ErrorPayload::internal_error_message(err.into()))), } } } @@ -414,51 +306,3 @@ impl fmt::Display for ErrorPayload { ) } } - -#[cfg(test)] -mod test { - use super::*; - use crate::test_utils::assert_rv_eq; - - #[test] - fn ser_failure() { - let id = RawValue::from_string("1".to_string()).unwrap(); - let res = Response::<(), ()>::serialization_failure(&id); - assert_rv_eq( - &res, - r#"{"jsonrpc":"2.0","id":1,"error":{"code":-32700,"message":"response serialization error"}}"#, - ); - } -} - -// Some code is this file is reproduced under the terms of the MIT license. It -// originates from the `alloy` crate. The original source code can be found at -// the following URL, and the original license is included below. -// -// https://github.com/alloy-rs/alloy -// -// The MIT License (MIT) -// -// Permission is hereby granted, free of charge, to any -// person obtaining a copy of this software and associated -// documentation files (the "Software"), to deal in the -// Software without restriction, including without -// limitation the rights to use, copy, modify, merge, -// publish, distribute, sublicense, and/or sell copies of -// the Software, and to permit persons to whom the Software -// is furnished to do so, subject to the following -// conditions: -// -// The above copyright notice and this permission notice -// shall be included in all copies or substantial portions -// of the Software. -// -// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF -// ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED -// TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A -// PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT -// SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY -// CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION -// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR -// IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER -// DEALINGS IN THE SOFTWARE. diff --git a/src/types/resp/ser.rs b/src/types/resp/ser.rs new file mode 100644 index 0000000..dda923d --- /dev/null +++ b/src/types/resp/ser.rs @@ -0,0 +1,138 @@ +use crate::ResponsePayload; +use serde::{ser::SerializeMap, Serialize, Serializer}; +use serde_json::value::RawValue; + +/// Response struct. +#[derive(Debug, Clone)] +pub(crate) struct Response<'a, 'b, T, E> { + pub(crate) id: &'b RawValue, + pub(crate) payload: &'a ResponsePayload, +} + +impl Response<'_, '_, (), ()> { + /// Parse error response, used when the request is not valid JSON. + pub(crate) fn parse_error() -> Box { + Response::<(), ()> { + id: RawValue::NULL, + payload: &ResponsePayload::parse_error(), + } + .to_json() + } + + /// Method not found response, used in default fallback handler. + pub(crate) fn method_not_found(id: &RawValue) -> Box { + Response::<(), ()> { + id, + payload: &ResponsePayload::method_not_found(), + } + .to_json() + } + + /// Method not found response, used in default fallback handler. This + /// function exists to simplify notification responses, which should be + /// omitted. + pub(crate) fn maybe_method_not_found(id: Option<&RawValue>) -> Option> { + id.map(Self::method_not_found) + } + + /// Response failed to serialize + pub(crate) fn serialization_failure(id: &RawValue) -> Box { + RawValue::from_string(format!( + r#"{{"jsonrpc":"2.0","id":{},"error":{{"code":-32700,"message":"response serialization error"}}}}"#, + id.get() + )) + .expect("valid json") + } +} + +impl<'a, 'b, T, E> Response<'a, 'b, T, E> +where + T: Serialize, + E: Serialize, +{ + pub(crate) fn build_response( + id: Option<&'b RawValue>, + payload: &'a ResponsePayload, + ) -> Option> { + id.map(move |id| Self { id, payload }.to_json()) + } + + pub(crate) fn to_json(&self) -> Box { + serde_json::value::to_raw_value(self).unwrap_or_else(|err| { + tracing::debug!(%err, id = ?self.id, "failed to serialize response"); + Response::serialization_failure(self.id) + }) + } +} + +impl Serialize for Response<'_, '_, T, E> +where + T: Serialize, + E: Serialize, +{ + fn serialize(&self, serializer: S) -> Result + where + S: Serializer, + { + let mut map = serializer.serialize_map(Some(3))?; + map.serialize_entry("jsonrpc", "2.0")?; + map.serialize_entry("id", &self.id)?; + match &self.payload { + ResponsePayload(Ok(result)) => { + map.serialize_entry("result", result)?; + } + ResponsePayload(Err(error)) => { + map.serialize_entry("error", error)?; + } + } + map.end() + } +} + +#[cfg(test)] +mod test { + use super::*; + use crate::test_utils::assert_rv_eq; + + #[test] + fn ser_failure() { + let id = RawValue::from_string("1".to_string()).unwrap(); + let res = Response::<(), ()>::serialization_failure(&id); + assert_rv_eq( + &res, + r#"{"jsonrpc":"2.0","id":1,"error":{"code":-32700,"message":"response serialization error"}}"#, + ); + } +} + +// Some code is this file is reproduced under the terms of the MIT license. It +// originates from the `alloy` crate. The original source code can be found at +// the following URL, and the original license is included below. +// +// https://github.com/alloy-rs/alloy +// +// The MIT License (MIT) +// +// Permission is hereby granted, free of charge, to any +// person obtaining a copy of this software and associated +// documentation files (the "Software"), to deal in the +// Software without restriction, including without +// limitation the rights to use, copy, modify, merge, +// publish, distribute, sublicense, and/or sell copies of +// the Software, and to permit persons to whom the Software +// is furnished to do so, subject to the following +// conditions: +// +// The above copyright notice and this permission notice +// shall be included in all copies or substantial portions +// of the Software. +// +// THE SOFTWARE IS PROVIDED "AS IS", WITHOUT WARRANTY OF +// ANY KIND, EXPRESS OR IMPLIED, INCLUDING BUT NOT LIMITED +// TO THE WARRANTIES OF MERCHANTABILITY, FITNESS FOR A +// PARTICULAR PURPOSE AND NONINFRINGEMENT. IN NO EVENT +// SHALL THE AUTHORS OR COPYRIGHT HOLDERS BE LIABLE FOR ANY +// CLAIM, DAMAGES OR OTHER LIABILITY, WHETHER IN AN ACTION +// OF CONTRACT, TORT OR OTHERWISE, ARISING FROM, OUT OF OR +// IN CONNECTION WITH THE SOFTWARE OR THE USE OR OTHER +// DEALINGS IN THE SOFTWARE. From e263b34b85e2c0a274da3b074be87631282189e2 Mon Sep 17 00:00:00 2001 From: James Date: Thu, 9 Oct 2025 11:21:30 -0400 Subject: [PATCH 22/24] feat: metric for method not found --- src/metrics.rs | 27 +++++++++++++++++++++++++++ src/routes/ctx.rs | 10 ++++++++++ src/routes/handler.rs | 26 +++++++++++++------------- src/routes/mod.rs | 6 +++++- src/types/resp/payload.rs | 5 ++--- 5 files changed, 57 insertions(+), 17 deletions(-) diff --git a/src/metrics.rs b/src/metrics.rs index ce55b96..f1bab84 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -33,6 +33,11 @@ pub(crate) const ROUTER_PARSE_ERRORS: &str = "ajj.router.parse_errors"; pub(crate) const ROUTER_PARSE_ERRORS_HELP: &str = "Number of parse errors encountered by ajj router methods. This implies no response was sent."; +/// Metric for counting method not found errors. +pub(crate) const ROUTER_METHOD_NOT_FOUND: &str = "ajj.router.method_not_found"; +pub(crate) const ROUTER_METHOD_NOT_FOUND_HELP: &str = + "Number of times ajj router methods encountered a method not found error. This implies a response was sent."; + static DESCRIBE: LazyLock<()> = LazyLock::new(|| { metrics::describe_counter!(ROUTER_CALLS, metrics::Unit::Count, ROUTER_CALLS_HELP); metrics::describe_counter!(ROUTER_ERRORS, metrics::Unit::Count, ROUTER_ERRORS_HELP); @@ -56,6 +61,11 @@ static DESCRIBE: LazyLock<()> = LazyLock::new(|| { metrics::Unit::Count, ROUTER_PARSE_ERRORS_HELP ); + metrics::describe_counter!( + ROUTER_METHOD_NOT_FOUND, + metrics::Unit::Count, + ROUTER_METHOD_NOT_FOUND_HELP + ); }); /// Get or register a counter for calls to a specific service and method. @@ -168,3 +178,20 @@ pub(crate) fn record_parse_error(service_name: &'static str) { let counter = parse_errors(service_name); counter.increment(1); } + +/// Get or register a counter for method not found errors. +pub(crate) fn method_not_found_errors(service_name: &'static str, method: &str) -> Counter { + let _ = &DESCRIBE; + metrics::counter!(ROUTER_METHOD_NOT_FOUND, "service" => service_name.to_string(), "method" => method.to_string()) +} + +/// Record a method not found error. +pub(crate) fn record_method_not_found( + response_sent: bool, + service_name: &'static str, + method: &str, +) { + let counter = method_not_found_errors(service_name, method); + counter.increment(1); + record_output(response_sent, service_name, method); +} diff --git a/src/routes/ctx.rs b/src/routes/ctx.rs index bf001bb..0ff15a7 100644 --- a/src/routes/ctx.rs +++ b/src/routes/ctx.rs @@ -464,8 +464,18 @@ impl HandlerArgs { self.req.id_owned() } + /// Get the method of the JSON-RPC request. + pub fn method(&self) -> &str { + self.req.method() + } + /// Get the OpenTelemetry span name for this handler invocation. pub fn otel_span_name(&self) -> String { format!("{}/{}", self.ctx.service_name(), self.req.method()) } + + /// Get the service name for this handler invocation. + pub const fn service_name(&self) -> &'static str { + self.ctx.service_name() + } } diff --git a/src/routes/handler.rs b/src/routes/handler.rs index ffa833c..51bbd13 100644 --- a/src/routes/handler.rs +++ b/src/routes/handler.rs @@ -440,7 +440,7 @@ pub struct OutputResponsePayload { _sealed: (), } -/// Takes nothing, returns ResponsePayload +// Takes nothing, returns ResponsePayload impl Handler<(OutputResponsePayload,), S> for F where F: FnOnce() -> Fut + Clone + Send + Sync + 'static, @@ -455,7 +455,7 @@ where } } -/// Takes Ctx, returns ResponsePayload +// Takes Ctx, returns ResponsePayload impl Handler<(OutputResponsePayload, HandlerCtx), S> for F where F: FnOnce(HandlerCtx) -> Fut + Clone + Send + Sync + 'static, @@ -470,7 +470,7 @@ where } } -/// Takes Params, returns ResponsePayload +// Takes Params, returns ResponsePayload impl Handler<(OutputResponsePayload, PhantomParams), S> for F where @@ -487,7 +487,7 @@ where } } -/// Takes Params, returns ResponsePayload +// Takes Params, returns ResponsePayload impl Handler<(OutputResponsePayload, Params), S> for F where F: FnOnce(Params) -> Fut + Clone + Send + Sync + 'static, @@ -503,7 +503,7 @@ where } } -/// Takes State, returns ResponsePayload +// Takes State, returns ResponsePayload impl Handler<(OutputResponsePayload, PhantomState), S> for F where F: FnOnce(S) -> Fut + Clone + Send + Sync + 'static, @@ -519,7 +519,7 @@ where } } -/// Takes State, returns ResponsePayload +// Takes State, returns ResponsePayload impl Handler<(OutputResponsePayload, State), S> for F where F: FnOnce(State) -> Fut + Clone + Send + Sync + 'static, @@ -535,7 +535,7 @@ where } } -/// Takes Ctx, Params, returns ResponsePayload +// Takes Ctx, Params, returns ResponsePayload impl Handler<(OutputResponsePayload, HandlerCtx, PhantomParams), S> for F where @@ -552,7 +552,7 @@ where } } -/// Takes Ctx, Params, returns ResponsePayload +// Takes Ctx, Params, returns ResponsePayload impl Handler<(OutputResponsePayload, HandlerCtx, Params), S> for F where @@ -569,7 +569,7 @@ where } } -/// Takes Params, State, returns ResponsePayload +// Takes Params, State, returns ResponsePayload impl Handler<(OutputResponsePayload, Input, S), S> for F where F: FnOnce(Input, S) -> Fut + Clone + Send + Sync + 'static, @@ -586,7 +586,7 @@ where } } -/// Takes Ctx, State, returns ResponsePayload +// Takes Ctx, State, returns ResponsePayload impl Handler<(OutputResponsePayload, HandlerCtx, PhantomState), S> for F where @@ -603,7 +603,7 @@ where } } -/// Takes Ctx, State, returns ResponsePayload +// Takes Ctx, State, returns ResponsePayload impl Handler<(OutputResponsePayload, HandlerCtx, State), S> for F where F: FnOnce(HandlerCtx, State) -> Fut + Clone + Send + Sync + 'static, @@ -619,7 +619,7 @@ where } } -/// Takes Ctx, Params, State, returns ResponsePayload +// Takes Ctx, Params, State, returns ResponsePayload impl Handler<(OutputResponsePayload, HandlerCtx, Input, S), S> for F where @@ -637,7 +637,7 @@ where } } -/// Takes nothing, returns Result +// Takes nothing, returns Result impl Handler<(OutputResult,), S> for F where F: FnOnce() -> Fut + Clone + Send + Sync + 'static, diff --git a/src/routes/mod.rs b/src/routes/mod.rs index 07af8e8..9493716 100644 --- a/src/routes/mod.rs +++ b/src/routes/mod.rs @@ -52,8 +52,12 @@ impl Route { pub(crate) fn default_fallback() -> Self { Self::new(tower::service_fn(|args: HandlerArgs| async { let id = args.id_owned(); + crate::metrics::record_method_not_found( + id.is_some(), + args.service_name(), + args.method(), + ); drop(args); // no longer needed - Ok(Response::maybe_method_not_found(id.as_deref())) })) } diff --git a/src/types/resp/payload.rs b/src/types/resp/payload.rs index 58f92e0..8b19c52 100644 --- a/src/types/resp/payload.rs +++ b/src/types/resp/payload.rs @@ -8,9 +8,8 @@ const INTERNAL_ERROR: Cow<'_, str> = Cow::Borrowed("Internal error"); /// A JSON-RPC 2.0 response payload. /// -/// This enum covers both the success and error cases of a JSON-RPC 2.0 -/// response. It is used to represent the `result` and `error` fields of a -/// response object. +/// This is a thin wrapper around a [`Result`] type containing either +/// the successful payload or an error payload. #[derive(Clone, Debug, PartialEq, Eq)] #[repr(transparent)] pub struct ResponsePayload(pub Result>); From bda33c7af0f4a6492d8baecf313bd44705945ef2 Mon Sep 17 00:00:00 2001 From: James Date: Fri, 10 Oct 2025 11:46:24 -0400 Subject: [PATCH 23/24] feat: record completed calls and active calls --- src/metrics.rs | 67 +++++++++++++++++++++++++++++++++++++++++++------- src/router.rs | 13 ++++++++++ 2 files changed, 71 insertions(+), 9 deletions(-) diff --git a/src/metrics.rs b/src/metrics.rs index f1bab84..e6acdcc 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -1,7 +1,6 @@ +use metrics::{counter, gauge, Counter, Gauge}; use std::sync::LazyLock; -use metrics::Counter; - /// Metric name for counting router calls. pub(crate) const ROUTER_CALLS: &str = "ajj.router.calls"; pub(crate) const ROUTER_CALLS_HELP: &str = @@ -38,6 +37,14 @@ pub(crate) const ROUTER_METHOD_NOT_FOUND: &str = "ajj.router.method_not_found"; pub(crate) const ROUTER_METHOD_NOT_FOUND_HELP: &str = "Number of times ajj router methods encountered a method not found error. This implies a response was sent."; +/// Metric for tracking active calls. +pub(crate) const ACTIVE_CALLS: &str = "ajj.router.active_calls"; +pub(crate) const ACTIVE_CALLS_HELP: &str = "Number of active calls being processed"; + +/// Metric for tracking completed calls. +pub(crate) const COMPLETED_CALLS: &str = "ajj.router.completed_calls"; +pub(crate) const COMPLETED_CALLS_HELP: &str = "Number of completed calls handled"; + static DESCRIBE: LazyLock<()> = LazyLock::new(|| { metrics::describe_counter!(ROUTER_CALLS, metrics::Unit::Count, ROUTER_CALLS_HELP); metrics::describe_counter!(ROUTER_ERRORS, metrics::Unit::Count, ROUTER_ERRORS_HELP); @@ -66,12 +73,14 @@ static DESCRIBE: LazyLock<()> = LazyLock::new(|| { metrics::Unit::Count, ROUTER_METHOD_NOT_FOUND_HELP ); + metrics::describe_gauge!(ACTIVE_CALLS, metrics::Unit::Count, ACTIVE_CALLS_HELP); + metrics::describe_counter!(COMPLETED_CALLS, metrics::Unit::Count, COMPLETED_CALLS_HELP); }); /// Get or register a counter for calls to a specific service and method. pub(crate) fn calls(service_name: &'static str, method: &str) -> Counter { let _ = &DESCRIBE; - metrics::counter!( + counter!( ROUTER_CALLS, "service" => service_name.to_string(), "method" => method.to_string() @@ -82,12 +91,13 @@ pub(crate) fn calls(service_name: &'static str, method: &str) -> Counter { pub(crate) fn record_call(service_name: &'static str, method: &str) { let counter = calls(service_name, method); counter.increment(1); + increment_active_calls(service_name, method); } /// Get or register a counter for errors from a specific service and method. pub(crate) fn errors(service_name: &'static str, method: &str) -> Counter { let _ = &DESCRIBE; - metrics::counter!( + counter!( ROUTER_ERRORS, "service" => service_name.to_string(), "method" => method.to_string() @@ -103,7 +113,7 @@ pub(crate) fn record_execution_error(service_name: &'static str, method: &str) { /// Get or register a counter for successes from a specific service and method. pub(crate) fn successes(service_name: &'static str, method: &str) -> Counter { let _ = &DESCRIBE; - metrics::counter!( + counter!( ROUTER_SUCCESSES, "service" => service_name.to_string(), "method" => method.to_string() @@ -129,7 +139,7 @@ pub(crate) fn record_execution(success: bool, service_name: &'static str, method /// Get or register a counter for responses from a specific service and method. pub(crate) fn responses(service_name: &'static str, method: &str) -> Counter { let _ = &DESCRIBE; - metrics::counter!( + counter!( ROUTER_RESPONSES, "service" => service_name.to_string(), "method" => method.to_string() @@ -145,7 +155,7 @@ pub(crate) fn record_response(service_name: &'static str, method: &str) { /// Get or register a counter for omitted notification responses from a specific service and method. pub(crate) fn response_omitted(service_name: &'static str, method: &str) -> Counter { let _ = &DESCRIBE; - metrics::counter!( + counter!( ROUTER_NOTIFICATION_RESPONSE_OMITTED, "service" => service_name.to_string(), "method" => method.to_string() @@ -165,12 +175,14 @@ pub(crate) fn record_output(response_sent: bool, service_name: &'static str, met } else { record_response_omitted(service_name, method); } + record_completed_call(service_name, method); + decrement_active_calls(service_name, method); } // Get or register a counter for parse errors. pub(crate) fn parse_errors(service_name: &'static str) -> Counter { let _ = &DESCRIBE; - metrics::counter!(ROUTER_PARSE_ERRORS, "service" => service_name.to_string()) + counter!(ROUTER_PARSE_ERRORS, "service" => service_name.to_string()) } /// Record a parse error. @@ -182,7 +194,7 @@ pub(crate) fn record_parse_error(service_name: &'static str) { /// Get or register a counter for method not found errors. pub(crate) fn method_not_found_errors(service_name: &'static str, method: &str) -> Counter { let _ = &DESCRIBE; - metrics::counter!(ROUTER_METHOD_NOT_FOUND, "service" => service_name.to_string(), "method" => method.to_string()) + counter!(ROUTER_METHOD_NOT_FOUND, "service" => service_name.to_string(), "method" => method.to_string()) } /// Record a method not found error. @@ -195,3 +207,40 @@ pub(crate) fn record_method_not_found( counter.increment(1); record_output(response_sent, service_name, method); } + +/// Get or register a gauge for active calls to a specific service. +pub(crate) fn active_calls(service_name: &'static str, method: &str) -> Gauge { + let _ = &DESCRIBE; + gauge!(ACTIVE_CALLS, "service" => service_name.to_string(), "method" => method.to_string()) +} + +/// Increment the active calls gauge for a specific service. +pub(crate) fn increment_active_calls(service_name: &'static str, method: &str) { + let _ = &DESCRIBE; + let gauge = active_calls(service_name, method); + gauge.increment(1); +} + +/// Decrement the active calls gauge for a specific service. +pub(crate) fn decrement_active_calls(service_name: &'static str, method: &str) { + let _ = &DESCRIBE; + let gauge = active_calls(service_name, method); + gauge.decrement(1); +} + +/// Get or register a counter for completed calls to a specific service. +pub(crate) fn completed_calls(service_name: &'static str, method: &str) -> Counter { + let _ = &DESCRIBE; + counter!( + COMPLETED_CALLS, + "service" => service_name.to_string(), + "method" => method.to_string() + ) +} + +/// Record a completed call to a specific service and method. +pub(crate) fn record_completed_call(service_name: &'static str, method: &str) { + let _ = &DESCRIBE; + let counter = completed_calls(service_name, method); + counter.increment(1); +} diff --git a/src/router.rs b/src/router.rs index 884066f..c53b97f 100644 --- a/src/router.rs +++ b/src/router.rs @@ -111,6 +111,19 @@ where self.inner.service_name() } + /// Set the OpenTelemetry service name for this router, overriding any + /// existing name. + /// + /// ## Note + /// + /// Routers wrap an `Arc`. If multiple references to the router exist, + /// this method will clone the inner state before setting the name. + pub fn set_name(self, service_name: &'static str) -> Self { + tap_inner!(self, mut this => { + this.service_name = Some(service_name); + }) + } + /// If this router is the only reference to its inner state, return the /// inner state. Otherwise, clone the inner state and return the clone. fn into_inner(self) -> RouterInner { From ec245bac8ec3b6d2de20c81ccea1411dd2722b3b Mon Sep 17 00:00:00 2001 From: James Date: Fri, 10 Oct 2025 11:48:21 -0400 Subject: [PATCH 24/24] chore: make some fns more priv --- src/metrics.rs | 34 +++++++++++++++++----------------- 1 file changed, 17 insertions(+), 17 deletions(-) diff --git a/src/metrics.rs b/src/metrics.rs index e6acdcc..1530066 100644 --- a/src/metrics.rs +++ b/src/metrics.rs @@ -78,7 +78,7 @@ static DESCRIBE: LazyLock<()> = LazyLock::new(|| { }); /// Get or register a counter for calls to a specific service and method. -pub(crate) fn calls(service_name: &'static str, method: &str) -> Counter { +fn calls(service_name: &'static str, method: &str) -> Counter { let _ = &DESCRIBE; counter!( ROUTER_CALLS, @@ -95,7 +95,7 @@ pub(crate) fn record_call(service_name: &'static str, method: &str) { } /// Get or register a counter for errors from a specific service and method. -pub(crate) fn errors(service_name: &'static str, method: &str) -> Counter { +fn errors(service_name: &'static str, method: &str) -> Counter { let _ = &DESCRIBE; counter!( ROUTER_ERRORS, @@ -105,13 +105,13 @@ pub(crate) fn errors(service_name: &'static str, method: &str) -> Counter { } /// Record an error from a specific service and method. -pub(crate) fn record_execution_error(service_name: &'static str, method: &str) { +fn record_execution_error(service_name: &'static str, method: &str) { let counter = errors(service_name, method); counter.increment(1); } /// Get or register a counter for successes from a specific service and method. -pub(crate) fn successes(service_name: &'static str, method: &str) -> Counter { +fn successes(service_name: &'static str, method: &str) -> Counter { let _ = &DESCRIBE; counter!( ROUTER_SUCCESSES, @@ -121,7 +121,7 @@ pub(crate) fn successes(service_name: &'static str, method: &str) -> Counter { } /// Record a success from a specific service and method. -pub(crate) fn record_execution_success(service_name: &'static str, method: &str) { +fn record_execution_success(service_name: &'static str, method: &str) { let counter = successes(service_name, method); counter.increment(1); } @@ -137,7 +137,7 @@ pub(crate) fn record_execution(success: bool, service_name: &'static str, method } /// Get or register a counter for responses from a specific service and method. -pub(crate) fn responses(service_name: &'static str, method: &str) -> Counter { +fn responses(service_name: &'static str, method: &str) -> Counter { let _ = &DESCRIBE; counter!( ROUTER_RESPONSES, @@ -147,13 +147,13 @@ pub(crate) fn responses(service_name: &'static str, method: &str) -> Counter { } /// Record a response from a specific service and method. -pub(crate) fn record_response(service_name: &'static str, method: &str) { +fn record_response(service_name: &'static str, method: &str) { let counter = responses(service_name, method); counter.increment(1); } /// Get or register a counter for omitted notification responses from a specific service and method. -pub(crate) fn response_omitted(service_name: &'static str, method: &str) -> Counter { +fn response_omitted(service_name: &'static str, method: &str) -> Counter { let _ = &DESCRIBE; counter!( ROUTER_NOTIFICATION_RESPONSE_OMITTED, @@ -163,7 +163,7 @@ pub(crate) fn response_omitted(service_name: &'static str, method: &str) -> Coun } /// Record an omitted notification response from a specific service and method. -pub(crate) fn record_response_omitted(service_name: &'static str, method: &str) { +fn record_response_omitted(service_name: &'static str, method: &str) { let counter = response_omitted(service_name, method); counter.increment(1); } @@ -179,8 +179,8 @@ pub(crate) fn record_output(response_sent: bool, service_name: &'static str, met decrement_active_calls(service_name, method); } -// Get or register a counter for parse errors. -pub(crate) fn parse_errors(service_name: &'static str) -> Counter { +/// Get or register a counter for parse errors. +fn parse_errors(service_name: &'static str) -> Counter { let _ = &DESCRIBE; counter!(ROUTER_PARSE_ERRORS, "service" => service_name.to_string()) } @@ -192,7 +192,7 @@ pub(crate) fn record_parse_error(service_name: &'static str) { } /// Get or register a counter for method not found errors. -pub(crate) fn method_not_found_errors(service_name: &'static str, method: &str) -> Counter { +fn method_not_found_errors(service_name: &'static str, method: &str) -> Counter { let _ = &DESCRIBE; counter!(ROUTER_METHOD_NOT_FOUND, "service" => service_name.to_string(), "method" => method.to_string()) } @@ -209,27 +209,27 @@ pub(crate) fn record_method_not_found( } /// Get or register a gauge for active calls to a specific service. -pub(crate) fn active_calls(service_name: &'static str, method: &str) -> Gauge { +fn active_calls(service_name: &'static str, method: &str) -> Gauge { let _ = &DESCRIBE; gauge!(ACTIVE_CALLS, "service" => service_name.to_string(), "method" => method.to_string()) } /// Increment the active calls gauge for a specific service. -pub(crate) fn increment_active_calls(service_name: &'static str, method: &str) { +fn increment_active_calls(service_name: &'static str, method: &str) { let _ = &DESCRIBE; let gauge = active_calls(service_name, method); gauge.increment(1); } /// Decrement the active calls gauge for a specific service. -pub(crate) fn decrement_active_calls(service_name: &'static str, method: &str) { +fn decrement_active_calls(service_name: &'static str, method: &str) { let _ = &DESCRIBE; let gauge = active_calls(service_name, method); gauge.decrement(1); } /// Get or register a counter for completed calls to a specific service. -pub(crate) fn completed_calls(service_name: &'static str, method: &str) -> Counter { +fn completed_calls(service_name: &'static str, method: &str) -> Counter { let _ = &DESCRIBE; counter!( COMPLETED_CALLS, @@ -239,7 +239,7 @@ pub(crate) fn completed_calls(service_name: &'static str, method: &str) -> Count } /// Record a completed call to a specific service and method. -pub(crate) fn record_completed_call(service_name: &'static str, method: &str) { +fn record_completed_call(service_name: &'static str, method: &str) { let _ = &DESCRIBE; let counter = completed_calls(service_name, method); counter.increment(1);