Revert "Change OTLP HTTP content_type default to binary (open-telemetry#2558)" (open-telemetry#2562)

lalitb · web-flow · commit 719c60fd8ed5 · 2024-02-27T11:46:40.000-08:00
This reverts commit a883dc7.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -15,9 +15,6 @@ Increment the:
 
 ## [Unreleased]
 
-* [SDK] Change OTLP HTTP content_type default to binary
-  [#2558](https://github.com/open-telemetry/opentelemetry-cpp/pull/2558)
-
 ## [1.14.1] 2024-02-23
 
 * [SDK] Restore Recordable API compatibility with versions < 1.14.0
diff --git a/exporters/otlp/BUILD b/exporters/otlp/BUILD
@@ -104,7 +104,6 @@ cc_library(
 cc_library(
     name = "otlp_http_client",
     srcs = [
-        "src/otlp_http.cc",
         "src/otlp_http_client.cc",
     ],
     hdrs = [
diff --git a/exporters/otlp/CMakeLists.txt b/exporters/otlp/CMakeLists.txt
@@ -108,8 +108,7 @@ if(WITH_OTLP_GRPC)
 endif()
 
 if(WITH_OTLP_HTTP)
-  add_library(opentelemetry_exporter_otlp_http_client src/otlp_http.cc
-                                                      src/otlp_http_client.cc)
+  add_library(opentelemetry_exporter_otlp_http_client src/otlp_http_client.cc)
   set_target_properties(opentelemetry_exporter_otlp_http_client
                         PROPERTIES EXPORT_NAME otlp_http_client)
   set_target_version(opentelemetry_exporter_otlp_http_client)
diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h
@@ -41,10 +41,6 @@ std::string GetOtlpDefaultHttpTracesEndpoint();
 std::string GetOtlpDefaultHttpMetricsEndpoint();
 std::string GetOtlpDefaultHttpLogsEndpoint();
 
-std::string GetOtlpDefaultHttpTracesProtocol();
-std::string GetOtlpDefaultHttpMetricsProtocol();
-std::string GetOtlpDefaultHttpLogsProtocol();
-
 // Compatibility with OTELCPP 1.8.2
 inline std::string GetOtlpDefaultHttpEndpoint()
 {
diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http.h
@@ -3,8 +3,6 @@
 
 #pragma once
 
-#include "opentelemetry/common/macros.h"
-#include "opentelemetry/nostd/string_view.h"
 #include "opentelemetry/sdk/version/version.h"
 
 OPENTELEMETRY_BEGIN_NAMESPACE
@@ -26,9 +24,6 @@ enum class HttpRequestContentType
   kBinary,
 };
 
-OPENTELEMETRY_EXPORT HttpRequestContentType
-GetOtlpHttpProtocolFromString(nostd::string_view name) noexcept;
-
 }  // namespace otlp
 }  // namespace exporter
 OPENTELEMETRY_END_NAMESPACE
diff --git a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http_client.h b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http_client.h
@@ -52,8 +52,8 @@ struct OtlpHttpClientOptions
   /** SSL options. */
   ext::http::client::HttpSslOptions ssl_options;
 
-  // By default, post binary data
-  HttpRequestContentType content_type = HttpRequestContentType::kBinary;
+  // By default, post json data
+  HttpRequestContentType content_type = HttpRequestContentType::kJson;
 
   // If convert bytes into hex. By default, we will convert all bytes but id into base64
   // This option is ignored if content_type is not kJson
diff --git a/exporters/otlp/src/otlp_environment.cc b/exporters/otlp/src/otlp_environment.cc
@@ -207,78 +207,6 @@ std::string GetOtlpDefaultHttpLogsEndpoint()
   return kDefault;
 }
 
-std::string GetOtlpDefaultHttpTracesProtocol()
-{
-  constexpr char kSignalEnv[]  = "OTEL_EXPORTER_OTLP_TRACES_PROTOCOL";
-  constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_PROTOCOL";
-  constexpr char kDefault[]    = "http/protobuf";
-
-  std::string value;
-  bool exists;
-
-  exists = sdk_common::GetStringEnvironmentVariable(kSignalEnv, value);
-  if (exists)
-  {
-    return value;
-  }
-
-  exists = sdk_common::GetStringEnvironmentVariable(kGenericEnv, value);
-  if (exists)
-  {
-    return value;
-  }
-
-  return kDefault;
-}
-
-std::string GetOtlpDefaultHttpMetricsProtocol()
-{
-  constexpr char kSignalEnv[]  = "OTEL_EXPORTER_OTLP_METRICS_PROTOCOL";
-  constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_PROTOCOL";
-  constexpr char kDefault[]    = "http/protobuf";
-
-  std::string value;
-  bool exists;
-
-  exists = sdk_common::GetStringEnvironmentVariable(kSignalEnv, value);
-  if (exists)
-  {
-    return value;
-  }
-
-  exists = sdk_common::GetStringEnvironmentVariable(kGenericEnv, value);
-  if (exists)
-  {
-    return value;
-  }
-
-  return kDefault;
-}
-
-std::string GetOtlpDefaultHttpLogsProtocol()
-{
-  constexpr char kSignalEnv[]  = "OTEL_EXPORTER_OTLP_LOGS_PROTOCOL";
-  constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_PROTOCOL";
-  constexpr char kDefault[]    = "http/protobuf";
-
-  std::string value;
-  bool exists;
-
-  exists = sdk_common::GetStringEnvironmentVariable(kSignalEnv, value);
-  if (exists)
-  {
-    return value;
-  }
-
-  exists = sdk_common::GetStringEnvironmentVariable(kGenericEnv, value);
-  if (exists)
-  {
-    return value;
-  }
-
-  return kDefault;
-}
-
 bool GetOtlpDefaultGrpcTracesIsInsecure()
 {
   std::string endpoint = GetOtlpDefaultGrpcTracesEndpoint();
diff --git a/exporters/otlp/src/otlp_http.cc b/exporters/otlp/src/otlp_http.cc
diff --git a/exporters/otlp/src/otlp_http_exporter_options.cc b/exporters/otlp/src/otlp_http_exporter_options.cc
@@ -17,7 +17,7 @@ namespace otlp
 OtlpHttpExporterOptions::OtlpHttpExporterOptions()
 {
   url                = GetOtlpDefaultHttpTracesEndpoint();
-  content_type       = GetOtlpHttpProtocolFromString(GetOtlpDefaultHttpTracesProtocol());
+  content_type       = HttpRequestContentType::kJson;
   json_bytes_mapping = JsonBytesMappingKind::kHexId;
   use_json_name      = false;
   console_debug      = false;
diff --git a/exporters/otlp/src/otlp_http_log_record_exporter_options.cc b/exporters/otlp/src/otlp_http_log_record_exporter_options.cc
@@ -17,7 +17,7 @@ namespace otlp
 OtlpHttpLogRecordExporterOptions::OtlpHttpLogRecordExporterOptions()
 {
   url                = GetOtlpDefaultHttpLogsEndpoint();
-  content_type       = GetOtlpHttpProtocolFromString(GetOtlpDefaultHttpLogsProtocol());
+  content_type       = HttpRequestContentType::kJson;
   json_bytes_mapping = JsonBytesMappingKind::kHexId;
   use_json_name      = false;
   console_debug      = false;
diff --git a/exporters/otlp/src/otlp_http_metric_exporter_options.cc b/exporters/otlp/src/otlp_http_metric_exporter_options.cc
@@ -17,7 +17,7 @@ namespace otlp
 OtlpHttpMetricExporterOptions::OtlpHttpMetricExporterOptions()
 {
   url                     = GetOtlpDefaultMetricsEndpoint();
-  content_type            = GetOtlpHttpProtocolFromString(GetOtlpDefaultHttpMetricsProtocol());
+  content_type            = HttpRequestContentType::kJson;
   json_bytes_mapping      = JsonBytesMappingKind::kHexId;
   use_json_name           = false;
   console_debug           = false;
diff --git a/exporters/otlp/test/otlp_http_exporter_test.cc b/exporters/otlp/test/otlp_http_exporter_test.cc
@@ -522,12 +522,6 @@ TEST_F(OtlpHttpExporterTestPeer, ConfigJsonBytesMappingTest)
   EXPECT_EQ(GetOptions(exporter).json_bytes_mapping, JsonBytesMappingKind::kHex);
 }
 
-TEST(OtlpHttpExporterTest, ConfigDefaultProtocolTest)
-{
-  OtlpHttpExporterOptions opts;
-  EXPECT_EQ(opts.content_type, HttpRequestContentType::kBinary);
-}
-
 #  ifndef NO_GETENV
 // Test exporter configuration options with use_ssl_credentials
 TEST_F(OtlpHttpExporterTestPeer, ConfigFromEnv)
@@ -537,7 +531,6 @@ TEST_F(OtlpHttpExporterTestPeer, ConfigFromEnv)
   setenv("OTEL_EXPORTER_OTLP_TIMEOUT", "20s", 1);
   setenv("OTEL_EXPORTER_OTLP_HEADERS", "k1=v1,k2=v2", 1);
   setenv("OTEL_EXPORTER_OTLP_TRACES_HEADERS", "k1=v3,k1=v4", 1);
-  setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "http/json", 1);
 
   std::unique_ptr<OtlpHttpExporter> exporter(new OtlpHttpExporter());
   EXPECT_EQ(GetOptions(exporter).url, url);
@@ -564,13 +557,11 @@ TEST_F(OtlpHttpExporterTestPeer, ConfigFromEnv)
     ++range.first;
     EXPECT_TRUE(range.first == range.second);
   }
-  EXPECT_EQ(GetOptions(exporter).content_type, HttpRequestContentType::kJson);
 
   unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT");
   unsetenv("OTEL_EXPORTER_OTLP_TIMEOUT");
   unsetenv("OTEL_EXPORTER_OTLP_HEADERS");
   unsetenv("OTEL_EXPORTER_OTLP_TRACES_HEADERS");
-  unsetenv("OTEL_EXPORTER_OTLP_PROTOCOL");
 }
 
 TEST_F(OtlpHttpExporterTestPeer, ConfigFromTracesEnv)
@@ -580,7 +571,6 @@ TEST_F(OtlpHttpExporterTestPeer, ConfigFromTracesEnv)
   setenv("OTEL_EXPORTER_OTLP_TRACES_TIMEOUT", "1eternity", 1);
   setenv("OTEL_EXPORTER_OTLP_HEADERS", "k1=v1,k2=v2", 1);
   setenv("OTEL_EXPORTER_OTLP_TRACES_HEADERS", "k1=v3,k1=v4", 1);
-  setenv("OTEL_EXPORTER_OTLP_TRACES_PROTOCOL", "http/json", 1);
 
   std::unique_ptr<OtlpHttpExporter> exporter(new OtlpHttpExporter());
   EXPECT_EQ(GetOptions(exporter).url, url);
@@ -607,13 +597,11 @@ TEST_F(OtlpHttpExporterTestPeer, ConfigFromTracesEnv)
     ++range.first;
     EXPECT_TRUE(range.first == range.second);
   }
-  EXPECT_EQ(GetOptions(exporter).content_type, HttpRequestContentType::kJson);
 
   unsetenv("OTEL_EXPORTER_OTLP_TRACES_ENDPOINT");
   unsetenv("OTEL_EXPORTER_OTLP_TRACES_TIMEOUT");
   unsetenv("OTEL_EXPORTER_OTLP_HEADERS");
   unsetenv("OTEL_EXPORTER_OTLP_TRACES_HEADERS");
-  unsetenv("OTEL_EXPORTER_OTLP_TRACES_PROTOCOL");
 }
 #  endif
 
