open-telemetry / opentelemetry-rust

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

How to customize buckets for different histograms #1174

Closed lerouxrgd closed 1 year ago

lerouxrgd commented 1 year ago

In version 0.19 we could use:

impl AggregatorSelector for MyCustomSelector {
    fn aggregator_for(&self, descriptor: &Descriptor) -> Option<Arc<dyn Aggregator + Send + Sync>> {
        todo!("Something based on descriptor.name()")
    }
}

With version 0.20 the equivalent seems to be using AggregationSelector which only provides a InstrumentKind with much less information.

How can we now customize a histogram buckets using its name ?

jtescher commented 1 year ago

You use the view api as defined in this part of the spec https://github.com/open-telemetry/opentelemetry-specification/blob/v1.21.0/specification/metrics/sdk.md#view

See an example here of using views to configure specific instruments https://github.com/open-telemetry/opentelemetry-rust/blob/dfeac078ff7853e7dc814778524b93470dfa5c9c/opentelemetry-prometheus/tests/integration_test.rs#L342-L353

lerouxrgd commented 1 year ago

Thanks you for pointing this example out !

Actually, for opentelemetry-otlp this doesn't seem doable as we don't have access to its MeterProviderBuilder. Would it make sense to add a .with_view method to OtlpMetricPipeline ?

Sushisource commented 1 year ago

@lerouxrgd Apparently the trick is to do this:

let mp = opentelemetry_otlp::new_pipeline()
    .metrics(runtime::Tokio)
// yada yada
    .build()?;
let mp = MeterProvider::builder().with_reader(mp).with_view(your_view).build()?;
cijothomas commented 1 year ago

Will try to document how to do this for OTLP Exporter (part of https://github.com/open-telemetry/opentelemetry-rust/issues/1060), but i think there is scope to improve the OTLP Exporter part to make it easier. Will check this myself.

lerouxrgd commented 1 year ago

let mp = MeterProvider::builder().with_reader(mp).with_view(your_view).build()?;

@Sushisource Well the MetricProvider returned by OtlpMetricPipeline::build() doesn't implement MetricReader so this doesn't compile... (It does work for PrometheusExporter though).

lerouxrgd commented 1 year ago

In the end I have been able to reproduce the behavior I wanted by mostly copying what OtlpMetricPipeline::build() is doing.

Here is the code if you are interested:

use opentelemetry::global;
use opentelemetry::sdk::{metrics, runtime};

let exporter = opentelemetry_otlp::MetricsExporter::new_http(
    opentelemetry_otlp::HttpExporterBuilder::default()
        .with_endpoint("http://localhost:4318/v1/metrics")
        .with_timeout(Duration::from_secs(3)),
    Box::new(metrics::reader::DefaultTemporalitySelector::new()),
    Box::new(metrics::reader::DefaultAggregationSelector::new()),
)?;

let reader = metrics::PeriodicReader::builder(exporter, runtime::Tokio).build();

let meter_provider = metrics::MeterProvider::builder()
    .with_reader(reader)
    .with_view(metrics::new_view(
        metrics::Instrument::new().name("histogram_*"),
        metrics::Stream::new().aggregation(
            metrics::Aggregation::ExplicitBucketHistogram {
                boundaries: vec![
                    0.0, 5.0, 10.0, 25.0, 50.0, 75.0, 100.0, 250.0, 500.0, 1000.0,
                ],
                record_min_max: true,
            },
        ),
    )?)
    .build();

global::set_meter_provider(meter_provider);

I guess that closes the issue, even though the API could be improved.

Sushisource commented 1 year ago

let mp = MeterProvider::builder().with_reader(mp).with_view(your_view).build()?;

@Sushisource Well the MetricProvider returned by OtlpMetricPipeline::build() doesn't implement MetricReader so this doesn't compile... (It does work for PrometheusExporter though).

Ah, indeed. Thanks for that sample. I was doing the Prom one and the Otel one at the same time and assumed it was going to work lol.