FormatException is thrown when an invalid env var value is set

[pellared]

According the OTel spec:

For variables accepting an enum value, if the user provides a value the SDK does not recognize, the SDK MUST generate a warning and gracefully ignore the setting.

This is not what is done in multiple places like here.

Moreover, because of this, it won't be easy to set defaults for these values in .NET Automatic Instrumentation.

Can/do we want to address this? Won't it be seen as a breaking change?

Reference: open-telemetry/opentelemetry-specification#2766

CC @reyang @cijothomas @CodeBlanch

[reyang]

This is not what is done in multiple places like here.

The link is broken?

Moreover, because of this, it won't be easy to set defaults for these values in .NET Automatic Instrumentation.

Can/do we want to address this? Won't it be seen as a breaking change?

This looks like an implementation bug to me. Guess the fix should be swallow the exception and fallback to a default value. Doesn't sound like a breaking change.

[pellared]

@reyang Thanks. I fixed the link. Feel free to hide/delete this comment

[moonbox3]

I see in the example code you linked to @pellared, the Protocol property has a default value (OtlpExportProtocol.Grpc):

public OtlpExportProtocol Protocol { get; set; } = DefaultOtlpExportProtocol;

Based on @reyang's comment to swallow the exception and fallback to a default value would mean we can essentially remove the else block where the FormatException is thrown, correct? Thus, the code would attempt to parse the proper protocol, and if it can't, the fallback value will be that default value of OtlpExportProtocol.Grpc.

I'm new to the project and if my understanding is incorrect, please let me know. If my understanding is correct, I can work on the fix. Thanks!

[moonbox3]

@pellared I've been looking through the code and it looks like this issue has been handled in the following PRs:

• [SDK + Jaeger] Support loading environment variables from IConfiguration in Traces & Metrics #3720
• [SDK + Otlp] Support loading envvars from IConfiguration #3760

I've done a more thorough search for any remaining clean-up that may be needed and I am not finding anything. It feels safe to close this issue.

[pellared]

Thanks

[pellared]

@moonbox3 Reopening; see:

• opentelemetry-dotnet/src/OpenTelemetry/Internal/ConfigurationExtensions.cs

  Line 71 in 39283c7

  throw new FormatException($"{key} environment variable has an invalid value: '{stringValue}'");

• opentelemetry-dotnet/src/OpenTelemetry/Internal/ConfigurationExtensions.cs

  Line 90 in 39283c7

  throw new FormatException($"{key} environment variable has an invalid value: '{stringValue}'");

Also the docs are not updated (find: FormatException)

There are even some tests which checks if FormatException is thrown.

[TimothyMothra]

Hi, I'm interested in working on this.
Seems pretty straightforward, I found the tests and the related docs.