diff --git a/exporters/otlp/test/otlp_http_log_record_exporter_test.cc b/exporters/otlp/test/otlp_http_log_record_exporter_test.cc
@@ -642,12 +642,6 @@ TEST_F(OtlpHttpLogRecordExporterTestPeer, ConfigJsonBytesMappingTest)
   EXPECT_EQ(GetOptions(exporter).json_bytes_mapping, JsonBytesMappingKind::kHex);
 }
 
-TEST(OtlpHttpLogRecordExporterTest, ConfigDefaultProtocolTest)
-{
-  OtlpHttpLogRecordExporterOptions opts;
-  EXPECT_EQ(opts.content_type, HttpRequestContentType::kBinary);
-}
-
 #  ifndef NO_GETENV
 // Test exporter configuration options with use_ssl_credentials
 TEST_F(OtlpHttpLogRecordExporterTestPeer, ConfigFromEnv)
@@ -657,7 +651,6 @@ TEST_F(OtlpHttpLogRecordExporterTestPeer, ConfigFromEnv)
   setenv("OTEL_EXPORTER_OTLP_TIMEOUT", "20s", 1);
   setenv("OTEL_EXPORTER_OTLP_HEADERS", "k1=v1,k2=v2", 1);
   setenv("OTEL_EXPORTER_OTLP_LOGS_HEADERS", "k1=v3,k1=v4", 1);
