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

Spec compliance - bool env vars SHOULD default to false #2090

Closed
pellared opened this issue Jan 27, 2023 · 3 comments
Closed

Spec compliance - bool env vars SHOULD default to false #2090

pellared opened this issue Jan 27, 2023 · 3 comments

Comments

@pellared
Copy link
Member

pellared commented Jan 27, 2023

https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/sdk-environment-variables.md#boolean-value

All Boolean environment variables SHOULD be named and defined such that false is the expected safe default behavior. Renaming or changing the default value MUST NOT happen without a major version upgrade.

Main reasoning by @MrAlias: open-telemetry/opentelemetry-specification#2755 (comment)

Removing user confusion is key. User confusion around if they are or are not required to provide an environment variable diminishes when they know that by not providing it it will be assumed false. They do not need to go look up documentation elsewhere about the value, they just know because that is the OTel default.

Similarly, they know these values will always have the default of false. If they upgrade their use of OTel they know that default value will not change. It would be a huge frustration for a user to upgrade their system and all of a sudden they now need to set OTEL_SDK_DISABLED=false because we allowed the default to change.

If there does exist a valid example where a boolean variable needs to have it's default value changed, though I have yet to hear one, it still makes sense from the user perspective to keep static defaults. A new variable would be defined and the old variable deprecated. Old code would still work, maybe with a logged error to encourage moving the new value, and the new configuration would also work. Still the user is able to rely on each defaulting to false without further documentation lookup. If you version past a major version and you drop support for the old configuration variable, then you have kept in line with this projects versioning and stability guarantees. Allowing the default value to change would violate these guarantees.

@pellared
Copy link
Member Author

Even though it is a SHOULD I think it is probably better to follow the recommendation as it would probably make the TC GA review easier.

@pjanotti
Copy link
Contributor

Related #2088 (comment)

@nrcventura
Copy link
Member

Closing per SIG meeting.

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

No branches or pull requests

3 participants