Update telemetry

src/routes/admin.rs

@@ -25,7 +25,7 @@ pub enum AdminError {
     #[error("Trying to access admin dashboard without authentication.")]
     NotAuthenticated,
     #[error("Updating password failed.")]
-    ChangePassword(String),
+    ChangePassword(anyhow::Error),
     #[error("Could not publish newsletter.")]
     Publish(#[source] anyhow::Error),
     #[error("The idempotency key was invalid.")]

src/routes/admin/change_password.rs

@@ -31,16 +31,16 @@ pub async fn change_password(
         password: form.current_password,
     };
     if form.new_password.expose_secret() != form.new_password_check.expose_secret() {
-        Err(AdminError::ChangePassword(
-            "You entered two different passwords - the field values must match.".to_string(),
-        )
+        Err(AdminError::ChangePassword(anyhow::anyhow!(
+            "You entered two different passwords - the field values must match."
+        ))
         .into())
     } else if let Err(e) = validate_credentials(credentials, &connection_pool).await {
         match e {
             AuthError::UnexpectedError(error) => Err(AdminError::UnexpectedError(error).into()),
-            AuthError::InvalidCredentials(_) => Err(AdminError::ChangePassword(
-                "The current password is incorrect.".to_string(),
-            )
+            AuthError::InvalidCredentials(_) => Err(AdminError::ChangePassword(anyhow::anyhow!(
+                "The current password is incorrect."
+            ))
             .into()),
         }
     } else if let Err(e) = verify_password(form.new_password.expose_secret()) {
@@ -48,7 +48,7 @@ pub async fn change_password(
     } else {
         authentication::change_password(user_id, form.new_password, &connection_pool)
             .await
-            .map_err(|e| AdminError::ChangePassword(e.to_string()))?;
+            .map_err(AdminError::ChangePassword)?;
         let template = MessageTemplate::Success {
             message: "Your password has been changed.".to_string(),
         };
@@ -56,9 +56,9 @@ pub async fn change_password(
     }
 }
 
-fn verify_password(password: &str) -> Result<(), String> {
+fn verify_password(password: &str) -> Result<(), anyhow::Error> {
     if password.len() < 12 || password.len() > 128 {
-        return Err("The password must contain between 12 and 128 characters.".into());
+        anyhow::bail!("The password must contain between 12 and 128 characters.");
     }
     Ok(())
 }

src/routes/admin/dashboard.rs

@@ -57,6 +57,7 @@ pub async fn admin_dashboard(
     Ok(Html(template.render().unwrap()).into_response())
 }
 
+#[tracing::instrument("Computing dashboard stats", skip_all)]
 async fn get_stats(connection_pool: &PgPool) -> Result<DashboardStats, anyhow::Error> {
     let subscribers =
         sqlx::query_scalar!("SELECT count(*) FROM subscriptions WHERE status = 'confirmed'")

src/routes/admin/newsletters.rs

@@ -1,5 +1,3 @@
-use std::fmt::Display;
-
 use crate::{
     authentication::AuthenticatedUser,
     idempotency::{IdempotencyKey, save_response, try_processing},
@@ -15,6 +13,7 @@ use axum::{
     response::{Html, IntoResponse, Response},
 };
 use sqlx::{Executor, Postgres, Transaction};
+use std::fmt::Display;
 use uuid::Uuid;
 
 #[derive(serde::Deserialize)]
@@ -25,13 +24,14 @@ pub struct BodyData {
     idempotency_key: String,
 }
 
-#[tracing::instrument(skip_all)]
+#[tracing::instrument(name = "Creating newsletter isue", skip_all, fields(issue_id = tracing::field::Empty))]
 pub async fn insert_newsletter_issue(
     transaction: &mut Transaction<'static, Postgres>,
     title: &str,
     email_template: &dyn EmailTemplate,
 ) -> Result<Uuid, sqlx::Error> {
     let newsletter_issue_id = Uuid::new_v4();
+    tracing::Span::current().record("issue_id", newsletter_issue_id.to_string());
     let query = sqlx::query!(
         r#"
         INSERT INTO newsletter_issues (
@@ -48,6 +48,7 @@ pub async fn insert_newsletter_issue(
     Ok(newsletter_issue_id)
 }
 
+#[derive(Debug)]
 pub enum EmailType {
     NewPost,
     Newsletter,
@@ -62,7 +63,7 @@ impl Display for EmailType {
     }
 }
 
-#[tracing::instrument(skip_all)]
+#[tracing::instrument(name = "Adding new task to queue", skip(transaction))]
 pub async fn enqueue_delivery_tasks(
     transaction: &mut Transaction<'static, Postgres>,
     newsletter_issue_id: Uuid,
@@ -87,7 +88,7 @@ pub async fn enqueue_delivery_tasks(
     Ok(())
 }
 
-#[tracing::instrument(name = "Publishing a newsletter", skip(connection_pool, form))]
+#[tracing::instrument(name = "Publishing a newsletter", skip_all, fields(title = %form.title))]
 pub async fn publish_newsletter(
     State(AppState {
         connection_pool,
@@ -134,12 +135,12 @@ pub async fn publish_newsletter(
     Ok(response)
 }
 
-fn validate_form(form: &BodyData) -> Result<(), &'static str> {
+fn validate_form(form: &BodyData) -> Result<(), anyhow::Error> {
     if form.title.is_empty() {
-        return Err("The title was empty.");
+        anyhow::bail!("The title was empty.");
     }
     if form.html.is_empty() || form.text.is_empty() {
-        return Err("The content was empty.");
+        anyhow::bail!("The content was empty.");
     }
     Ok(())
 }

src/routes/admin/posts.rs

@@ -33,7 +33,11 @@ fn validate_form(form: &CreatePostForm) -> Result<(), anyhow::Error> {
     }
 }
 
-#[tracing::instrument(name = "Creating a post", skip(connection_pool, form))]
+#[tracing::instrument(
+    name = "Publishing new blog post",
+    skip(connection_pool, base_url, form)
+    fields(title = %form.title)
+)]
 pub async fn create_post(
     State(AppState {
         connection_pool,
@@ -79,10 +83,7 @@ pub async fn create_post(
     Ok(response)
 }
 
-#[tracing::instrument(
-    name = "Saving new post in the database",
-    skip(transaction, title, content, author)
-)]
+#[tracing::instrument(name = "Saving new blog post in the database", skip_all)]
 pub async fn insert_post(
     transaction: &mut Transaction<'static, Postgres>,
     title: &str,
@@ -105,10 +106,7 @@ pub async fn insert_post(
     Ok(post_id)
 }
 
-#[tracing::instrument(
-    name = "Creating newsletter for new post",
-    skip(transaction, post_title, post_id)
-)]
+#[tracing::instrument(name = "Creating newsletter for new post", skip_all)]
 pub async fn create_newsletter(
     transaction: &mut Transaction<'static, Postgres>,
     base_url: &str,

src/routes/admin/subscribers.rs

@@ -15,6 +15,10 @@ use uuid::Uuid;
 
 const SUBS_PER_PAGE: i64 = 5;
 
+#[tracing::instrument(
+    name = "Retrieving most recent subscribers from database",
+    skip(connection_pool)
+)]
 pub async fn get_subscribers_page(
     State(AppState {
         connection_pool, ..
@@ -38,34 +42,52 @@ pub async fn get_subscribers_page(
     Ok(Html(template.render().unwrap()).into_response())
 }
 
+#[tracing::instrument(
+    name = "Deleting subscriber from database",
+    skip(connection_pool),
+    fields(email=tracing::field::Empty)
+)]
 pub async fn delete_subscriber(
     State(AppState {
         connection_pool, ..
     }): State<AppState>,
     Path(subscriber_id): Path<Uuid>,
 ) -> Result<Response, AppError> {
-    let res = sqlx::query!("DELETE FROM subscriptions WHERE id = $1", subscriber_id)
-        .execute(&connection_pool)
-        .await
-        .context("Failed to delete subscriber from database.")
-        .map_err(AppError::unexpected_message)?;
-    if res.rows_affected() > 1 {
+    let res = sqlx::query!(
+        "DELETE FROM subscriptions WHERE id = $1 RETURNING email",
+        subscriber_id
+    )
+    .fetch_optional(&connection_pool)
+    .await
+    .context("Failed to delete subscriber from database.")
+    .map_err(AppError::unexpected_message)?;
+    if let Some(record) = res {
+        tracing::Span::current().record("email", tracing::field::display(&record.email));
+        let template = MessageTemplate::Success {
+            message: format!(
+                "The subscriber with email '{}' has been deleted.",
+                record.email
+            ),
+        };
+        Ok(template.render().unwrap().into_response())
+    } else {
         Err(AppError::unexpected_message(anyhow::anyhow!(
             "We could not find the subscriber in the database."
         )))
-    } else {
-        let template = MessageTemplate::Success {
-            message: "The subscriber has been deleted.".into(),
-        };
-        Ok(template.render().unwrap().into_response())
     }
 }
 
+#[tracing::instrument(
+    name = "Retrieving next subscribers in database",
+    skip(connection_pool),
+    fields(offset = tracing::field::Empty)
+)]
 pub async fn get_subs(
     connection_pool: &PgPool,
     page: i64,
 ) -> Result<Vec<SubscriberEntry>, sqlx::Error> {
     let offset = (page - 1) * SUBS_PER_PAGE;
+    tracing::Span::current().record("offset", tracing::field::display(&offset));
     let subscribers = sqlx::query_as!(
         SubscriberEntry,
         "SELECT * FROM subscriptions ORDER BY subscribed_at DESC LIMIT $1 OFFSET $2",

src/routes/health_check.rs

@@ -1,5 +1,5 @@
-use axum::{http::StatusCode, response::IntoResponse};
+use axum::http::StatusCode;
 
-pub async fn health_check() -> impl IntoResponse {
+pub async fn health_check() -> StatusCode {
     StatusCode::OK
 }

src/routes/home.rs

@@ -1,8 +1,7 @@
-use askama::Template;
-use axum::response::Html;
+use crate::templates::{HomeTemplate, HtmlTemplate};
+use axum::response::{IntoResponse, Response};
 
-use crate::templates::HomeTemplate;
-
-pub async fn home() -> Html<String> {
-    Html(HomeTemplate.render().unwrap())
+pub async fn home() -> Response {
+    let template = HtmlTemplate(HomeTemplate);
+    template.into_response()
 }

src/routes/login.rs

@@ -35,6 +35,7 @@ pub async fn get_login(session: TypedSession) -> Result<Response, AppError> {
     }
 }
 
+#[tracing::instrument(name = "Authenticating user", skip_all, fields(name = %form.username))]
 pub async fn post_login(
     session: TypedSession,
     State(AppState {

src/routes/posts.rs

@@ -15,6 +15,7 @@ use uuid::Uuid;
 
 const NUM_PER_PAGE: i64 = 3;
 
+#[tracing::instrument(name = "Fetching most recent posts from database", skip_all)]
 pub async fn list_posts(
     State(AppState {
         connection_pool, ..
@@ -67,6 +68,7 @@ pub struct PostParams {
     origin: Option<Uuid>,
 }
 
+#[tracing::instrument(name = "Fetching post from database", skip(connection_pool, origin))]
 pub async fn see_post(
     State(AppState {
         connection_pool, ..
@@ -94,6 +96,7 @@ pub async fn see_post(
     }
 }
 
+#[tracing::instrument(name = "Mark email notification as opened", skip(connection_pool))]
 async fn mark_email_as_opened(connection_pool: &PgPool, email_id: Uuid) -> Result<(), AppError> {
     sqlx::query!(
         "UPDATE notifications_delivered SET opened = TRUE WHERE email_id = $1",
@@ -129,6 +132,7 @@ pub struct LoadMoreParams {
     page: i64,
 }
 
+#[tracing::instrument(name = "Fetching next posts in the database", skip(connection_pool))]
 pub async fn load_more(
     State(AppState {
         connection_pool, ..

src/routes/subscriptions.rs

@@ -19,9 +19,9 @@ use uuid::Uuid;
 
 #[tracing::instrument(
     name = "Adding a new subscriber",
-    skip(connection_pool, email_client, base_url, form),
+    skip_all,
     fields(
-        subscriber_email = %form.email,
+        email = %form.email,
     )
 )]
 pub async fn subscribe(
@@ -72,10 +72,7 @@ pub async fn subscribe(
     Ok(Html(template.render().unwrap()).into_response())
 }
 
-#[tracing::instrument(
-    name = "Saving new subscriber details in the database",
-    skip(transaction, new_subscriber)
-)]
+#[tracing::instrument(name = "Saving new subscriber details in the database", skip_all)]
 pub async fn insert_subscriber(
     transaction: &mut Transaction<'_, Postgres>,
     new_subscriber: &NewSubscriber,
@@ -123,10 +120,7 @@ async fn store_token(
     Ok(())
 }
 
-#[tracing::instrument(
-    name = "Send a confirmation email to a new subscriber",
-    skip(email_client, new_subscriber, base_url, subscription_token)
-)]
+#[tracing::instrument(name = "Send confirmation email to the new subscriber", skip_all)]
 pub async fn send_confirmation_email(
     email_client: &EmailClient,
     new_subscriber: &NewSubscriber,

src/routes/subscriptions_confirm.rs

@@ -1,48 +1,39 @@
 use crate::{
-    routes::{Query, generate_token},
+    routes::{AppError, Query, generate_token, not_found_html},
     startup::AppState,
-    templates::ConfirmTemplate,
+    templates::{ConfirmTemplate, HtmlTemplate},
 };
-use askama::Template;
+use anyhow::Context;
 use axum::{
     extract::State,
-    http::StatusCode,
-    response::{Html, IntoResponse, Response},
+    response::{IntoResponse, Response},
 };
 use serde::Deserialize;
 use sqlx::PgPool;
 use uuid::Uuid;
 
-#[tracing::instrument(name = "Confirming new subscriber", skip(params))]
+#[tracing::instrument(name = "Confirming new subscriber", skip_all)]
 pub async fn confirm(
     State(AppState {
         connection_pool, ..
     }): State<AppState>,
     Query(params): Query<Params>,
-) -> Response {
-    let Ok(subscriber_id) =
-        get_subscriber_id_from_token(&connection_pool, &params.subscription_token).await
-    else {
-        return StatusCode::INTERNAL_SERVER_ERROR.into_response();
-    };
-    if let Some(subscriber_id) = subscriber_id {
-        if confirm_subscriber(&connection_pool, &subscriber_id)
+) -> Result<Response, AppError> {
+    let subscriber_id = get_subscriber_id_from_token(&connection_pool, &params.subscription_token)
+        .await
+        .context("Could not fetch subscriber id given subscription token.")?;
+    if let Some(id) = subscriber_id {
+        confirm_subscriber(&connection_pool, &id)
             .await
-            .is_err()
-        {
-            StatusCode::INTERNAL_SERVER_ERROR.into_response()
-        } else {
-            Html(ConfirmTemplate.render().unwrap()).into_response()
-        }
+            .context("Failed to update subscriber status.")?;
+        let template = HtmlTemplate(ConfirmTemplate);
+        Ok(template.into_response())
     } else {
-        StatusCode::UNAUTHORIZED.into_response()
+        Ok(not_found_html())
     }
 }
 
-#[tracing::instrument(
-    name = "Mark subscriber as confirmed",
-    skip(connection_pool, subscriber_id)
-)]
+#[tracing::instrument(name = "Mark subscriber as confirmed", skip(connection_pool))]
 async fn confirm_subscriber(
     connection_pool: &PgPool,
     subscriber_id: &Uuid,
@@ -53,18 +44,11 @@ async fn confirm_subscriber(
         subscriber_id
     )
     .execute(connection_pool)
-    .await
-    .map_err(|e| {
-        tracing::error!("Failed to execute query: {:?}", e);
-        e
-    })?;
+    .await?;
     Ok(())
 }
 
-#[tracing::instrument(
-    name = "Get subscriber_id from token",
-    skip(connection, subscription_token)
-)]
+#[tracing::instrument(name = "Get subscriber id from token", skip(connection))]
 async fn get_subscriber_id_from_token(
     connection: &PgPool,
     subscription_token: &str,
@@ -74,11 +58,7 @@ async fn get_subscriber_id_from_token(
         subscription_token
     )
     .fetch_optional(connection)
-    .await
-    .map_err(|e| {
-        tracing::error!("Failed to execute query: {:?}", e);
-        e
-    })?;
+    .await?;
     Ok(saved.map(|r| r.subscriber_id))
 }
 

src/routes/unsubscribe.rs

@@ -31,6 +31,10 @@ pub struct UnsubFormData {
     email: String,
 }
 
+#[tracing::instrument(
+    name = "Removing subscriber from database",
+    skip(connection_pool, email_client, base_url)
+)]
 pub async fn post_unsubscribe(
     State(AppState {
         connection_pool,
@@ -54,7 +58,7 @@ pub async fn post_unsubscribe(
     Ok(Html(template.render().unwrap()).into_response())
 }
 
-#[tracing::instrument(name = "Fetching unsubscribe token from the database", skip_all)]
+#[tracing::instrument(name = "Fetching unsubscribe token from database", skip_all)]
 async fn fetch_unsubscribe_token(
     connection_pool: &PgPool,
     subscriber_email: &SubscriberEmail,
@@ -69,7 +73,7 @@ async fn fetch_unsubscribe_token(
     Ok(r.and_then(|r| r.unsubscribe_token))
 }
 
-#[tracing::instrument(name = "Send an unsubscribe confirmation email", skip_all)]
+#[tracing::instrument(name = "Send an confirmation email", skip_all)]
 pub async fn send_unsubscribe_email(
     email_client: &EmailClient,
     subscriber_email: &SubscriberEmail,
@@ -102,7 +106,7 @@ If you did not request this, you can safely ignore this email."#,
         .await
 }
 
-#[tracing::instrument(name = "Removing user from database if he exists", skip_all)]
+#[tracing::instrument(name = "Removing user from database", skip(connection_pool))]
 pub async fn unsubscribe_confirm(
     Query(UnsubQueryParams { token }): Query<UnsubQueryParams>,
     State(AppState {