-  setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "http/json", 1);
 
   std::unique_ptr<OtlpHttpLogRecordExporter> exporter(new OtlpHttpLogRecordExporter());
   EXPECT_EQ(GetOptions(exporter).url, url);
@@ -684,13 +677,11 @@ TEST_F(OtlpHttpLogRecordExporterTestPeer, ConfigFromEnv)
     ++range.first;
     EXPECT_TRUE(range.first == range.second);
   }
-  EXPECT_EQ(GetOptions(exporter).content_type, HttpRequestContentType::kJson);
 
   unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT");
   unsetenv("OTEL_EXPORTER_OTLP_TIMEOUT");
   unsetenv("OTEL_EXPORTER_OTLP_HEADERS");
   unsetenv("OTEL_EXPORTER_OTLP_LOGS_HEADERS");
-  unsetenv("OTEL_EXPORTER_OTLP_PROTOCOL");
 }
 
 TEST_F(OtlpHttpLogRecordExporterTestPeer, ConfigFromLogsEnv)
@@ -700,7 +691,6 @@ TEST_F(OtlpHttpLogRecordExporterTestPeer, ConfigFromLogsEnv)
   setenv("OTEL_EXPORTER_OTLP_LOGS_TIMEOUT", "20s", 1);
   setenv("OTEL_EXPORTER_OTLP_HEADERS", "k1=v1,k2=v2", 1);
   setenv("OTEL_EXPORTER_OTLP_LOGS_HEADERS", "k1=v3,k1=v4", 1);
