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

NH-70998 Set histogram preferred temporality delta #296

Merged
merged 2 commits into from
Feb 1, 2024

Conversation

tammy-baylis-swi
Copy link
Contributor

@tammy-baylis-swi tammy-baylis-swi commented Feb 1, 2024

Depends on #289. EDIT: that's done thanks!

Sets preference for all histograms to report using delta aggregation, including response_time.

The entry point load is called for any metrics exporters specified by OTEL_METRICS_EXPORTER. Otel Python's otlp metrics exporters for http and grpc both accept optional preferred_temporality. I don't think there are others in Otel Python. If someone configures a custom metrics exporter, it needs to also accept temporality or general **kwargs, else this might error.

Please let me know what you think!

@tammy-baylis-swi tammy-baylis-swi requested a review from a team as a code owner February 1, 2024 01:37
@tammy-baylis-swi tammy-baylis-swi changed the title Set histogram preferred temporality delta NH-70998 Set histogram preferred temporality delta Feb 1, 2024
Base automatically changed from NH-70998-reorder-otlp-metrics-setup to main February 1, 2024 18:52
Copy link
Contributor

@cheempz cheempz left a comment

Choose a reason for hiding this comment

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

LGTM, thanks @tammy-baylis-swi for highlighting the impact of this change.

From quick read of info online, the temporality to choose comes down to what the backend supports--since we're building that out in SWO based on delta, and metrics exporting is still behind the experimental setting, it seems fine to go with setting it on all exporters for now.

In the future we may want more flexibility which might only be possible by creating a SWO-specific exporter.

@tammy-baylis-swi tammy-baylis-swi merged commit 22556f6 into main Feb 1, 2024
9 checks passed
@tammy-baylis-swi tammy-baylis-swi deleted the NH-70998-set-preferred-temporality branch February 1, 2024 22:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants