open-telemetry / opentelemetry-rust

The Rust OpenTelemetry implementation
https://opentelemetry.io
Apache License 2.0
1.82k stars 424 forks source link

[Bug]: example code is misleading about how `with_view` works #1849

Open CobaltCause opened 4 months ago

CobaltCause commented 4 months ago

What happened?

The way these variable names are worded make it sound like that's the only effect each view has and neglects to mention that the implementation of these closures also drops all pieces of information about each instrument that aren't specified again via Stream combinators. To demonstrate:

$ cargo run -qp metrics-advanced | jq '.resourceMetrics.scopeMetrics[].metrics[].name'
"my_histogram_renamed"
""
""

It's quite surprising that the names are dropped despite no indication that this would happen.

Here's a small program that demonstrates how this is problematic and also demonstrates how to achieve the desired (and implied) behavior:

// [dependencies]
// opentelemetry = { version = "0.23.0", features = ["metrics"] }
// opentelemetry-prometheus = "0.16.0"
// opentelemetry_sdk = "0.23.0"
// prometheus = "0.13.4"

use std::error::Error;

use opentelemetry::metrics::{MeterProvider, Unit};
use opentelemetry_prometheus::exporter;
use opentelemetry_sdk::metrics::{
    new_view, Aggregation, Instrument, SdkMeterProvider, Stream,
};
use prometheus::{Registry, TextEncoder};

fn main() {
    if let Err(e) = minimal_repro() {
        eprintln!("error: {e}");
    }
    if this_works().is_err() {
        unreachable!()
    }
}

// This does what the example implies you should do to only change the
// boundaries but leave everything else the same.
fn minimal_repro() -> Result<(), Box<dyn Error>> {
    let registry = Registry::new();
    let exporter = exporter().with_registry(registry.clone()).build()?;
    let provider = SdkMeterProvider::builder()
        .with_reader(exporter)
        .with_view(|_: &Instrument| {
            Some(Stream::new().aggregation(
                Aggregation::ExplicitBucketHistogram {
                    boundaries: vec![1., 2., 3.],
                    record_min_max: true,
                },
            ))
        })
        .build();
    let meter = provider.meter("example");
    let histogram =
        meter.u64_histogram("histogram").with_unit(Unit::new("seconds")).init();
    histogram.record(1, &[]);
    let encoded = TextEncoder::new().encode_to_string(&registry.gather())?;
    println!("{encoded}");
    Ok(())
}

// This version actually works. The secret sauce is to use the `new_view`
// function.
fn this_works() -> Result<(), Box<dyn Error>> {
    let registry = Registry::new();
    let exporter = exporter().with_registry(registry.clone()).build()?;
    let provider = SdkMeterProvider::builder()
        .with_reader(exporter)
        .with_view(new_view(
            Instrument::new().name("histogram"),
            Stream::new().aggregation(Aggregation::ExplicitBucketHistogram {
                boundaries: vec![1., 2., 3.],
                record_min_max: true,
            }),
        )?)
        .build();
    let meter = provider.meter("example");
    let histogram =
        meter.u64_histogram("histogram").with_unit(Unit::new("seconds")).init();
    histogram.record(1, &[]);
    let encoded = TextEncoder::new().encode_to_string(&registry.gather())?;
    println!("{encoded}");
    Ok(())
}

I've pasted the output of this program in the "Relevant log output" section.

API Version

0.23.0

SDK Version

0.23.0

What Exporter(s) are you seeing the problem on?

Prometheus

Relevant log output

attempt one failed: Error: MetricFamily has no name: name: "" help: "" type: HISTOGRAM metric {label {name: "otel_scope_name" value: "example"} histogram {sample_count: 1 sample_sum: 1 bucket {cumulative_count: 1upper_bound: 1} bucket {cumulative_count: 1 upper_bound: 2} bucket {cumulative_count: 1 upper_bound: 3}}}
# TYPE histogram histogram
histogram_bucket{otel_scope_name="example",le="1"} 1
histogram_bucket{otel_scope_name="example",le="2"} 1
histogram_bucket{otel_scope_name="example",le="3"} 1
histogram_bucket{otel_scope_name="example",le="+Inf"} 1
histogram_sum{otel_scope_name="example"} 1
histogram_count{otel_scope_name="example"} 1
# HELP target_info Target metadata
# TYPE target_info gauge
target_info{service_name="unknown_service",telemetry_sdk_language="rust",telemetry_sdk_name="opentelemetry",telemetry_sdk_version="0.23.0"} 1
cijothomas commented 4 months ago

Thanks for reporting! It looks like a bug in view implementation, not just an issue with doc. We won't be fixing the View bug, as Views are about to be removed for the initial stable release.

For customizing histogram buckets, https://github.com/open-telemetry/opentelemetry-rust/issues/1241 will be offered instead.