From f4b93b6d7211501d9471fb5e53474d473da3a4f8 Mon Sep 17 00:00:00 2001 From: evalir Date: Wed, 2 Jul 2025 12:23:54 +0200 Subject: [PATCH 1/4] fix(tracing): actually use span on permissioning We were not actually entering the span, so the logs show up pretty empty. --- src/perms/middleware.rs | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/perms/middleware.rs b/src/perms/middleware.rs index bc61291..d36a4fd 100644 --- a/src/perms/middleware.rs +++ b/src/perms/middleware.rs @@ -164,6 +164,8 @@ where permissioning_error = tracing::field::Empty, ); + let guard = span.enter(); + info!("builder permissioning check started"); // Check if the sub is in the header. @@ -187,6 +189,8 @@ where info!("builder permissioned successfully"); + drop(guard); + this.inner.call(req).await }) } From ea74130e65e2e09aa13144fe7eb90064772b9ac7 Mon Sep 17 00:00:00 2001 From: evalir Date: Wed, 2 Jul 2025 12:54:02 +0200 Subject: [PATCH 2/4] chore: improve logging, add more info --- src/perms/middleware.rs | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/perms/middleware.rs b/src/perms/middleware.rs index d36a4fd..2f963fe 100644 --- a/src/perms/middleware.rs +++ b/src/perms/middleware.rs @@ -160,7 +160,9 @@ where "builder::permissioning", builder = tracing::field::Empty, permissioned_builder = this.builders.current_builder().sub(), + requesting_builder = tracing::field::Empty, current_slot = this.builders.calc().current_slot(), + current_timepoint_within_slot = this.builders.calc().current_timepoint_within_slot(), permissioning_error = tracing::field::Empty, ); @@ -172,15 +174,17 @@ where let sub = match validate_header_sub(req.headers().get("x-jwt-claim-sub")) { Ok(sub) => sub, Err(err) => { - info!(api_err = %err.1.message, "permission denied"); span.record("permissioning_error", err.1.message); + info!(api_err = %err.1.message, "permission denied"); return Ok(err.into_response()); } }; + span.record("requesting_builder", sub); + if let Err(err) = this.builders.is_builder_permissioned(sub) { - info!(api_err = %err, "permission denied"); span.record("permissioning_error", err.to_string()); + info!(api_err = %err, "permission denied"); let hint = builder_permissioning_hint(&err); From 70264a2280d2c69d811343b327432c113e78806f Mon Sep 17 00:00:00 2001 From: evalir Date: Wed, 2 Jul 2025 13:03:28 +0200 Subject: [PATCH 3/4] chore: make `BuilderPermissionError more useful --- src/perms/builders.rs | 8 ++++---- src/perms/middleware.rs | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/src/perms/builders.rs b/src/perms/builders.rs index 6c73fe3..40d2197 100644 --- a/src/perms/builders.rs +++ b/src/perms/builders.rs @@ -20,7 +20,7 @@ fn now() -> u64 { } /// Possible errors when permissioning a builder. -#[derive(Debug, thiserror::Error, Clone, Copy, PartialEq, Eq)] +#[derive(Debug, thiserror::Error, Clone, PartialEq, Eq)] pub enum BuilderPermissionError { /// Action attempt too early. #[error("action attempt too early")] @@ -31,8 +31,8 @@ pub enum BuilderPermissionError { ActionAttemptTooLate, /// Builder not permissioned for this slot. - #[error("builder not permissioned for this slot")] - NotPermissioned, + #[error("builder not permissioned for this slot: requesting builder {0}, permissioned builder {1}")] + NotPermissioned(String, String), } /// An individual builder. @@ -178,7 +178,7 @@ impl Builders { permissioned_builder = %self.current_builder().sub, "Builder not permissioned for this slot" ); - return Err(BuilderPermissionError::NotPermissioned); + return Err(BuilderPermissionError::NotPermissioned(sub.to_owned(), self.current_builder().sub.to_owned())); } Ok(()) diff --git a/src/perms/middleware.rs b/src/perms/middleware.rs index 2f963fe..86c9eed 100644 --- a/src/perms/middleware.rs +++ b/src/perms/middleware.rs @@ -226,7 +226,7 @@ const fn builder_permissioning_hint( crate::perms::BuilderPermissionError::ActionAttemptTooLate => { Some("Action attempted too late in the slot.") } - crate::perms::BuilderPermissionError::NotPermissioned => { + crate::perms::BuilderPermissionError::NotPermissioned(_, _) => { Some("Builder is not permissioned for this slot.") } } From 31a0864d6fefb6de697c3d1239298b3119cabe8e Mon Sep 17 00:00:00 2001 From: evalir Date: Wed, 2 Jul 2025 13:07:37 +0200 Subject: [PATCH 4/4] chore: clippy --- src/perms/builders.rs | 9 +++++++-- src/perms/middleware.rs | 3 ++- 2 files changed, 9 insertions(+), 3 deletions(-) diff --git a/src/perms/builders.rs b/src/perms/builders.rs index 40d2197..3cc2fa7 100644 --- a/src/perms/builders.rs +++ b/src/perms/builders.rs @@ -31,7 +31,9 @@ pub enum BuilderPermissionError { ActionAttemptTooLate, /// Builder not permissioned for this slot. - #[error("builder not permissioned for this slot: requesting builder {0}, permissioned builder {1}")] + #[error( + "builder not permissioned for this slot: requesting builder {0}, permissioned builder {1}" + )] NotPermissioned(String, String), } @@ -178,7 +180,10 @@ impl Builders { permissioned_builder = %self.current_builder().sub, "Builder not permissioned for this slot" ); - return Err(BuilderPermissionError::NotPermissioned(sub.to_owned(), self.current_builder().sub.to_owned())); + return Err(BuilderPermissionError::NotPermissioned( + sub.to_owned(), + self.current_builder().sub.to_owned(), + )); } Ok(()) diff --git a/src/perms/middleware.rs b/src/perms/middleware.rs index 86c9eed..0cb899d 100644 --- a/src/perms/middleware.rs +++ b/src/perms/middleware.rs @@ -162,7 +162,8 @@ where permissioned_builder = this.builders.current_builder().sub(), requesting_builder = tracing::field::Empty, current_slot = this.builders.calc().current_slot(), - current_timepoint_within_slot = this.builders.calc().current_timepoint_within_slot(), + current_timepoint_within_slot = + this.builders.calc().current_timepoint_within_slot(), permissioning_error = tracing::field::Empty, );