Skip to content
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

Configure exporter default aggregation and temporality preference #107

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

jack-berg
Copy link
Member

With open-telemetry/opentelemetry-specification#4142 merged, the spec now has explicit configuration options for each metric exporter's temporality preference and default temporality.

This PR extends the metric exporters with new properties:

  • console exporter has new temporality_preference and default_aggregation_by_instrument_kind properties
  • prometheus exporter has new default_aggregation_by_instrument_kind property
  • otlp exporter's default_histogram_aggregation property migrates to new more expressive default_aggregation_by_instrument_kind property

This supersedes #99.

@jack-berg jack-berg requested a review from a team July 24, 2024 19:57
}
}
},
"AggregationTemporalityPreference": {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The spec says that aggregation temporality should be configurable as a function of instrument kind, but most users will want to use these shorthand options.

This is much simpler / more compact than writing the preference for each instrument kind. In the future, we can add a separate property to give full control over temporality by instrument kind if needed.

}
}
},
"Aggregation": {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lifting this aggregation type def up from the view stream definition. Its now reused both for view stream config and in exporter default aggregation config.

@@ -32,7 +32,9 @@ meter_provider:
<<: *otlp-exporter
endpoint: http://localhost:4318/v1/metrics
temporality_preference: delta
default_histogram_aggregation: base2_exponential_bucket_histogram
default_aggregation_by_instrument_kind:
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what if I want to

  • have an aggregation preference for all kinds?
  • have settings that apply for all exporters?

I see that there is a yaml anchor - but I'm not sure this should be the only option to share values

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Using anchors makes using the config easier. No need to merge properties from some global exporter config with per-exporter config. And simplifies the schema.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's a lesser known and complicated feature

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants