opentelemetry-dotnet-instrumentation icon indicating copy to clipboard operation
opentelemetry-dotnet-instrumentation copied to clipboard

Published 20 hours ago • verified publisher icon open-telemetry

Fail fast vs silently continue for wrong env var values

Open Kielek opened this issue 2 years ago • 4 comments

Discussion

References

  • "if the user provides a value the SDK does not recognize, the SDK MUST generate a warning and gracefully ignore the setting" - https://github.com/open-telemetry/opentelemetry-specification/blob/1fd2db2ad56668959a2dc99551b76d0ff2fc1e9e/specification/sdk-environment-variables.md?plain=1#L24
  • "The API or SDK MAY fail fast and cause the application to fail on initialization, e.g. because of a bad user config or environment, but MUST NOT cause the application to fail later at run time, e.g. due to dynamic config settings received from the Collector." - https://github.com/open-telemetry/opentelemetry-specification/blob/1fd2db2ad56668959a2dc99551b76d0ff2fc1e9e/specification/error-handling.md?plain=1#L18
  • #875

Solution

Our current solution is to fail fast when given values does not met requirements. There is a second option, silently ignore problems (proffered by sdk-env-variables).

We should decide if we stick with current approach or we change it.

Kielek avatar Jul 19 '22 09:07 Kielek

From my "spec reading" understanding is that the first rule (regarding enum values parsing) overrides the second (regarding initialization error handling) as:

  • MUST is stronger than MAY
  • it is a more concrete scenario

However, I prefer the current behavior as it clearly "shouts" that the given configuration is not supported.

Is it something that should be discussed with the OTel Specification SIG?

pellared avatar Jul 19 '22 10:07 pellared

@mtwo, do you have any comment here? Could you please share with OTel Specification SIG, if needed?

Kielek avatar Jul 20 '22 17:07 Kielek

@Kielek do you feel comfortable presenting this at the spec SIG yourself (I can join as well!)? I'll have trouble presenting it with specific examples of where this has been a problem

mtwo avatar Jul 20 '22 17:07 mtwo

PR created https://github.com/open-telemetry/opentelemetry-specification/pull/2766

pellared avatar Aug 31 '22 11:08 pellared

@open-telemetry/dotnet-instrumentation-approvers

It looks as if it is not a problem as the spec defined the behavior for SDK and not for automatic instrumentation.

pellared avatar Sep 26 '22 15:09 pellared

Closign per SIG discussion.

Kielek avatar Sep 28 '22 16:09 Kielek