From 719c60fd8ed5318baf93af4cd4b56e05c82b3488 Mon Sep 17 00:00:00 2001 From: Lalit Kumar Bhasin Date: Tue, 27 Feb 2024 11:46:40 -0800 Subject: [PATCH] Revert "Change OTLP HTTP content_type default to binary (#2558)" (#2562) This reverts commit a883dc72559aef081925f9dc16f200773fc613ef. --- CHANGELOG.md | 3 - exporters/otlp/BUILD | 1 - exporters/otlp/CMakeLists.txt | 3 +- .../exporters/otlp/otlp_environment.h | 4 -- .../opentelemetry/exporters/otlp/otlp_http.h | 5 -- .../exporters/otlp/otlp_http_client.h | 4 +- exporters/otlp/src/otlp_environment.cc | 72 ------------------- exporters/otlp/src/otlp_http.cc | 26 ------- .../otlp/src/otlp_http_exporter_options.cc | 2 +- .../otlp_http_log_record_exporter_options.cc | 2 +- .../src/otlp_http_metric_exporter_options.cc | 2 +- .../otlp/test/otlp_http_exporter_test.cc | 12 ---- .../otlp_http_log_record_exporter_test.cc | 12 ---- .../test/otlp_http_metric_exporter_test.cc | 12 ---- 14 files changed, 6 insertions(+), 154 deletions(-) delete mode 100644 exporters/otlp/src/otlp_http.cc diff --git a/CHANGELOG.md b/CHANGELOG.md index 952c6c6958..c0c3bb411b 100644 --- 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 index a9f1c887e3..a70153edd8 100644 --- 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 index 243a7a2bd3..ec55211a6d 100644 --- 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 index 306dff8f09..8234dc913e 100644 --- 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 index 7d1e7bac55..e5bbab9435 100644 --- 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 index 163cf7b57f..6af73f7e6b 100644 --- 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 index 0cb39cb68e..a355b13162 100644 --- 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 deleted file mode 100644 index 098de41c44..0000000000 --- a/exporters/otlp/src/otlp_http.cc +++ /dev/null @@ -1,26 +0,0 @@ -// Copyright The OpenTelemetry Authors -// SPDX-License-Identifier: Apache-2.0 - -#include "opentelemetry/exporters/otlp/otlp_http.h" - -OPENTELEMETRY_BEGIN_NAMESPACE -namespace exporter -{ -namespace otlp -{ - -HttpRequestContentType GetOtlpHttpProtocolFromString(nostd::string_view name) noexcept -{ - if (name == "http/json") - { - return HttpRequestContentType::kJson; - } - else - { - return HttpRequestContentType::kBinary; - } -} - -} // namespace otlp -} // namespace exporter -OPENTELEMETRY_END_NAMESPACE diff --git a/exporters/otlp/src/otlp_http_exporter_options.cc b/exporters/otlp/src/otlp_http_exporter_options.cc index ae768a0dc5..3b8c6ad5b8 100644 --- 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 index 35b80f48ac..2f58bce768 100644 --- 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 index ebed1c4c83..ffcd502bd4 100644 --- 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 index 26325142be..63f919c6cc 100644 --- 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 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 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 index 3455b7cc36..611bebb541 100644 --- 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 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 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 index 5812dddc25..def56682d3 100644 --- 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 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 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)