[blob] add errorType to tracing error logs

Summary: This adds error types to our tracing error logs for filtering by our alarms Depends on D13393 Test Plan: Tested later when triggering alarms Reviewers: bartek, varun Reviewed By: bartek Subscribers: ashoat, tomek Differential Revision: https://phab.comm.dev/D13394
CommE2E · Sep 20, 2024 · 335535c · 335535c
1 parent 8ede856
commit 335535c
Show file tree

Hide file tree

Showing 5 changed files with 90 additions and 20 deletions.
diff --git a/services/blob/src/constants.rs b/services/blob/src/constants.rs
@@ -42,3 +42,12 @@ pub const DEFAULT_S3_BUCKET_NAME: &str = "commapp-blob";
 pub const S3_MULTIPART_UPLOAD_MINIMUM_CHUNK_SIZE: u64 = 5 * 1024 * 1024;
 
 pub const INVITE_LINK_BLOB_HASH_PREFIX: &str = "invite_";
+
+// Error Types
+
+pub mod error_types {
+  pub const S3_ERROR: &str = "S3 Error";
+  pub const DDB_ERROR: &str = "DDB Error";
+  pub const HTTP_ERROR: &str = "HTTP Error";
+  pub const OTHER_ERROR: &str = "Other Error";
+}
diff --git a/services/blob/src/database/client.rs b/services/blob/src/database/client.rs
@@ -14,6 +14,7 @@ use std::collections::HashMap;
 use tracing::{debug, error, trace};
 
 use crate::constants::db::*;
+use crate::constants::error_types;
 
 use super::errors::{BlobDBError, Error as DBError};
 use super::types::*;
@@ -205,7 +206,10 @@ impl DatabaseClient {
       .send()
       .await
       .map_err(|err| {
-        error!("DynamoDB client failed to query holders: {:?}", err);
+        error!(
+          errorType = error_types::DDB_ERROR,
+          "DynamoDB client failed to query holders: {:?}", err
+        );
         DBError::AwsSdk(Box::new(err.into()))
       })?;
 
@@ -274,7 +278,10 @@ impl DatabaseClient {
       .send()
       .await
       .map_err(|err| {
-        error!("DynamoDB client failed to query unchecked items: {:?}", err);
+        error!(
+          errorType = error_types::DDB_ERROR,
+          "DynamoDB client failed to query unchecked items: {:?}", err
+        );
         DBError::AwsSdk(Box::new(err.into()))
       })?;
 

diff --git a/services/blob/src/http/errors.rs b/services/blob/src/http/errors.rs
@@ -7,6 +7,7 @@ use aws_sdk_dynamodb::Error as DynamoDBError;
 use http::StatusCode;
 use tracing::{debug, error, trace, warn};
 
+use crate::constants::error_types;
 use crate::database::errors::{BlobDBError, Error as DBError};
 use crate::s3::Error as S3Error;
 use crate::service::BlobServiceError;
@@ -28,32 +29,47 @@ pub(super) fn handle_blob_service_error(err: &BlobServiceError) -> HttpError {
           ErrorServiceUnavailable("please retry")
         }
         unexpected => {
-          error!("Received an unexpected AWS error: {0:?} - {0}", unexpected);
+          error!(
+            errorType = error_types::OTHER_ERROR,
+            "Received an unexpected AWS error: {0:?} - {0}", unexpected
+          );
           ErrorInternalServerError("server error")
         }
       },
       DBError::Blob(BlobDBError::InvalidInput(_)) => {
         ErrorBadRequest("bad request")
       }
       unexpected => {
-        error!("Received an unexpected DB error: {0:?} - {0}", unexpected);
+        error!(
+          errorType = error_types::DDB_ERROR,
+          "Received an unexpected DB error: {0:?} - {0}", unexpected
+        );
         ErrorInternalServerError("server error")
       }
     },
     BlobServiceError::S3(s3_err) => match s3_err {
       S3Error::AwsSdk(aws_err) => match aws_err.as_ref() {
         aws_sdk_s3::Error::NotFound(_) | aws_sdk_s3::Error::NoSuchKey(_) => {
-          error!("Data inconsistency! Blob is present in database but not present in S3!");
+          error!(
+            errorType = error_types::S3_ERROR,
+            "Data inconsistency! Blob is present in database but not present in S3!"
+          );
           ErrorInternalServerError("server error")
         }
         err => {
-          error!("Received an unexpected AWS S3 error: {0:?} - {0}", err);
+          error!(
+            errorType = error_types::S3_ERROR,
+            "Received an unexpected AWS S3 error: {0:?} - {0}", err
+          );
           ErrorInternalServerError("server error")
         }
       },
       S3Error::EmptyUpload => ErrorBadRequest("empty upload"),
       unexpected => {
-        error!("Received an unexpected S3 error: {0:?} - {0}", unexpected);
+        error!(
+          errorType = error_types::S3_ERROR,
+          "Received an unexpected S3 error: {0:?} - {0}", unexpected
+        );
         ErrorInternalServerError("server error")
       }
     },
