Merge pull request #255 from mozilla/pb/logging-enums

https://github.com/mozilla/fxa-email-service/pull/255
r=vbudhram

scripts/test_standalone.sh

@@ -2,8 +2,8 @@
 
 export RUST_BACKTRACE=1
 
-if [ -z "$FXA_EMAIL_LOG_FORMAT" ]; then
-  export FXA_EMAIL_LOG_FORMAT=null
+if [ -z "$FXA_EMAIL_LOG_LEVEL" ]; then
+  export FXA_EMAIL_LOG_LEVEL=off
 fi
 
 cargo test -- --test-threads=1

scripts/test_with_authdb.sh

@@ -17,9 +17,10 @@ fi
 
 sleep 2
 
-if [ -z "$FXA_EMAIL_LOG_FORMAT" ]; then
-  export FXA_EMAIL_LOG_FORMAT=null
+if [ -z "$FXA_EMAIL_LOG_LEVEL" ]; then
+  export FXA_EMAIL_LOG_LEVEL=off
 fi
 
 export RUST_BACKTRACE=1
+
 cargo test -- --test-threads=1

src/api/healthcheck/test.rs

@@ -16,7 +16,7 @@ fn setup() -> Client {
     let settings = Settings::new().unwrap();
     let db = DbClient::new(&settings);
     let delivery_problems = DeliveryProblems::new(&settings, db);
-    let logger = MozlogLogger::new(&settings).expect("MozlogLogger::init error");
+    let logger = MozlogLogger::new(&settings);
     let message_data = MessageData::new(&settings);
     let providers = Providers::new(&settings);
     let server = rocket::ignite()

src/api/send/test.rs

@@ -18,7 +18,7 @@ fn setup() -> Client {
     settings.provider.forcedefault = false;
     let db = DbClient::new(&settings);
     let delivery_problems = DeliveryProblems::new(&settings, db);
-    let logger = MozlogLogger::new(&settings).expect("MozlogLogger::init error");
+    let logger = MozlogLogger::new(&settings);
     let message_data = MessageData::new(&settings);
     let providers = Providers::new(&settings);
     let server = rocket::ignite()

src/bin/queues.rs

@@ -75,7 +75,7 @@ fn main() {
         register_panic_handler();
     }
 
-    let logger = MozlogLogger::new(&SETTINGS).expect("MozlogLogger::init error");
+    let logger = MozlogLogger::new(&SETTINGS);
     let _guard = slog_scope::set_global_logger(logger.0);
     let process_queues: &Fn(usize) -> LoopResult = &|previous_count: usize| {
         let future = QUEUES

src/bin/service.rs

@@ -29,6 +29,7 @@ use fxa_email_service::{
     logging::MozlogLogger,
     providers::Providers,
     settings::Settings,
+    types::logging::LogLevel,
 };
 
 fn main() {
@@ -51,14 +52,14 @@ fn main() {
 
     let db = DbClient::new(&settings);
     let delivery_problems = DeliveryProblems::new(&settings, db);
-    let logger = MozlogLogger::new(&settings).expect("MozlogLogger::init error");
+    let logger = MozlogLogger::new(&settings);
     let message_data = MessageData::new(&settings);
     let providers = Providers::new(&settings);
 
     let config = settings
         .build_rocket_config()
         .expect("Error creating rocket config");
-    rocket::custom(config, settings.log.level.as_ref() != "off")
+    rocket::custom(config, settings.log.level != LogLevel::Off)
         .manage(settings)
         .manage(delivery_problems)
         .manage(logger)

src/logging/mod.rs

@@ -15,7 +15,10 @@ use slog_mozlog_json::MozLogJson;
 use slog_term;
 
 use settings::Settings;
-use types::error::AppError;
+use types::{
+    error::AppError,
+    logging::{LogFormat, LogLevel},
+};
 
 lazy_static! {
     static ref LOGGER_NAME: String =
@@ -110,28 +113,31 @@ pub struct MozlogLogger(pub slog::Logger);
 
 impl MozlogLogger {
     /// Construct a logger.
-    pub fn new(settings: &Settings) -> Result<MozlogLogger, Error> {
-        let logger = match settings.log.format.as_ref() {
-            "mozlog" => {
-                let drain = MozLogJson::new(io::stdout())
-                    .logger_name(LOGGER_NAME.to_owned())
-                    .msg_type(MSG_TYPE.to_owned())
-                    .build()
-                    .fuse();
-                let drain = slog_async::Async::new(drain).build().fuse();
-                Ok(slog::Logger::root(drain, slog_o!()))
+    pub fn new(settings: &Settings) -> Self {
+        let logger = if settings.log.level == LogLevel::Off {
+            slog::Logger::root(Discard, slog_o!())
+        } else {
+            match settings.log.format {
+                LogFormat::Mozlog => {
+                    let drain = MozLogJson::new(io::stdout())
+                        .logger_name(LOGGER_NAME.to_owned())
+                        .msg_type(MSG_TYPE.to_owned())
+                        .build()
+                        .fuse();
+                    let drain = slog_async::Async::new(drain).build().fuse();
+                    slog::Logger::root(drain, slog_o!())
+                }
+
+                LogFormat::Pretty => {
+                    let decorator = slog_term::TermDecorator::new().build();
+                    let drain = slog_term::FullFormat::new(decorator).build().fuse();
+                    let drain = slog_async::Async::new(drain).build().fuse();
+                    slog::Logger::root(drain, slog_o!())
+                }
             }
-            "pretty" => {
-                let decorator = slog_term::TermDecorator::new().build();
-                let drain = slog_term::FullFormat::new(decorator).build().fuse();
-                let drain = slog_async::Async::new(drain).build().fuse();
-                Ok(slog::Logger::root(drain, slog_o!()))
-            }
-            "null" => Ok(slog::Logger::root(Discard, slog_o!())),
-            _0 => Err(err_msg(format!("Unknown logger format: {}", _0))),
         };
 
-        Ok(MozlogLogger(logger?))
+        MozlogLogger(logger)
     }
 
     /// Log a rocket request.

src/settings/mod.rs

@@ -23,7 +23,11 @@ use serde::de::{Deserialize, Deserializer, Error, Unexpected};
 
 use logging::MozlogLogger;
 use types::{
-    duration::Duration, email_address::EmailAddress, env::Env, provider::Provider as ProviderType,
+    duration::Duration,
+    email_address::EmailAddress,
+    env::Env,
+    logging::{LogFormat, LogLevel},
+    provider::Provider as ProviderType,
     validate,
 };
 
@@ -73,10 +77,6 @@ deserialize_and_validate! {
     (BaseUri, base_uri, "base URI"),
     /// Host name or IP address type.
     (Host, host, "host name or IP address"),
-    /// Logging level type.
-    (LoggingLevel, logging_level, "'normal', 'debug', 'critical' or 'off'"),
-    /// Logging format type.
-    (LoggingFormat, logging_format, "'mozlog', 'pretty' or 'null'"),
     /// Sender name type.
     (SenderName, sender_name, "sender name"),
     /// Sendgrid API key type.
@@ -158,10 +158,10 @@ pub struct DeliveryProblemLimits {
 #[derive(Debug, Default, Deserialize, Serialize)]
 pub struct Log {
     /// The logging level.
-    pub level: LoggingLevel,
+    pub level: LogLevel,
 
     /// The logging format.
-    pub format: LoggingFormat,
+    pub format: LogFormat,
 }
 
 /// Email provider settings.
@@ -402,8 +402,7 @@ impl Settings {
                     }
                 }
 
-                let logger =
-                    MozlogLogger::new(&settings).expect("Unable to create MozlogLogger instance");
+                let logger = MozlogLogger::new(&settings);
                 slog_info!(logger, "Settings::new"; "settings" => &settings);
                 Ok(settings)
             }
@@ -414,9 +413,9 @@ impl Settings {
     /// Create rocket configuration based on the environment
     /// variable. Defaults to `dev` mode if `FXA_EMAIL_ENV` is not set.
     pub fn build_rocket_config(&self) -> Result<RocketConfig, RocketConfigError> {
-        let log_level = match self.log.level.0.as_str() {
-            "debug" => RocketLoggingLevel::Debug,
-            "critical" => RocketLoggingLevel::Critical,
+        let log_level = match self.log.level {
+            LogLevel::Debug => RocketLoggingLevel::Debug,
+            LogLevel::Critical => RocketLoggingLevel::Critical,
             _ => RocketLoggingLevel::Normal,
         };
         let rocket_config = match self.env {

src/settings/test.rs

@@ -186,15 +186,15 @@ fn env_vars_take_precedence() {
             };
 
             let log = Log {
-                level: if settings.log.level == LoggingLevel("debug".to_string()) {
-                    LoggingLevel("off".to_string())
+                level: if settings.log.level == LogLevel::Debug {
+                    LogLevel::Off
                 } else {
-                    LoggingLevel("debug".to_string())
+                    LogLevel::Debug
                 },
-                format: if settings.log.format == LoggingFormat("null".to_string()) {
-                    LoggingFormat("pretty".to_string())
+                format: if settings.log.format == LogFormat::Mozlog {
+                    LogFormat::Pretty
                 } else {
-                    LoggingFormat("null".to_string())
+                    LogFormat::Mozlog
                 },
             };
 
@@ -215,8 +215,8 @@ fn env_vars_take_precedence() {
             );
             env::set_var("FXA_EMAIL_HMACKEY", &hmac_key.to_string());
             env::set_var("FXA_EMAIL_ENV", current_env.as_ref());
-            env::set_var("FXA_EMAIL_LOG_LEVEL", &log.level.0);
-            env::set_var("FXA_EMAIL_LOG_FORMAT", &log.format.0);
+            env::set_var("FXA_EMAIL_LOG_LEVEL", log.level.as_ref());
+            env::set_var("FXA_EMAIL_LOG_FORMAT", log.format.as_ref());
             env::set_var("FXA_EMAIL_PROVIDER_DEFAULT", provider.default.as_ref());
             env::set_var(
                 "FXA_EMAIL_PROVIDER_FORCEDEFAULT",

src/types/error/mod.rs

@@ -191,6 +191,12 @@ pub enum AppErrorKind {
 
     #[fail(display = "Invalid environment: {}", _0)]
     InvalidEnv(String),
+
+    #[fail(display = "Invalid log level: {}", _0)]
+    InvalidLogLevel(String),
+
+    #[fail(display = "Invalid log format: {}", _0)]
+    InvalidLogFormat(String),
 }
 
 impl AppErrorKind {
@@ -217,6 +223,8 @@ impl AppErrorKind {
             AppErrorKind::SoftBounce { .. } => Some(107),
             AppErrorKind::HardBounce { .. } => Some(108),
             AppErrorKind::InvalidEnv { .. } => Some(109),
+            AppErrorKind::InvalidLogLevel { .. } => Some(110),
+            AppErrorKind::InvalidLogFormat { .. } => Some(111),
         }
     }
 

src/types/logging/mod.rs

@@ -0,0 +1,36 @@
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, you can obtain one at https://mozilla.org/MPL/2.0/.
+
+//! Logging types.
+
+#[cfg(test)]
+mod test;
+
+use serde::de::Error;
+
+use types::error::{AppError, AppErrorKind};
+
+enum_boilerplate!(LogLevel ("log level", InvalidLogLevel) {
+    Normal => "normal",
+    Debug => "debug",
+    Critical => "critical",
+    Off => "off",
+});
+
+impl Default for LogLevel {
+    fn default() -> Self {
+        LogLevel::Normal
+    }
+}
+
+enum_boilerplate!(LogFormat ("log format", InvalidLogFormat) {
+    Mozlog => "mozlog",
+    Pretty => "pretty",
+});
+
+impl Default for LogFormat {
+    fn default() -> Self {
+        LogFormat::Mozlog
+    }
+}

src/types/logging/test.rs

@@ -0,0 +1,62 @@
+// This Source Code Form is subject to the terms of the Mozilla Public
+// License, v. 2.0. If a copy of the MPL was not distributed with this
+// file, you can obtain one at https://mozilla.org/MPL/2.0/.
+
+use std::convert::TryFrom;
+
+use super::*;
+
+#[test]
+fn level_try_from() {
+    let result: Result<LogLevel, AppError> = TryFrom::try_from("normal");
+    assert!(result.is_ok());
+    assert_eq!(result.unwrap(), LogLevel::Normal);
+
+    let result: Result<LogLevel, AppError> = TryFrom::try_from("debug");
+    assert!(result.is_ok());
+    assert_eq!(result.unwrap(), LogLevel::Debug);
+
+    let result: Result<LogLevel, AppError> = TryFrom::try_from("critical");
+    assert!(result.is_ok());
+    assert_eq!(result.unwrap(), LogLevel::Critical);
+
+    let result: Result<LogLevel, AppError> = TryFrom::try_from("off");
+    assert!(result.is_ok());
+    assert_eq!(result.unwrap(), LogLevel::Off);
+
+    let result: Result<LogLevel, AppError> = TryFrom::try_from("wibble");
+    assert!(result.is_err());
+    assert_eq!(result.unwrap_err().to_string(), "Invalid log level: wibble");
+}
+
+#[test]
+fn level_as_ref() {
+    assert_eq!(LogLevel::Normal.as_ref(), "normal");
+    assert_eq!(LogLevel::Debug.as_ref(), "debug");
+    assert_eq!(LogLevel::Critical.as_ref(), "critical");
+    assert_eq!(LogLevel::Off.as_ref(), "off");
+}
+
+#[test]
+fn format_try_from() {
+    let result: Result<LogFormat, AppError> = TryFrom::try_from("mozlog");
+    assert!(result.is_ok());
+    assert_eq!(result.unwrap(), LogFormat::Mozlog);
+
+    let result: Result<LogFormat, AppError> = TryFrom::try_from("pretty");
+    assert!(result.is_ok());
+    assert_eq!(result.unwrap(), LogFormat::Pretty);
+
+    let result: Result<LogFormat, AppError> = TryFrom::try_from("wibble");
+    assert!(result.is_err());
+    assert_eq!(
+        result.unwrap_err().to_string(),
+        "Invalid log format: wibble"
+    );
+}
+
+#[test]
+fn format_as_ref() {
+    assert_eq!(LogFormat::Mozlog.as_ref(), "mozlog");
+    assert_eq!(LogFormat::Pretty.as_ref(), "pretty");
+}

src/types/mod.rs

@@ -70,5 +70,6 @@ pub mod email_address;
 pub mod env;
 pub mod error;
 pub mod headers;
+pub mod logging;
 pub mod provider;
 pub mod validate;

src/types/validate/mod.rs

@@ -29,8 +29,6 @@ lazy_static! {
         r"^[a-zA-Z0-9.\pL\pN!#$%&’*+/=?^_`{|}~-]{1,64}@[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,253}[a-zA-Z0-9])?(?:\.[a-zA-Z0-9](?:[a-zA-Z0-9-]{0,253}[a-zA-Z0-9])?)+$"
     ).unwrap();
     static ref HOST_FORMAT: Regex = Regex::new(r"^[A-Za-z0-9-]+(?:\.[A-Za-z0-9-]+)*$").unwrap();
-    static ref LOGGING_LEVEL: Regex = Regex::new(r"^(?:normal|debug|critical|off)$").unwrap();
-    static ref LOGGING_FORMAT: Regex = Regex::new(r"^(?:mozlog|pretty|null)$").unwrap();
     static ref SENDER_NAME_FORMAT: Regex =
         Regex::new(r"^[A-Za-z0-9-]+(?: [A-Za-z0-9-]+)*$").unwrap();
     static ref SENDGRID_API_KEY_FORMAT: Regex = Regex::new("^[A-Za-z0-9._-]+$").unwrap();
@@ -70,16 +68,6 @@ pub fn host(value: &str) -> bool {
     HOST_FORMAT.is_match(value)
 }
 
-/// Validate logging level.
-pub fn logging_level(value: &str) -> bool {
-    LOGGING_LEVEL.is_match(value)
-}
-
-/// Validate logging format.
-pub fn logging_format(value: &str) -> bool {
-    LOGGING_FORMAT.is_match(value)
-}
-
 /// Validate a sender name.
 pub fn sender_name(value: &str) -> bool {
     SENDER_NAME_FORMAT.is_match(value)

src/types/validate/test.rs

@@ -134,23 +134,6 @@ fn host() {
     assert!(validate::host("127.0.0.1"));
 }
 
-#[test]
-fn logging_level() {
-    assert!(validate::logging_level("normal"));
-    assert!(validate::logging_level("debug"));
-    assert!(validate::logging_level("critical"));
-    assert!(validate::logging_level("off"));
-    assert_eq!(false, validate::logging_level("something else"));
-}
-
-#[test]
-fn logging_format() {
-    assert!(validate::logging_format("mozlog"));
-    assert!(validate::logging_format("pretty"));
-    assert!(validate::logging_format("null"));
-    assert_eq!(false, validate::logging_format("something else"));
-}
-
 #[test]
 fn invalid_host() {
     assert_eq!(validate::host("foo/bar"), false);