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

Is TimeSpan.Infinite the best default aggregation default for console exporter? #2575

Closed
tomkerkhove opened this issue Nov 9, 2021 · 5 comments · Fixed by #2648
Closed
Assignees
Labels
metrics Metrics signal related question Further information is requested
Milestone

Comments

@tomkerkhove
Copy link
Contributor

Question

Describe your environment.

Local development

What are you trying to achieve?

I was creating a POC to use OpenTelemetry metrics and couldn't get it to work. After trying a lot of things, it turns out that the default aggregation interval is TimeSpan.Infinite.

This means that, by default, the metrics will only be exported when the application shuts down (if the MetricProvider is disposed properly)

Wouldn't it make more sense to use 5 min as a default? What was the reasoning behind this?

@tomkerkhove tomkerkhove added the question Further information is requested label Nov 9, 2021
@tomkerkhove tomkerkhove changed the title Is TimeSpan.Infinite the best default aggregation default? Is TimeSpan.Infinite the best default aggregation default for console exporter? Nov 9, 2021
@pellared
Copy link
Member

pellared commented Nov 9, 2021

when do we deal with infinity? There's always some limit somewhere

By @yurishkuro 😉

@cijothomas cijothomas added the metrics Metrics signal related label Nov 18, 2021
@cijothomas cijothomas added this to the 1.2.0 milestone Nov 18, 2021
@alanwest
Copy link
Member

alanwest commented Nov 18, 2021

Good question. I agree Timeout.Infinite does not make much sense. The spec seems to suggest we should use 60 seconds.

The OTLP exporter currently has a default of 1 second. I think both console and OTLP should use 60 seconds as default.

@tomkerkhove
Copy link
Contributor Author

60 sounds reasonable to me! I'm happy to send a PR for the console one if you are open for it @cijothomas?

@cijothomas
Copy link
Member

@tomkerkhove I am afraid its not a simple change Infinite to 60 sec.
See #2641 (comment)

@tomkerkhove
Copy link
Contributor Author

Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
metrics Metrics signal related question Further information is requested
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants