[EXPORTER] GRPC endpoint scheme should take precedence over OTEL_EXPO…

…RTER_OTLP_TRACES_INSECURE (open-telemetry#2060)

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
 

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();

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";
 

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<OtlpGrpcExporter> 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<OtlpGrpcExporter> 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<OtlpGrpcExporter> 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<OtlpGrpcExporter> 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