-  setenv("OTEL_EXPORTER_OTLP_LOGS_PROTOCOL", "http/json", 1);
 
   std::unique_ptr<OtlpHttpLogRecordExporter> exporter(new OtlpHttpLogRecordExporter());
   EXPECT_EQ(GetOptions(exporter).url, url);
@@ -727,13 +717,11 @@ TEST_F(OtlpHttpLogRecordExporterTestPeer, ConfigFromLogsEnv)
     ++range.first;
     EXPECT_TRUE(range.first == range.second);
   }
-  EXPECT_EQ(GetOptions(exporter).content_type, HttpRequestContentType::kJson);
 
   unsetenv("OTEL_EXPORTER_OTLP_LOGS_ENDPOINT");
   unsetenv("OTEL_EXPORTER_OTLP_LOGS_TIMEOUT");
   unsetenv("OTEL_EXPORTER_OTLP_HEADERS");
   unsetenv("OTEL_EXPORTER_OTLP_LOGS_HEADERS");
-  unsetenv("OTEL_EXPORTER_OTLP_LOGS_PROTOCOL");
 }
 
 TEST_F(OtlpHttpLogRecordExporterTestPeer, DefaultEndpoint)
diff --git a/exporters/otlp/test/otlp_http_metric_exporter_test.cc b/exporters/otlp/test/otlp_http_metric_exporter_test.cc
@@ -862,12 +862,6 @@ TEST_F(OtlpHttpMetricExporterTestPeer, ConfigJsonBytesMappingTest)
   google::protobuf::ShutdownProtobufLibrary();
 }
 
