stackabletech / issues

This repository is only for issues that concern multiple repositories or don't fit into any specific repository
2 stars 0 forks source link

Replace operator logging initialization with stackable-telemetry initialization #639

Open NickLarsenNZ opened 2 months ago

NickLarsenNZ commented 2 months ago

The old initialize_logging implementation currently requires opentelemetry-jaeger which is now deprecated in favour of the vendor/collector agnostic OTLP protocol in opentelemetry-otlp.

This is preventing us from updating the opentelemetry related crates. (as seen in https://github.com/stackabletech/operator-rs/pull/867, among other past PRs).

The recommended path forward for using Jaeger is to use the OTLP protocol. stackable-telemetry already supports this, and so it is about time we deprecate the old log initialization and jaeger protocol and use stackable-telemetry.

Tasks

### Related PRs
- [ ] https://github.com/stackabletech/operator-rs/pull/901
- [ ] _Link for using `filter::Directive` for more granular level setting_
- [ ] _Link to PR for RollingFileAppender feature_
- [ ] _Link to PR for initialize_logging and Jaeger deprecation_
- [ ] _Link to PR for improving formatting of `print_startup_string()`_
- [ ] _Link to each operator PR_

Acceptance Criteria


Code example for secret-operator:

secret-operator logging (before)

pub const APP_NAME: &str = "secret";

// ...

stackable_operator::logging::initialize_logging(
    "SECRET_PROVISIONER_LOG",
    APP_NAME,
    tracing_target,
);

secret-operator logging (before)

secret-operator logging (after)

# rust/operator-binary/Cargo.toml
stackable-telemetry.workspace = true

# Cargo.toml
stackable-telemetry = { git = "https://github.com/stackabletech/operator-rs.git", tag = "stackable-telemetry-0.2.0" }

The code above will be replaced with code looking like this:

// update the app name to include "-operator"
pub const APP_NAME: &str = "secret-operator";

let _tracing_guard = Tracing::builder()
    .service_name(APP_NAME)
    .with_console_output("SECRET_PROVISIONER_LOG", LevelFilter::INFO)
    // Todo: consider how to toggle log/tracing export:
    // .with_otlp_log_exporter("SECRET_PROVISIONER_OTLP_LOG", LevelFilter::DEBUG)
    // .with_otlp_trace_exporter("SECRET_PROVISIONER_OTLP_TRACE", LevelFilter::TRACE)
    .build()
    .init()
    .context("Failed to initialize tracing")?;

todo screenshot

NickLarsenNZ commented 2 months ago

Recording down some output from recent discussions with @soenkeliebau and @Techassi...

@soenkeliebau made otlp exporters optional at runtime via:

let mut builder = Tracing::builder()
    .service_name("whoyougonnacall")
    .with_console_output("WYGC_CONSOLE", LevelFilter::INFO);

// Read env vars for whether to enable trace and log exporting
// We do this first in order to have tracing properly initialized
// when we start parsing the config
if enable_trace_exporter().context(ParseConfigSnafu)? {
    builder = builder.with_otlp_trace_exporter("TEST_OTLP_TRACE", LevelFilter::TRACE);
}
if enable_log_exporter().context(ParseConfigSnafu)? {
    builder = builder.with_otlp_log_exporter("TEST_OTLP_LOG", LevelFilter::TRACE);
}

let _tracing_guard = builder.build().init().context(InitializeTelemetrySnafu)?;

I suggested we could make this a little more developer friendly with some additional builder methods (and type states):

let _tracing_guard = Tracing::builder()
    .service_name("whoyougonnacall")
    .with_console_output("WYGC_CONSOLE", LevelFilter::INFO);
    .with_otlp_trace_exporter("TEST_OTLP_TRACE", LevelFilter::TRACE)
        .enabled(enable_trace_exporter().context(ParseConfigSnafu)?)
    .with_otlp_log_exporter("TEST_OTLP_LOG", LevelFilter::TRACE)
        .enabled(enable_log_exporter().context(ParseConfigSnafu)?)
    .build().init().context(InitializeTelemetrySnafu)?;

@Techassi suggested a SettingsBuilder:

let _guard = Tracing::builder()
    .service_name("stuff")
    .with_console_output(
        Settings::builder()
            .env_var("MY_ENV_VAR")
            .enabled(true)
    )

To which I replied with the function signature taking anything that converts into a SettingsBuilder for the exporter:

fn with_console_output(settings: impl Into<ConsoleSettings>)

// example usage:
let _guard = Tracing::builder()
    .service_name("stuff")
    .with_console_output(
        Settings::builder()
            .env_var("MY_ENV_VAR")
            .enabled(true)
            .into() // gives us a ConfigSettingsBuilder
            .log_format(Format::JSON)
            .build()
    )
    // ...