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

[EXPORTER] Boolean environment variables not parsed per the spec #1859

Closed
marcalff opened this issue Dec 13, 2022 · 1 comment · Fixed by #1982
Closed

[EXPORTER] Boolean environment variables not parsed per the spec #1859

marcalff opened this issue Dec 13, 2022 · 1 comment · Fixed by #1982
Assignees
Labels
area:exporter bug Something isn't working spec-compliance Not compliant to OpenTelemetry specs

Comments

@marcalff
Copy link
Member

Per the spec:

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

boolean environment variables should accept "true" or "false", with case insensitive comparison.

Other values should cause a warnings.

Per the code:

inline bool GetOtlpDefaultIsSslEnable()
{
  constexpr char kOtlpTracesIsSslEnableEnv[] = "OTEL_EXPORTER_OTLP_TRACES_SSL_ENABLE";
  constexpr char kOtlpIsSslEnableEnv[]       = "OTEL_EXPORTER_OTLP_SSL_ENABLE";

  auto ssl_enable = opentelemetry::sdk::common::GetEnvironmentVariable(kOtlpTracesIsSslEnableEnv);
  if (ssl_enable.empty())
  {
    ssl_enable = opentelemetry::sdk::common::GetEnvironmentVariable(kOtlpIsSslEnableEnv);
  }
  if (ssl_enable == "True" || ssl_enable == "TRUE" || ssl_enable == "true" || ssl_enable == "1")
  {
    return true;
  }
  return false;
}

Issues:

  • the string comparison is not case insensitive, for example TrUe should be true
  • no warning is raised

Beside, this code has been copied and pasted all other the place, some cleanup is needed.

@marcalff marcalff added the bug Something isn't working label Dec 13, 2022
@marcalff marcalff self-assigned this Dec 13, 2022
@marcalff marcalff added spec-compliance Not compliant to OpenTelemetry specs area:exporter labels Dec 13, 2022
@github-actions
Copy link

This issue was marked as stale due to lack of activity.

@github-actions github-actions bot added the Stale label Feb 12, 2023
@marcalff marcalff removed the Stale label Feb 13, 2023
marcalff added a commit to marcalff/opentelemetry-cpp that referenced this issue Feb 13, 2023
[EXPORTER] Boolean environment variables not parsed per the spec
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:exporter bug Something isn't working spec-compliance Not compliant to OpenTelemetry specs
Projects
None yet
Development

Successfully merging a pull request may close this issue.

1 participant