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

Added metricExportTimeoutMillis to google-cloud.md #818

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

chees
Copy link
Contributor

@chees chees commented Aug 26, 2024

Added metricExportTimeoutMillis: 5_000, to the telemetryConfig. This is needed when you define metricExportIntervalMillis like in the example. Without it won't run due to errors.

Added `metricExportTimeoutMillis: 5_000,` to the telemetryConfig. This is needed when you define `metricExportIntervalMillis` like in the example. Without it won't run due to errors.
@@ -88,6 +88,7 @@ googleCloud({
'@opentelemetry/instrumentation-net': { enabled: false },
},
metricExportIntervalMillis: 5_000,
metricExportTimeoutMillis: 5_000,
Copy link
Contributor

Choose a reason for hiding this comment

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

The default values for both of these fields is 300_000. I'd suggest having this example use those defaults explicitly. Using 5s intervals is useful during development, but could end up using a lot more metric quota than is needed if deployed to production.

My guess is that this example was failing because the export interval would be lower than the default timeout.

Copy link
Member

Choose a reason for hiding this comment

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

@chees if you can make the change above, we'll get this merged in asap. Thanks!

@MichaelDoyle
Copy link
Member

Quick ping @chees if you can make the change above, we'll get this merged in asap. Thanks!

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