-
Notifications
You must be signed in to change notification settings - Fork 861
feat(exporter/prometheus): add default_aggregation parameter to PrometheusMetricReader #5117
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from 3 commits
c6c9fb6
7496e21
a3a346d
43ac409
9c9e127
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -105,6 +105,9 @@ | |
| MetricsData, | ||
| Sum, | ||
| ) | ||
| from opentelemetry.sdk.metrics.view import ( | ||
| Aggregation, | ||
| ) | ||
| from opentelemetry.semconv._incubating.attributes.otel_attributes import ( | ||
| OtelComponentTypeValues, | ||
| ) | ||
|
|
@@ -135,7 +138,10 @@ class PrometheusMetricReader(MetricReader): | |
| """Prometheus metric exporter for OpenTelemetry.""" | ||
|
|
||
| def __init__( | ||
| self, disable_target_info: bool = False, prefix: str = "" | ||
| self, | ||
| disable_target_info: bool = False, | ||
| prefix: str = "", | ||
| default_aggregation: dict[type, Aggregation] | None = None, | ||
| ) -> None: | ||
| super().__init__( | ||
| preferred_temporality={ | ||
|
|
@@ -146,6 +152,7 @@ def __init__( | |
| ObservableUpDownCounter: AggregationTemporality.CUMULATIVE, | ||
| ObservableGauge: AggregationTemporality.CUMULATIVE, | ||
| }, | ||
| preferred_aggregation=default_aggregation, | ||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why call this
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Renamed to preferred_aggregation for consistency. |
||
| otel_component_type=OtelComponentTypeValues.PROMETHEUS_HTTP_TEXT_METRIC_EXPORTER, | ||
| ) | ||
| self._collector = _CustomCollector( | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -28,6 +28,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _CustomCollector, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from opentelemetry.metrics import NoOpMeterProvider | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from opentelemetry.sdk.metrics import Histogram as HistogramInstrument | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from opentelemetry.sdk.metrics import MeterProvider | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from opentelemetry.sdk.metrics.export import ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| AggregationTemporality, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -38,6 +39,7 @@ | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ResourceMetrics, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ScopeMetrics, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from opentelemetry.sdk.metrics.view import ExplicitBucketHistogramAggregation | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from opentelemetry.sdk.resources import Resource | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from opentelemetry.test.metrictestutil import ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _generate_gauge, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
@@ -719,3 +721,21 @@ def test_multiple_data_points_with_different_label_sets(self): | |||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ), | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def test_default_aggregation(self): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| """Test that default_aggregation parameter is passed to MetricReader.""" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| custom_aggregation = { | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| HistogramInstrument: ExplicitBucketHistogramAggregation( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| boundaries=[1.0, 5.0, 10.0] | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| reader = PrometheusMetricReader( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| default_aggregation=custom_aggregation | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| self.assertEqual( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| reader._instrument_class_aggregation[ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| HistogramInstrument | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ].__class__, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ExplicitBucketHistogramAggregation, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| reader.shutdown() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Indentation is wrong, this won't get run.
Suggested change
Contributor
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Fixed the indentation and updated the test to use public API instead of internal attributes |
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think #5109 is the correct issue number here, #5117 is the PR number. Please correct me if I'm wrong.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We prefer the changelog to use the PR number, not the associated issue number. You're welcome to add the issue number to the PR description if it's not there already 🙂
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for clarifying. Updated the changelog to use the PR number and also renamed default_aggregation to preferred_aggregation in the changelog entry.