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

[EXAMPLES] Periodic exporter interval default value is inconsistent. #108

Closed
marcalff opened this issue Aug 4, 2024 · 1 comment · Fixed by #143
Closed

[EXAMPLES] Periodic exporter interval default value is inconsistent. #108

marcalff opened this issue Aug 4, 2024 · 1 comment · Fixed by #143

Comments

@marcalff
Copy link
Member

marcalff commented Aug 4, 2024

The examples reads as:

    # Configure a periodic metric reader.
    - periodic:
        # Configure delay interval (in milliseconds) between start of two consecutive exports.
        #
        # Environment variable: OTEL_METRIC_EXPORT_INTERVAL
        interval: 5000
        # Configure maximum allowed time (in milliseconds) to export data.
        #
        # Environment variable: OTEL_METRIC_EXPORT_TIMEOUT
        timeout: 30000

Per the spec:

the default value for interval is 60000.

The problem here is not so much the values used (examples are free to use anything), but the fact that this setup is invalid, at least for opentelemetry-cpp:

  • the C++ SDK enforces that timeout < interval

Since every other numerical value provided in examples seem to match defaults, please consider to use the default (60000) for interval, to avoid confusion.

Currently:

[malff@malff-desktop examples]$ grep "interval:" *
anchors.yaml:        interval: 5000
kitchen-sink.yaml:        interval: 5000
sdk-config.yaml:        interval: 60000
sdk-migration-config.yaml:        interval: ${OTEL_METRIC_EXPORT_INTERVAL:-60000}

Files anchors.yaml and kitchen-sink.yaml use 5000 instead.

@jack-berg
Copy link
Member

Good find.

The problem here is not so much the values used (examples are free to use anything)

While examples are free to use anything, I think its helpful for us to have consistent patterns we follow. The sdk-config.yaml and sdk-migration-config.yaml target users, and so their default values do actually matter and should align with defaults. The kitchen-sink.yaml is contrived and can use any values, but its still probably helpful to use the defaults. One problem with the defaults is that they're not always defined. For example, the default value for .attribute_limits.attribute_value_length_limit is undefined. However, we do want to demonstrate setting some value for this in the example to verify we have the correct type in the schema. Maybe we can adopt a strategy of using the default when available, and when unavailable, using some dummy value with a callout that its not the default.

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

Successfully merging a pull request may close this issue.

2 participants