@@ -66,7 +82,10 @@ pub(super) fn handle_blob_service_error(err: &BlobServiceError) -> HttpError {
       ErrorBadRequest("bad request")
     }
     err => {
-      error!("Received an unexpected error: {0:?} - {0}", err);
+      error!(
+        errorType = error_types::OTHER_ERROR,
+        "Received an unexpected error: {0:?} - {0}", err
+      );
       ErrorInternalServerError("server error")
     }
   }

diff --git a/services/blob/src/s3.rs b/services/blob/src/s3.rs
@@ -7,6 +7,8 @@ use aws_sdk_s3::{
 use std::ops::{Bound, RangeBounds};
 use tracing::{debug, error, trace};
 
+use crate::constants::error_types;
+
 #[derive(
   Debug, derive_more::Display, derive_more::From, derive_more::Error,
 )]
@@ -132,7 +134,10 @@ impl S3Client {
       .send()
       .await
       .map_err(|e| {
-        error!("S3 failed to get object metadata");
+        error!(
+          errorType = error_types::S3_ERROR,
+          "S3 failed to get object metadata"
+        );
         Error::AwsSdk(Box::new(e.into()))
       })?;
 
@@ -171,11 +176,14 @@ impl S3Client {
     }
 
     let response = request.send().await.map_err(|e| {
-      error!("S3 failed to get object");
+      error!(errorType = error_types::S3_ERROR, "S3 failed to get object");
       Error::AwsSdk(Box::new(e.into()))
     })?;
     let data = response.body.collect().await.map_err(|e| {
-      error!("S3 failed to stream object bytes");
+      error!(
+        errorType = error_types::S3_ERROR,
+        "S3 failed to stream object bytes"
+      );
       Error::ByteStream(e.into())
     })?;
     Ok(data.to_vec())
@@ -191,7 +199,10 @@ impl S3Client {
       .send()
       .await
       .map_err(|e| {
-        error!("S3 failed to delete object");
+        error!(
+          errorType = error_types::S3_ERROR,
+          "S3 failed to delete object"
+        );
         Error::AwsSdk(Box::new(e.into()))
       })?;
 
@@ -228,7 +239,10 @@ impl S3Client {
       .send()
       .await
       .map_err(|e| {
-        error!("S3 failed to batch delete objects");
+        error!(
+          errorType = error_types::S3_ERROR,
+          "S3 failed to batch delete objects"
+        );
         Error::AwsSdk(Box::new(e.into()))
       })?;
 
@@ -259,12 +273,18 @@ impl MultiPartUploadSession {
       .send()
       .await
       .map_err(|e| {
-        error!("S3 failed to start upload session");
+        error!(
+          errorType = error_types::S3_ERROR,
+          "S3 failed to start upload session"
+        );
         Error::AwsSdk(Box::new(e.into()))
       })?;
 
     let upload_id = multipart_upload_res.upload_id().ok_or_else(|| {
-      error!("Upload ID expected to be present");
+      error!(
+        errorType = error_types::S3_ERROR,
+        "Upload ID expected to be present"
+      );
       Error::MissingUploadID
     })?;
     debug!("Started multipart upload session with ID: {}", upload_id);
@@ -293,7 +313,10 @@ impl MultiPartUploadSession {
       .send()
       .await
       .map_err(|e| {
-        error!("Failed to add upload part");
+        error!(
+          errorType = error_types::S3_ERROR,
+          "Failed to add upload part"
+        );
         Error::AwsSdk(Box::new(e.into()))
       })?;
 
@@ -331,7 +354,10 @@ impl MultiPartUploadSession {
       .send()
       .await
       .map_err(|e| {
-        error!("Failed to finish upload session");
+        error!(
+          errorType = error_types::S3_ERROR,
+          "Failed to finish upload session"
+        );
         Error::AwsSdk(Box::new(e.into()))
       })?;
 

diff --git a/services/blob/src/service.rs b/services/blob/src/service.rs
@@ -24,7 +24,10 @@ use crate::database::types::{
 use crate::database::DBError;
 use crate::s3::{Error as S3Error, S3Client, S3Path};
 use crate::tools::MemOps;
-use crate::{constants::BLOB_DOWNLOAD_CHUNK_SIZE, database::DatabaseClient};
+use crate::{
+  constants::error_types, constants::BLOB_DOWNLOAD_CHUNK_SIZE,
+  database::DatabaseClient,
+};
 
 #[derive(
   Debug, derive_more::Display, derive_more::From, derive_more::Error,
@@ -120,14 +123,20 @@ impl BlobService {
     let blob_size = object_metadata
       .content_length()
       .ok_or_else(|| {
-        error!("Failed to get S3 object content length");
+        error!(
+          errorType = error_types::S3_ERROR,
+          "Failed to get S3 object content length"
+        );
         BlobServiceError::InvalidState
       })
       .and_then(|len| {
         if len >= 0 {
           Ok(len as u64)
         } else {
-          error!("S3 object content length is negative");
+          error!(
+            errorType = error_types::S3_ERROR,
+            "S3 object content length is negative"
+          );
           Err(BlobServiceError::InvalidState)
         }
       })?;