From 211d9c92d095ecd4f0782f4cddf3358b5caecdbf Mon Sep 17 00:00:00 2001 From: Marc Alff Date: Thu, 16 Mar 2023 16:20:35 +0100 Subject: [PATCH] [EXPORTER] GRPC endpoint scheme should take precedence over OTEL_EXPORTER_OTLP_TRACES_INSECURE (#2060) --- CHANGELOG.md | 25 +++++++ .../exporters/otlp/otlp_environment.h | 8 +-- exporters/otlp/src/otlp_environment.cc | 48 ++++++++++++- .../otlp/test/otlp_grpc_exporter_test.cc | 72 ++++++++++++++++++- 4 files changed, 145 insertions(+), 8 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 9ea45e1f48..94daec5ea3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -17,6 +17,31 @@ Increment the: * [RESOURCE SDK] Fix schema URL precedence bug in `Resource::Merge`. [#2036](https://github.com/open-telemetry/opentelemetry-cpp/pull/2036) +* [EXPORTER] GRPC endpoint scheme should take precedence over OTEL_EXPORTER_OTLP_TRACES_INSECURE + [#2060](https://github.com/open-telemetry/opentelemetry-cpp/pull/2060) + +Important changes: + +* [EXPORTER] GRPC endpoint scheme should take precedence over OTEL_EXPORTER_OTLP_TRACES_INSECURE + [#2060](https://github.com/open-telemetry/opentelemetry-cpp/pull/2060) + * The logic to decide whether or not an OTLP GRPC exporter uses SSL has + changed to comply with the specification: + * Before this change, the following settings were evaluated, in order: + * OTEL_EXPORTER_OTLP_TRACES_INSECURE (starting with 1.8.3) + * OTEL_EXPORTER_OTLP_INSECURE (starting with 1.8.3) + * OTEL_EXPORTER_OTLP_TRACES_SSL_ENABLE + * OTEL_EXPORTER_OTLP_SSL_ENABLE + * With this change, the following settings are evaluated, in order: + * The GRPC endpoint scheme, if provided: + * "https" imply with SSL, + * "http" imply without ssl. + * OTEL_EXPORTER_OTLP_TRACES_INSECURE + * OTEL_EXPORTER_OTLP_INSECURE + * OTEL_EXPORTER_OTLP_TRACES_SSL_ENABLE + * OTEL_EXPORTER_OTLP_SSL_ENABLE + * As a result, a behavior change for GRPC SSL is possible, + because the endpoint scheme now takes precedence. + Please verify configuration settings for the GRPC endpoint. ## [1.8.3] 2023-03-06 diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h index e517f256b4..c420529a64 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h @@ -53,14 +53,14 @@ inline std::string GetOtlpDefaultMetricsEndpoint() return GetOtlpDefaultHttpMetricsEndpoint(); } -bool GetOtlpDefaultTracesIsInsecure(); -bool GetOtlpDefaultMetricsIsInsecure(); -bool GetOtlpDefaultLogsIsInsecure(); +bool GetOtlpDefaultGrpcTracesIsInsecure(); +bool GetOtlpDefaultGrpcMetricsIsInsecure(); +bool GetOtlpDefaultGrpcLogsIsInsecure(); // Compatibility with OTELCPP 1.8.2 inline bool GetOtlpDefaultIsSslEnable() { - return (!GetOtlpDefaultTracesIsInsecure()); + return (!GetOtlpDefaultGrpcTracesIsInsecure()); } std::string GetOtlpDefaultTracesSslCertificatePath(); diff --git a/exporters/otlp/src/otlp_environment.cc b/exporters/otlp/src/otlp_environment.cc index f47d7c3720..8bcb2c3aa0 100644 --- a/exporters/otlp/src/otlp_environment.cc +++ b/exporters/otlp/src/otlp_environment.cc @@ -207,8 +207,22 @@ std::string GetOtlpDefaultHttpLogsEndpoint() return kDefault; } -bool GetOtlpDefaultTracesIsInsecure() +bool GetOtlpDefaultGrpcTracesIsInsecure() { + std::string endpoint = GetOtlpDefaultGrpcTracesEndpoint(); + + /* The trace endpoint, when providing a scheme, takes precedence. */ + + if (endpoint.substr(0, 6) == "https:") + { + return false; + } + + if (endpoint.substr(0, 5) == "http:") + { + return true; + } + constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_TRACES_INSECURE"; constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_INSECURE"; constexpr char kOldSignalEnv[] = "OTEL_EXPORTER_OTLP_TRACES_SSL_ENABLE"; @@ -251,8 +265,22 @@ bool GetOtlpDefaultTracesIsInsecure() return false; } -bool GetOtlpDefaultMetricsIsInsecure() +bool GetOtlpDefaultGrpcMetricsIsInsecure() { + std::string endpoint = GetOtlpDefaultGrpcMetricsEndpoint(); + + /* The metrics endpoint, when providing a scheme, takes precedence. */ + + if (endpoint.substr(0, 6) == "https:") + { + return false; + } + + if (endpoint.substr(0, 5) == "http:") + { + return true; + } + constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_METRICS_INSECURE"; constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_INSECURE"; constexpr char kOldSignalEnv[] = "OTEL_EXPORTER_OTLP_METRICS_SSL_ENABLE"; @@ -295,8 +323,22 @@ bool GetOtlpDefaultMetricsIsInsecure() return false; } -bool GetOtlpDefaultLogsIsInsecure() +bool GetOtlpDefaultGrpcLogsIsInsecure() { + std::string endpoint = GetOtlpDefaultGrpcLogsEndpoint(); + + /* The logs endpoint, when providing a scheme, takes precedence. */ + + if (endpoint.substr(0, 6) == "https:") + { + return false; + } + + if (endpoint.substr(0, 5) == "http:") + { + return true; + } + constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_LOGS_INSECURE"; constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_INSECURE"; diff --git a/exporters/otlp/test/otlp_grpc_exporter_test.cc b/exporters/otlp/test/otlp_grpc_exporter_test.cc index 091cb0d0c4..3298e5390d 100644 --- a/exporters/otlp/test/otlp_grpc_exporter_test.cc +++ b/exporters/otlp/test/otlp_grpc_exporter_test.cc @@ -168,7 +168,7 @@ TEST_F(OtlpGrpcExporterTestPeer, ConfigFromEnv) const std::string cacert_str = "--begin and end fake cert--"; setenv("OTEL_EXPORTER_OTLP_CERTIFICATE_STRING", cacert_str.c_str(), 1); setenv("OTEL_EXPORTER_OTLP_SSL_ENABLE", "True", 1); - const std::string endpoint = "http://localhost:9999"; + const std::string endpoint = "https://localhost:9999"; setenv("OTEL_EXPORTER_OTLP_ENDPOINT", endpoint.c_str(), 1); setenv("OTEL_EXPORTER_OTLP_TIMEOUT", "20050ms", 1); setenv("OTEL_EXPORTER_OTLP_HEADERS", "k1=v1,k2=v2", 1); @@ -211,6 +211,76 @@ TEST_F(OtlpGrpcExporterTestPeer, ConfigFromEnv) } # endif +# ifndef NO_GETENV +// Test exporter configuration options with use_ssl_credentials +TEST_F(OtlpGrpcExporterTestPeer, ConfigHttpsSecureFromEnv) +{ + // https takes precedence over insecure + const std::string endpoint = "https://localhost:9999"; + setenv("OTEL_EXPORTER_OTLP_ENDPOINT", endpoint.c_str(), 1); + setenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE", "true", 1); + + std::unique_ptr exporter(new OtlpGrpcExporter()); + EXPECT_EQ(GetOptions(exporter).use_ssl_credentials, true); + EXPECT_EQ(GetOptions(exporter).endpoint, endpoint); + + unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT"); + unsetenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE"); +} +# endif + +# ifndef NO_GETENV +// Test exporter configuration options with use_ssl_credentials +TEST_F(OtlpGrpcExporterTestPeer, ConfigHttpInsecureFromEnv) +{ + // http takes precedence over secure + const std::string endpoint = "http://localhost:9999"; + setenv("OTEL_EXPORTER_OTLP_ENDPOINT", endpoint.c_str(), 1); + setenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE", "false", 1); + + std::unique_ptr exporter(new OtlpGrpcExporter()); + EXPECT_EQ(GetOptions(exporter).use_ssl_credentials, false); + EXPECT_EQ(GetOptions(exporter).endpoint, endpoint); + + unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT"); + unsetenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE"); +} +# endif + +# ifndef NO_GETENV +// Test exporter configuration options with use_ssl_credentials +TEST_F(OtlpGrpcExporterTestPeer, ConfigUnknownSecureFromEnv) +{ + const std::string endpoint = "localhost:9999"; + setenv("OTEL_EXPORTER_OTLP_ENDPOINT", endpoint.c_str(), 1); + setenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE", "false", 1); + + std::unique_ptr exporter(new OtlpGrpcExporter()); + EXPECT_EQ(GetOptions(exporter).use_ssl_credentials, true); + EXPECT_EQ(GetOptions(exporter).endpoint, endpoint); + + unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT"); + unsetenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE"); +} +# endif + +# ifndef NO_GETENV +// Test exporter configuration options with use_ssl_credentials +TEST_F(OtlpGrpcExporterTestPeer, ConfigUnknownInsecureFromEnv) +{ + const std::string endpoint = "localhost:9999"; + setenv("OTEL_EXPORTER_OTLP_ENDPOINT", endpoint.c_str(), 1); + setenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE", "true", 1); + + std::unique_ptr exporter(new OtlpGrpcExporter()); + EXPECT_EQ(GetOptions(exporter).use_ssl_credentials, false); + EXPECT_EQ(GetOptions(exporter).endpoint, endpoint); + + unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT"); + unsetenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE"); +} +# endif + } // namespace otlp } // namespace exporter OPENTELEMETRY_END_NAMESPACE