OTLP exporter assumes it must use SSL because GetOtlpDefaultIsSslEnable() is returning a wrong value

[MrSparc]

Describe your environment

• opentelemetry-cpp v1.8.3
• OTLP gRPC
• No SSL
• No OTLP env vars, just default settings

Steps to reproduce
I'm facing an issue with SSL after upgrade from v1.8.2 to v1.8.3 with my gRPC telemetry.
The error is:

E0315 15:58:51.193959723  145649 ssl_transport_security.cc:1501] Handshake failed with fatal error SSL_ERROR_SSL: error:1408F10B:SSL routines:ssl3_get_record:wrong version number.

So debugging and comparing the behavior of both versions, I noticed that in v1.8.2 OtlpGrpcExporterOptions constructor is setting as default use_ssl_credentials: false but in the new v1.8.3 this default constructor is setting to use_ssl_credentials: true

This looks like a behaviour change introduced by this refactoring handling Boolean environment variables #1982

Digging into the code to look the initialization of use_ssl_credentials in the constructor:

opentelemetry-cpp/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_grpc_exporter_options.h

Lines 19 to 25 in 15ab9ed

	struct OtlpGrpcExporterOptions
	{
	// The endpoint to export to. By default the OpenTelemetry Collector's default endpoint.
	std::string endpoint = GetOtlpDefaultGrpcEndpoint();
	// By default when false, uses grpc::InsecureChannelCredentials(); If true,
	// uses ssl_credentials_cacert_path if non-empty, else uses ssl_credentials_cacert_as_string
	bool use_ssl_credentials = GetOtlpDefaultIsSslEnable();

This method GetOtlpDefaultIsSslEnable() to initialize use_ssl_credentials is based on GetOtlpDefaultTracesIsInsecure():

opentelemetry-cpp/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h

Lines 60 to 64 in 15ab9ed

	// Compatibility with OTELCPP 1.8.2
	inline bool GetOtlpDefaultIsSslEnable()
	{
	return (!GetOtlpDefaultTracesIsInsecure());
	}

The issue is in the default return value of the following methods:

• GetOtlpDefaultTracesIsInsecure()
  

  opentelemetry-cpp/exporters/otlp/src/otlp_environment.cc

  Line 251 in 15ab9ed

  return false;

• GetOtlpDefaultMetricsIsInsecure()
  

  opentelemetry-cpp/exporters/otlp/src/otlp_environment.cc

  Line 295 in 15ab9ed

  return false;

• GetOtlpDefaultLogsIsInsecure()
  

  opentelemetry-cpp/exporters/otlp/src/otlp_environment.cc

  Line 312 in 15ab9ed

  return false;

All these methods are returning by default false if no env vars are defined.
So a false return means, IS NOT insecure, then IS secure 🤯

I don't know the gRPC transpost must be by default secure or insecure, but if the default is secure then this behaviour was enforced in this version.

According to the specification:
Insecure: Whether to enable client transport security for the exporter's gRPC connection. This option only applies to OTLP/gRPC when an endpoint is provided without the http or https scheme

So, if the gRPC endpoint scheme returns 'http' or 'https' as the following default, I think this value should overrided:

opentelemetry-cpp/exporters/otlp/src/otlp_environment.cc

Lines 78 to 82 in 15ab9ed

	std::string GetOtlpDefaultGrpcTracesEndpoint()
	{
	constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_TRACES_ENDPOINT";
	constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_ENDPOINT";
	constexpr char kDefault[] = "http://localhost:4317";

What is the expected behavior?
Upgrade from v1.8.2 to v1.8.3 without issues.

What is the actual behavior?
Upgrade to v1.8.3 wrongly assumes that gRPC connection are using SSL and fails because no certificates are defined.

Additional context
As workaround I need to set the env var OTEL_EXPORTER_OTLP_INSECURE=true in order to do not use SSL

[marcalff]

@MrSparc

Thanks for the report, and for the detailed analysis.

So debugging and comparing the behavior of both versions, I noticed that in v1.8.2 OtlpGrpcExporterOptions constructor is setting as default use_ssl_credentials: false but in the new v1.8.3 this default constructor is setting to use_ssl_credentials: true

This looks like a behaviour change introduced by this refactoring handling Boolean environment variables #1982

It is, and I am the one to blame here.

According to the specification: Insecure: Whether to enable client transport security for the exporter's gRPC connection. This option only applies to OTLP/gRPC when an endpoint is provided without the http or https scheme

So, if the gRPC endpoint scheme returns 'http' or 'https' as the following default, I think this value should overrided:

So, the important part is:

• This option only applies to OTLP/gRPC when an endpoint is provided without the http or https scheme

The evaluation of OTEL_EXPORTER_OTLP_TRACES_INSECURE to false by default is per the spec.

What was missed is that this variable should not be used if the endpoint provides an http or https scheme, because the scheme takes precedence.

This is a valid bug report.

[marcalff]

Related to this spec change:

• Clarify gRPC insecure option opentelemetry-specification#2476

[MrSparc]

Thanks @marcalff for your fast feedback and the PR proposal to improve it.
The otel-cpp team are the best 👌
Thanks for the amazing work all you are doing for the dev community!

[marcalff]

@MrSparc

Thanks.

A fix is available, ready for review and test if you want to try it out:

• [EXPORTER] GRPC endpoint scheme should take precedence over OTEL_EXPORTER_OTLP_TRACES_INSECURE #2060

Feel free to comment on the PR.

[compoundradius]

Came here to report the same issue, but without the great analysis. Thanks!

[compoundradius]

Here to report that your fix works properly!
Checked against:

git status
On branch fix_otlp_grpc_ssl_default_2058
Your branch is up to date with 'origin/fix_otlp_grpc_ssl_default_2058'.