-TEST(OtlpHttpMetricExporterTest, ConfigDefaultProtocolTest)
-{
-  OtlpHttpMetricExporterOptions opts;
-  EXPECT_EQ(opts.content_type, HttpRequestContentType::kBinary);
-}
-
 #ifndef NO_GETENV
 // Test exporter configuration options with use_ssl_credentials
 TEST_F(OtlpHttpMetricExporterTestPeer, ConfigFromEnv)
@@ -877,7 +871,6 @@ TEST_F(OtlpHttpMetricExporterTestPeer, ConfigFromEnv)
   setenv("OTEL_EXPORTER_OTLP_TIMEOUT", "20s", 1);
   setenv("OTEL_EXPORTER_OTLP_HEADERS", "k1=v1,k2=v2", 1);
   setenv("OTEL_EXPORTER_OTLP_METRICS_HEADERS", "k1=v3,k1=v4", 1);
-  setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "http/json", 1);
 
   std::unique_ptr<OtlpHttpMetricExporter> exporter(new OtlpHttpMetricExporter());
   EXPECT_EQ(GetOptions(exporter).url, url);
@@ -904,13 +897,11 @@ TEST_F(OtlpHttpMetricExporterTestPeer, ConfigFromEnv)
     ++range.first;
     EXPECT_TRUE(range.first == range.second);
   }
-  EXPECT_EQ(GetOptions(exporter).content_type, HttpRequestContentType::kJson);
 
   unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT");
   unsetenv("OTEL_EXPORTER_OTLP_TIMEOUT");
   unsetenv("OTEL_EXPORTER_OTLP_HEADERS");
   unsetenv("OTEL_EXPORTER_OTLP_METRICS_HEADERS");
-  unsetenv("OTEL_EXPORTER_OTLP_PROTOCOL");
 }
 
 TEST_F(OtlpHttpMetricExporterTestPeer, ConfigFromMetricsEnv)
@@ -920,7 +911,6 @@ TEST_F(OtlpHttpMetricExporterTestPeer, ConfigFromMetricsEnv)
   setenv("OTEL_EXPORTER_OTLP_METRICS_TIMEOUT", "20s", 1);
   setenv("OTEL_EXPORTER_OTLP_HEADERS", "k1=v1,k2=v2", 1);
   setenv("OTEL_EXPORTER_OTLP_METRICS_HEADERS", "k1=v3,k1=v4", 1);
-  setenv("OTEL_EXPORTER_OTLP_METRICS_PROTOCOL", "http/json", 1);
 
   std::unique_ptr<OtlpHttpMetricExporter> exporter(new OtlpHttpMetricExporter());
   EXPECT_EQ(GetOptions(exporter).url, url);
@@ -947,13 +937,11 @@ TEST_F(OtlpHttpMetricExporterTestPeer, ConfigFromMetricsEnv)
     ++range.first;
     EXPECT_TRUE(range.first == range.second);
   }
-  EXPECT_EQ(GetOptions(exporter).content_type, HttpRequestContentType::kJson);
 
   unsetenv("OTEL_EXPORTER_OTLP_METRICS_ENDPOINT");
   unsetenv("OTEL_EXPORTER_OTLP_METRICS_TIMEOUT");
   unsetenv("OTEL_EXPORTER_OTLP_HEADERS");
   unsetenv("OTEL_EXPORTER_OTLP_METRICS_HEADERS");
-  unsetenv("OTEL_EXPORTER_OTLP_METRICS_PROTOCOL");
 }
 
 TEST_F(OtlpHttpMetricExporterTestPeer, DefaultEndpoint)

Original file line number	Diff line number	Diff line change
`@@ -17,7 +17,7 @@ namespace otlp`
`17`	`17`	`OtlpHttpExporterOptions::OtlpHttpExporterOptions()`
`18`	`18`	`{`
`19`	`19`	`url = GetOtlpDefaultHttpTracesEndpoint();`
`20`		`- content_type = GetOtlpHttpProtocolFromString(GetOtlpDefaultHttpTracesProtocol());`
	`20`	`+ content_type = HttpRequestContentType::kJson;`
`21`	`21`	`json_bytes_mapping = JsonBytesMappingKind::kHexId;`
`22`	`22`	`use_json_name = false;`
`23`	`23`	`console_debug = false;`