Skip to content

Commit

Permalink
AggregationSelector is not needed
Browse files Browse the repository at this point in the history
  • Loading branch information
Mindaugas Vinkelis committed Sep 8, 2024
1 parent 4a86e16 commit 9a02aab
Show file tree
Hide file tree
Showing 17 changed files with 36 additions and 212 deletions.
10 changes: 2 additions & 8 deletions opentelemetry-otlp/src/exporter/http/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -78,7 +78,7 @@ impl Default for HttpConfig {
/// ```
/// # #[cfg(feature="metrics")]
/// use opentelemetry_sdk::metrics::reader::{
/// DefaultAggregationSelector, DefaultTemporalitySelector,
/// DefaultTemporalitySelector,
/// };
///
/// # fn main() -> Result<(), Box<dyn std::error::Error>> {
Expand All @@ -91,7 +91,6 @@ impl Default for HttpConfig {
/// let metrics_exporter = opentelemetry_otlp::new_exporter()
/// .http()
/// .build_metrics_exporter(
/// Box::new(DefaultAggregationSelector::new()),
/// Box::new(DefaultTemporalitySelector::new()),
/// )?;
///
Expand Down Expand Up @@ -252,7 +251,6 @@ impl HttpExporterBuilder {
#[cfg(feature = "metrics")]
pub fn build_metrics_exporter(
mut self,
aggregation_selector: Box<dyn opentelemetry_sdk::metrics::reader::AggregationSelector>,
temporality_selector: Box<dyn opentelemetry_sdk::metrics::reader::TemporalitySelector>,
) -> opentelemetry::metrics::Result<crate::MetricsExporter> {
use crate::{
Expand All @@ -267,11 +265,7 @@ impl HttpExporterBuilder {
OTEL_EXPORTER_OTLP_METRICS_HEADERS,
)?;

Ok(crate::MetricsExporter::new(
client,
temporality_selector,
aggregation_selector,
))
Ok(crate::MetricsExporter::new(client, temporality_selector))
}
}

Expand Down
10 changes: 2 additions & 8 deletions opentelemetry-otlp/src/exporter/tonic/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ fn resolve_compression(
/// ```no_run
/// # #[cfg(feature="metrics")]
/// use opentelemetry_sdk::metrics::reader::{
/// DefaultAggregationSelector, DefaultTemporalitySelector,
/// DefaultTemporalitySelector,
/// };
///
/// # fn main() -> Result<(), Box<dyn std::error::Error>> {
Expand All @@ -110,7 +110,6 @@ fn resolve_compression(
/// let metrics_exporter = opentelemetry_otlp::new_exporter()
/// .tonic()
/// .build_metrics_exporter(
/// Box::new(DefaultAggregationSelector::new()),
/// Box::new(DefaultTemporalitySelector::new()),
/// )?;
///
Expand Down Expand Up @@ -332,7 +331,6 @@ impl TonicExporterBuilder {
#[cfg(feature = "metrics")]
pub fn build_metrics_exporter(
self,
aggregation_selector: Box<dyn opentelemetry_sdk::metrics::reader::AggregationSelector>,
temporality_selector: Box<dyn opentelemetry_sdk::metrics::reader::TemporalitySelector>,
) -> opentelemetry::metrics::Result<crate::MetricsExporter> {
use crate::MetricsExporter;
Expand All @@ -347,11 +345,7 @@ impl TonicExporterBuilder {

let client = TonicMetricsClient::new(channel, interceptor, compression);

Ok(MetricsExporter::new(
client,
temporality_selector,
aggregation_selector,
))
Ok(MetricsExporter::new(client, temporality_selector))
}

/// Build a new tonic span exporter
Expand Down
3 changes: 1 addition & 2 deletions opentelemetry-otlp/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -126,7 +126,7 @@
//! use opentelemetry::{global, KeyValue, trace::Tracer};
//! use opentelemetry_sdk::{trace::{self, RandomIdGenerator, Sampler}, Resource};
//! # #[cfg(feature = "metrics")]
//! use opentelemetry_sdk::metrics::reader::{DefaultAggregationSelector, DefaultTemporalitySelector};
//! use opentelemetry_sdk::metrics::reader::DefaultTemporalitySelector;
//! use opentelemetry_otlp::{Protocol, WithExportConfig, ExportConfig};
//! use std::time::Duration;
//! # #[cfg(feature = "grpc-tonic")]
Expand Down Expand Up @@ -184,7 +184,6 @@
//! .with_resource(Resource::new(vec![KeyValue::new("service.name", "example")]))
//! .with_period(Duration::from_secs(3))
//! .with_timeout(Duration::from_secs(10))
//! .with_aggregation_selector(DefaultAggregationSelector::new())
//! .with_temporality_selector(DefaultTemporalitySelector::new())
//! .build();
//! # }
Expand Down
35 changes: 4 additions & 31 deletions opentelemetry-otlp/src/metric.rs
Original file line number Diff line number Diff line change
Expand Up @@ -14,11 +14,8 @@ use opentelemetry_sdk::{
metrics::{
data::{ResourceMetrics, Temporality},
exporter::PushMetricsExporter,
reader::{
AggregationSelector, DefaultAggregationSelector, DefaultTemporalitySelector,
TemporalitySelector,
},
Aggregation, InstrumentKind, PeriodicReader, SdkMeterProvider,
reader::{DefaultTemporalitySelector, TemporalitySelector},
InstrumentKind, PeriodicReader, SdkMeterProvider,
},
runtime::Runtime,
Resource,
Expand Down Expand Up @@ -50,7 +47,6 @@ impl OtlpPipeline {
{
OtlpMetricPipeline {
rt,
aggregator_selector: None,
temporality_selector: None,
exporter_pipeline: NoExporterConfig(()),
resource: None,
Expand Down Expand Up @@ -82,21 +78,19 @@ impl MetricsExporterBuilder {
pub fn build_metrics_exporter(
self,
temporality_selector: Box<dyn TemporalitySelector>,
aggregation_selector: Box<dyn AggregationSelector>,
) -> Result<MetricsExporter> {
match self {
#[cfg(feature = "grpc-tonic")]
MetricsExporterBuilder::Tonic(builder) => {
builder.build_metrics_exporter(aggregation_selector, temporality_selector)
builder.build_metrics_exporter(temporality_selector)
}
#[cfg(feature = "http-proto")]
MetricsExporterBuilder::Http(builder) => {
builder.build_metrics_exporter(aggregation_selector, temporality_selector)
builder.build_metrics_exporter(temporality_selector)
}
#[cfg(not(any(feature = "http-proto", feature = "grpc-tonic")))]
MetricsExporterBuilder::Unconfigured => {
drop(temporality_selector);
drop(aggregation_selector);
Err(opentelemetry::metrics::MetricsError::Other(
"no configured metrics exporter, enable `http-proto` or `grpc-tonic` feature to configure a metrics exporter".into(),
))
Expand Down Expand Up @@ -125,7 +119,6 @@ impl From<HttpExporterBuilder> for MetricsExporterBuilder {
/// runtime.
pub struct OtlpMetricPipeline<RT, EB> {
rt: RT,
aggregator_selector: Option<Box<dyn AggregationSelector>>,
temporality_selector: Option<Box<dyn TemporalitySelector>>,
exporter_pipeline: EB,
resource: Option<Resource>,
Expand Down Expand Up @@ -178,14 +171,6 @@ where
pub fn with_delta_temporality(self) -> Self {
self.with_temporality_selector(DeltaTemporalitySelector)
}

/// Build with the given aggregation selector
pub fn with_aggregation_selector<T: AggregationSelector + 'static>(self, selector: T) -> Self {
OtlpMetricPipeline {
aggregator_selector: Some(Box::new(selector)),
..self
}
}
}

impl<RT> OtlpMetricPipeline<RT, NoExporterConfig>
Expand All @@ -200,7 +185,6 @@ where
OtlpMetricPipeline {
exporter_pipeline: pipeline.into(),
rt: self.rt,
aggregator_selector: self.aggregator_selector,
temporality_selector: self.temporality_selector,
resource: self.resource,
period: self.period,
Expand All @@ -218,8 +202,6 @@ where
let exporter = self.exporter_pipeline.build_metrics_exporter(
self.temporality_selector
.unwrap_or_else(|| Box::new(DefaultTemporalitySelector::new())),
self.aggregator_selector
.unwrap_or_else(|| Box::new(DefaultAggregationSelector::new())),
)?;

let mut builder = PeriodicReader::builder(exporter, self.rt);
Expand Down Expand Up @@ -295,7 +277,6 @@ pub trait MetricsClient: fmt::Debug + Send + Sync + 'static {
pub struct MetricsExporter {
client: Box<dyn MetricsClient>,
temporality_selector: Box<dyn TemporalitySelector>,
aggregation_selector: Box<dyn AggregationSelector>,
}

impl Debug for MetricsExporter {
Expand All @@ -310,12 +291,6 @@ impl TemporalitySelector for MetricsExporter {
}
}

impl AggregationSelector for MetricsExporter {
fn aggregation(&self, kind: InstrumentKind) -> Aggregation {
self.aggregation_selector.aggregation(kind)
}
}

#[async_trait]
impl PushMetricsExporter for MetricsExporter {
async fn export(&self, metrics: &mut ResourceMetrics) -> Result<()> {
Expand All @@ -337,12 +312,10 @@ impl MetricsExporter {
pub fn new(
client: impl MetricsClient,
temporality_selector: Box<dyn TemporalitySelector>,
aggregation_selector: Box<dyn AggregationSelector>,
) -> MetricsExporter {
MetricsExporter {
client: Box::new(client),
temporality_selector,
aggregation_selector,
}
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,12 @@
"value": {
"intValue": "100"
}
},
{
"key": "number/int",
"value": {
"intValue": "100"
}
}
],
"droppedAttributesCount": 0
Expand Down
15 changes: 1 addition & 14 deletions opentelemetry-prometheus/src/config.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,7 @@
use core::fmt;
use once_cell::sync::OnceCell;
use opentelemetry::metrics::{MetricsError, Result};
use opentelemetry_sdk::metrics::{
reader::{AggregationSelector, MetricProducer},
ManualReaderBuilder,
};
use opentelemetry_sdk::metrics::{reader::MetricProducer, ManualReaderBuilder};
use std::sync::{Arc, Mutex};

use crate::{Collector, PrometheusExporter, ResourceSelector};
Expand Down Expand Up @@ -105,16 +102,6 @@ impl ExporterBuilder {
self
}

/// Configure the [AggregationSelector] the exporter will use.
///
/// If no selector is provided, the [DefaultAggregationSelector] is used.
///
/// [DefaultAggregationSelector]: opentelemetry_sdk::metrics::reader::DefaultAggregationSelector
pub fn with_aggregation_selector(mut self, agg: impl AggregationSelector + 'static) -> Self {
self.reader = self.reader.with_aggregation_selector(agg);
self
}

/// Configures whether to export resource as attributes with every metric.
///
/// Note that this is orthogonal to the `target_info` metric, which can be disabled using `without_target_info`.
Expand Down
10 changes: 2 additions & 8 deletions opentelemetry-prometheus/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -105,8 +105,8 @@ use opentelemetry::{
use opentelemetry_sdk::{
metrics::{
data::{self, ResourceMetrics, Temporality},
reader::{AggregationSelector, MetricReader, TemporalitySelector},
Aggregation, InstrumentKind, ManualReader, Pipeline,
reader::{MetricReader, TemporalitySelector},
InstrumentKind, ManualReader, Pipeline,
},
Resource, Scope,
};
Expand Down Expand Up @@ -160,12 +160,6 @@ impl TemporalitySelector for PrometheusExporter {
}
}

impl AggregationSelector for PrometheusExporter {
fn aggregation(&self, kind: InstrumentKind) -> Aggregation {
self.reader.aggregation(kind)
}
}

impl MetricReader for PrometheusExporter {
fn register_pipeline(&self, pipeline: Weak<Pipeline>) {
self.reader.register_pipeline(pipeline)
Expand Down
8 changes: 1 addition & 7 deletions opentelemetry-sdk/benches/metric.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ use opentelemetry_sdk::{
metrics::{
data::{ResourceMetrics, Temporality},
new_view,
reader::{AggregationSelector, MetricReader, TemporalitySelector},
reader::{MetricReader, TemporalitySelector},
Aggregation, Instrument, InstrumentKind, ManualReader, Pipeline, SdkMeterProvider, Stream,
View,
},
Expand All @@ -26,12 +26,6 @@ impl TemporalitySelector for SharedReader {
}
}

impl AggregationSelector for SharedReader {
fn aggregation(&self, kind: InstrumentKind) -> Aggregation {
self.0.aggregation(kind)
}
}

impl MetricReader for SharedReader {
fn register_pipeline(&self, pipeline: Weak<Pipeline>) {
self.0.register_pipeline(pipeline)
Expand Down
9 changes: 2 additions & 7 deletions opentelemetry-sdk/src/metrics/exporter.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,18 +3,13 @@ use async_trait::async_trait;

use opentelemetry::metrics::Result;

use crate::metrics::{
data::ResourceMetrics,
reader::{AggregationSelector, TemporalitySelector},
};
use crate::metrics::{data::ResourceMetrics, reader::TemporalitySelector};

/// Exporter handles the delivery of metric data to external receivers.
///
/// This is the final component in the metric push pipeline.
#[async_trait]
pub trait PushMetricsExporter:
AggregationSelector + TemporalitySelector + Send + Sync + 'static
{
pub trait PushMetricsExporter: TemporalitySelector + Send + Sync + 'static {
/// Export serializes and transmits metric data to a receiver.
///
/// All retry logic must be contained in this function. The SDK does not
Expand Down
Loading

0 comments on commit 9a02aab

Please sign in to comment.