Breadcrumbs are computed even when disabled

[mixalturek]

Hi, I'm debugging Sentry code and I have just noticed that a breadcrumb is computed, added to scope and immediately removed when the breadcrumbs are disabled. This is quite ineffective, there can be a simple condition at the very beginning to prevent all that unnecessary work. We are using sentry = "~0.13" (0.13.0) and log integration, I found the same code in the current master branch.

This is our config.

use std::borrow::Cow;

use failure::Error;
use sentry;
use sentry::integrations::log::{Logger, LoggerOptions};
pub use sentry::internals::ClientInitGuard;

use crate::configuration;

pub(crate) fn init(cfg: &configuration::Sentry) -> Result<ClientInitGuard, Error> {
    let sentry_guard = sentry::init(sentry::ClientOptions {
        dsn: Some(cfg.dsn.parse()?),
        environment: Some(Cow::from(cfg.environment.clone())),

        // It was tracking last 100 log messages to have a context, fully disable.
        max_breadcrumbs: 0,     // <<<<< THE ONLY ACTIVE CONFIG
        attach_stacktrace: false,

        // Show what exactly sentry client is doing.
        // debug: true,
        ..Default::default()
    });

    Ok(sentry_guard)
}

/// Build a new Sentry logger.
pub(crate) fn new_sentry_logger() -> Logger {
    let options = LoggerOptions {
        global_filter: Some(log::LevelFilter::Warn),
        filter: log::LevelFilter::Warn,
        emit_breadcrumbs: false,    // <<<<< DEAD STORE, NEVER READ
        emit_error_events: true,
        emit_warning_events: true,
    };

    Logger::new(None, options)
}

Field LoggerOptions::emit_breadcrumbs is defined but never read so the only effective config is ClientOptions::max_breadcrumbs. Setting it to zero causes the described behavior that is visible the code below.

    /// Adds a new breadcrumb to the current scope.
    ///
    /// This is equivalent to the global [`sentry::add_breadcrumb`](fn.add_breadcrumb.html) but
    /// sends the breadcrumb into the hub instead.
    pub fn add_breadcrumb<B: IntoBreadcrumbs>(&self, breadcrumb: B) {
        with_client_impl! {{
            self.inner.with_mut(|stack| {
                let top = stack.top_mut();
                if let Some(ref client) = top.client {
                    let scope = Arc::make_mut(&mut top.scope);
                    let options = client.options();
                    for breadcrumb in breadcrumb.into_breadcrumbs() {
                        let breadcrumb_opt = match options.before_breadcrumb {
                            Some(ref callback) => callback(breadcrumb),
                            None => Some(breadcrumb)
                        };
                        if let Some(breadcrumb) = breadcrumb_opt {
                            scope.breadcrumbs.push_back(breadcrumb);
                        }
                        while scope.breadcrumbs.len() > options.max_breadcrumbs {  // <<<<< INEFFECTIVE, TOO LATE
                            scope.breadcrumbs.pop_front();
                        }
                    }
                }
            })
        }}
    }