Skip to content

Commit 299c1ba

Browse files
frailltMindaugas Vinkelis
authored and
Mindaugas Vinkelis
committed
AggregationSelector is not needed
1 parent 38f7fd5 commit 299c1ba

File tree

17 files changed

+69
-261
lines changed

17 files changed

+69
-261
lines changed

opentelemetry-otlp/src/exporter/http/mod.rs

+2-8
Original file line numberDiff line numberDiff line change
@@ -78,7 +78,7 @@ impl Default for HttpConfig {
7878
/// ```
7979
/// # #[cfg(feature="metrics")]
8080
/// use opentelemetry_sdk::metrics::reader::{
81-
/// DefaultAggregationSelector, DefaultTemporalitySelector,
81+
/// DefaultTemporalitySelector,
8282
/// };
8383
///
8484
/// # fn main() -> Result<(), Box<dyn std::error::Error>> {
@@ -91,7 +91,6 @@ impl Default for HttpConfig {
9191
/// let metrics_exporter = opentelemetry_otlp::new_exporter()
9292
/// .http()
9393
/// .build_metrics_exporter(
94-
/// Box::new(DefaultAggregationSelector::new()),
9594
/// Box::new(DefaultTemporalitySelector::new()),
9695
/// )?;
9796
///
@@ -252,7 +251,6 @@ impl HttpExporterBuilder {
252251
#[cfg(feature = "metrics")]
253252
pub fn build_metrics_exporter(
254253
mut self,
255-
aggregation_selector: Box<dyn opentelemetry_sdk::metrics::reader::AggregationSelector>,
256254
temporality_selector: Box<dyn opentelemetry_sdk::metrics::reader::TemporalitySelector>,
257255
) -> opentelemetry::metrics::Result<crate::MetricsExporter> {
258256
use crate::{
@@ -267,11 +265,7 @@ impl HttpExporterBuilder {
267265
OTEL_EXPORTER_OTLP_METRICS_HEADERS,
268266
)?;
269267

270-
Ok(crate::MetricsExporter::new(
271-
client,
272-
temporality_selector,
273-
aggregation_selector,
274-
))
268+
Ok(crate::MetricsExporter::new(client, temporality_selector))
275269
}
276270
}
277271

opentelemetry-otlp/src/exporter/tonic/mod.rs

+2-8
Original file line numberDiff line numberDiff line change
@@ -97,7 +97,7 @@ fn resolve_compression(
9797
/// ```no_run
9898
/// # #[cfg(feature="metrics")]
9999
/// use opentelemetry_sdk::metrics::reader::{
100-
/// DefaultAggregationSelector, DefaultTemporalitySelector,
100+
/// DefaultTemporalitySelector,
101101
/// };
102102
///
103103
/// # fn main() -> Result<(), Box<dyn std::error::Error>> {
@@ -110,7 +110,6 @@ fn resolve_compression(
110110
/// let metrics_exporter = opentelemetry_otlp::new_exporter()
111111
/// .tonic()
112112
/// .build_metrics_exporter(
113-
/// Box::new(DefaultAggregationSelector::new()),
114113
/// Box::new(DefaultTemporalitySelector::new()),
115114
/// )?;
116115
///
@@ -332,7 +331,6 @@ impl TonicExporterBuilder {
332331
#[cfg(feature = "metrics")]
333332
pub fn build_metrics_exporter(
334333
self,
335-
aggregation_selector: Box<dyn opentelemetry_sdk::metrics::reader::AggregationSelector>,
336334
temporality_selector: Box<dyn opentelemetry_sdk::metrics::reader::TemporalitySelector>,
337335
) -> opentelemetry::metrics::Result<crate::MetricsExporter> {
338336
use crate::MetricsExporter;
@@ -347,11 +345,7 @@ impl TonicExporterBuilder {
347345

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

350-
Ok(MetricsExporter::new(
351-
client,
352-
temporality_selector,
353-
aggregation_selector,
354-
))
348+
Ok(MetricsExporter::new(client, temporality_selector))
355349
}
356350

357351
/// Build a new tonic span exporter

opentelemetry-otlp/src/lib.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -126,7 +126,7 @@
126126
//! use opentelemetry::{global, KeyValue, trace::Tracer};
127127
//! use opentelemetry_sdk::{trace::{self, RandomIdGenerator, Sampler}, Resource};
128128
//! # #[cfg(feature = "metrics")]
129-
//! use opentelemetry_sdk::metrics::reader::{DefaultAggregationSelector, DefaultTemporalitySelector};
129+
//! use opentelemetry_sdk::metrics::reader::DefaultTemporalitySelector;
130130
//! use opentelemetry_otlp::{Protocol, WithExportConfig, ExportConfig};
131131
//! use std::time::Duration;
132132
//! # #[cfg(feature = "grpc-tonic")]
@@ -184,7 +184,6 @@
184184
//! .with_resource(Resource::new(vec![KeyValue::new("service.name", "example")]))
185185
//! .with_period(Duration::from_secs(3))
186186
//! .with_timeout(Duration::from_secs(10))
187-
//! .with_aggregation_selector(DefaultAggregationSelector::new())
188187
//! .with_temporality_selector(DefaultTemporalitySelector::new())
189188
//! .build();
190189
//! # }

opentelemetry-otlp/src/metric.rs

+4-31
Original file line numberDiff line numberDiff line change
@@ -14,11 +14,8 @@ use opentelemetry_sdk::{
1414
metrics::{
1515
data::{ResourceMetrics, Temporality},
1616
exporter::PushMetricsExporter,
17-
reader::{
18-
AggregationSelector, DefaultAggregationSelector, DefaultTemporalitySelector,
19-
TemporalitySelector,
20-
},
21-
Aggregation, InstrumentKind, PeriodicReader, SdkMeterProvider,
17+
reader::{DefaultTemporalitySelector, TemporalitySelector},
18+
InstrumentKind, PeriodicReader, SdkMeterProvider,
2219
},
2320
runtime::Runtime,
2421
Resource,
@@ -50,7 +47,6 @@ impl OtlpPipeline {
5047
{
5148
OtlpMetricPipeline {
5249
rt,
53-
aggregator_selector: None,
5450
temporality_selector: None,
5551
exporter_pipeline: NoExporterConfig(()),
5652
resource: None,
@@ -82,21 +78,19 @@ impl MetricsExporterBuilder {
8278
pub fn build_metrics_exporter(
8379
self,
8480
temporality_selector: Box<dyn TemporalitySelector>,
85-
aggregation_selector: Box<dyn AggregationSelector>,
8681
) -> Result<MetricsExporter> {
8782
match self {
8883
#[cfg(feature = "grpc-tonic")]
8984
MetricsExporterBuilder::Tonic(builder) => {
90-
builder.build_metrics_exporter(aggregation_selector, temporality_selector)
85+
builder.build_metrics_exporter(temporality_selector)
9186
}
9287
#[cfg(feature = "http-proto")]
9388
MetricsExporterBuilder::Http(builder) => {
94-
builder.build_metrics_exporter(aggregation_selector, temporality_selector)
89+
builder.build_metrics_exporter(temporality_selector)
9590
}
9691
#[cfg(not(any(feature = "http-proto", feature = "grpc-tonic")))]
9792
MetricsExporterBuilder::Unconfigured => {
9893
drop(temporality_selector);
99-
drop(aggregation_selector);
10094
Err(opentelemetry::metrics::MetricsError::Other(
10195
"no configured metrics exporter, enable `http-proto` or `grpc-tonic` feature to configure a metrics exporter".into(),
10296
))
@@ -125,7 +119,6 @@ impl From<HttpExporterBuilder> for MetricsExporterBuilder {
125119
/// runtime.
126120
pub struct OtlpMetricPipeline<RT, EB> {
127121
rt: RT,
128-
aggregator_selector: Option<Box<dyn AggregationSelector>>,
129122
temporality_selector: Option<Box<dyn TemporalitySelector>>,
130123
exporter_pipeline: EB,
131124
resource: Option<Resource>,
@@ -178,14 +171,6 @@ where
178171
pub fn with_delta_temporality(self) -> Self {
179172
self.with_temporality_selector(DeltaTemporalitySelector)
180173
}
181-
182-
/// Build with the given aggregation selector
183-
pub fn with_aggregation_selector<T: AggregationSelector + 'static>(self, selector: T) -> Self {
184-
OtlpMetricPipeline {
185-
aggregator_selector: Some(Box::new(selector)),
186-
..self
187-
}
188-
}
189174
}
190175

191176
impl<RT> OtlpMetricPipeline<RT, NoExporterConfig>
@@ -200,7 +185,6 @@ where
200185
OtlpMetricPipeline {
201186
exporter_pipeline: pipeline.into(),
202187
rt: self.rt,
203-
aggregator_selector: self.aggregator_selector,
204188
temporality_selector: self.temporality_selector,
205189
resource: self.resource,
206190
period: self.period,
@@ -218,8 +202,6 @@ where
218202
let exporter = self.exporter_pipeline.build_metrics_exporter(
219203
self.temporality_selector
220204
.unwrap_or_else(|| Box::new(DefaultTemporalitySelector::new())),
221-
self.aggregator_selector
222-
.unwrap_or_else(|| Box::new(DefaultAggregationSelector::new())),
223205
)?;
224206

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

301282
impl Debug for MetricsExporter {
@@ -310,12 +291,6 @@ impl TemporalitySelector for MetricsExporter {
310291
}
311292
}
312293

313-
impl AggregationSelector for MetricsExporter {
314-
fn aggregation(&self, kind: InstrumentKind) -> Aggregation {
315-
self.aggregation_selector.aggregation(kind)
316-
}
317-
}
318-
319294
#[async_trait]
320295
impl PushMetricsExporter for MetricsExporter {
321296
async fn export(&self, metrics: &mut ResourceMetrics) -> Result<()> {
@@ -337,12 +312,10 @@ impl MetricsExporter {
337312
pub fn new(
338313
client: impl MetricsClient,
339314
temporality_selector: Box<dyn TemporalitySelector>,
340-
aggregation_selector: Box<dyn AggregationSelector>,
341315
) -> MetricsExporter {
342316
MetricsExporter {
343317
client: Box::new(client),
344318
temporality_selector,
345-
aggregation_selector,
346319
}
347320
}
348321
}

opentelemetry-otlp/tests/integration_test/expected/serialized_traces.json

+6
Original file line numberDiff line numberDiff line change
@@ -112,6 +112,12 @@
112112
"value": {
113113
"intValue": "100"
114114
}
115+
},
116+
{
117+
"key": "number/int",
118+
"value": {
119+
"intValue": "100"
120+
}
115121
}
116122
],
117123
"droppedAttributesCount": 0

opentelemetry-prometheus/src/config.rs

+1-14
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,7 @@
11
use core::fmt;
22
use once_cell::sync::OnceCell;
33
use opentelemetry::metrics::{MetricsError, Result};
4-
use opentelemetry_sdk::metrics::{
5-
reader::{AggregationSelector, MetricProducer},
6-
ManualReaderBuilder,
7-
};
4+
use opentelemetry_sdk::metrics::{reader::MetricProducer, ManualReaderBuilder};
85
use std::sync::{Arc, Mutex};
96

107
use crate::{Collector, PrometheusExporter, ResourceSelector};
@@ -105,16 +102,6 @@ impl ExporterBuilder {
105102
self
106103
}
107104

108-
/// Configure the [AggregationSelector] the exporter will use.
109-
///
110-
/// If no selector is provided, the [DefaultAggregationSelector] is used.
111-
///
112-
/// [DefaultAggregationSelector]: opentelemetry_sdk::metrics::reader::DefaultAggregationSelector
113-
pub fn with_aggregation_selector(mut self, agg: impl AggregationSelector + 'static) -> Self {
114-
self.reader = self.reader.with_aggregation_selector(agg);
115-
self
116-
}
117-
118105
/// Configures whether to export resource as attributes with every metric.
119106
///
120107
/// Note that this is orthogonal to the `target_info` metric, which can be disabled using `without_target_info`.

opentelemetry-prometheus/src/lib.rs

+2-8
Original file line numberDiff line numberDiff line change
@@ -105,8 +105,8 @@ use opentelemetry::{
105105
use opentelemetry_sdk::{
106106
metrics::{
107107
data::{self, ResourceMetrics, Temporality},
108-
reader::{AggregationSelector, MetricReader, TemporalitySelector},
109-
Aggregation, InstrumentKind, ManualReader, Pipeline,
108+
reader::{MetricReader, TemporalitySelector},
109+
InstrumentKind, ManualReader, Pipeline,
110110
},
111111
Resource, Scope,
112112
};
@@ -160,12 +160,6 @@ impl TemporalitySelector for PrometheusExporter {
160160
}
161161
}
162162

163-
impl AggregationSelector for PrometheusExporter {
164-
fn aggregation(&self, kind: InstrumentKind) -> Aggregation {
165-
self.reader.aggregation(kind)
166-
}
167-
}
168-
169163
impl MetricReader for PrometheusExporter {
170164
fn register_pipeline(&self, pipeline: Weak<Pipeline>) {
171165
self.reader.register_pipeline(pipeline)

opentelemetry-sdk/benches/metric.rs

+1-7
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@ use opentelemetry_sdk::{
1010
metrics::{
1111
data::{ResourceMetrics, Temporality},
1212
new_view,
13-
reader::{AggregationSelector, MetricReader, TemporalitySelector},
13+
reader::{MetricReader, TemporalitySelector},
1414
Aggregation, Instrument, InstrumentKind, ManualReader, Pipeline, SdkMeterProvider, Stream,
1515
View,
1616
},
@@ -26,12 +26,6 @@ impl TemporalitySelector for SharedReader {
2626
}
2727
}
2828

29-
impl AggregationSelector for SharedReader {
30-
fn aggregation(&self, kind: InstrumentKind) -> Aggregation {
31-
self.0.aggregation(kind)
32-
}
33-
}
34-
3529
impl MetricReader for SharedReader {
3630
fn register_pipeline(&self, pipeline: Weak<Pipeline>) {
3731
self.0.register_pipeline(pipeline)

opentelemetry-sdk/src/metrics/aggregation.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -17,10 +17,10 @@ pub enum Aggregation {
1717
/// instrument kind that differs from the default. This aggregation ensures the
1818
/// default is used.
1919
///
20-
/// See the [DefaultAggregationSelector] for information about the default
20+
/// See the [the spec] for information about the default
2121
/// instrument kind selection mapping.
2222
///
23-
/// [DefaultAggregationSelector]: crate::metrics::reader::DefaultAggregationSelector
23+
/// [the spec]: https://github.com/open-telemetry/opentelemetry-specification/blob/v1.19.0/specification/metrics/sdk.md#default-aggregation
2424
Default,
2525

2626
/// An aggregation that summarizes a set of measurements as their arithmetic

opentelemetry-sdk/src/metrics/exporter.rs

+2-7
Original file line numberDiff line numberDiff line change
@@ -3,18 +3,13 @@ use async_trait::async_trait;
33

44
use opentelemetry::metrics::Result;
55

6-
use crate::metrics::{
7-
data::ResourceMetrics,
8-
reader::{AggregationSelector, TemporalitySelector},
9-
};
6+
use crate::metrics::{data::ResourceMetrics, reader::TemporalitySelector};
107

118
/// Exporter handles the delivery of metric data to external receivers.
129
///
1310
/// This is the final component in the metric push pipeline.
1411
#[async_trait]
15-
pub trait PushMetricsExporter:
16-
AggregationSelector + TemporalitySelector + Send + Sync + 'static
17-
{
12+
pub trait PushMetricsExporter: TemporalitySelector + Send + Sync + 'static {
1813
/// Export serializes and transmits metric data to a receiver.
1914
///
2015
/// All retry logic must be contained in this function. The SDK does not

0 commit comments

Comments
 (0)