From d036d8227f6f1f988c611aa6a0312f3ccd346d63 Mon Sep 17 00:00:00 2001 From: Tom Tan Date: Tue, 27 Feb 2024 14:16:03 -0800 Subject: [PATCH] Reapply "Change OTLP HTTP content_type default to binary (#2558)" (#2564) --- 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, 154 insertions(+), 6 deletions(-) create mode 100644 exporters/otlp/src/otlp_http.cc diff --git a/CHANGELOG.md b/CHANGELOG.md index dbae8e33e80..d9e079783ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -15,6 +15,9 @@ Increment the: ## [Unreleased] +* [SDK] Change OTLP HTTP content_type default to binary + [#2558](https://github.com/open-telemetry/opentelemetry-cpp/pull/2558) + ## [1.14.2] 2024-02-27 * [SDK] Fix observable attributes drop diff --git a/exporters/otlp/BUILD b/exporters/otlp/BUILD index a70153edd8f..a9f1c887e33 100644 --- a/exporters/otlp/BUILD +++ b/exporters/otlp/BUILD @@ -104,6 +104,7 @@ 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 ec55211a6d5..243a7a2bd34 100644 --- a/exporters/otlp/CMakeLists.txt +++ b/exporters/otlp/CMakeLists.txt @@ -108,7 +108,8 @@ if(WITH_OTLP_GRPC) endif() if(WITH_OTLP_HTTP) - add_library(opentelemetry_exporter_otlp_http_client src/otlp_http_client.cc) + add_library(opentelemetry_exporter_otlp_http_client src/otlp_http.cc + 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 8234dc913e9..306dff8f09c 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h @@ -41,6 +41,10 @@ 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 e5bbab94350..7d1e7bac55b 100644 --- a/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http.h +++ b/exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http.h @@ -3,6 +3,8 @@ #pragma once +#include "opentelemetry/common/macros.h" +#include "opentelemetry/nostd/string_view.h" #include "opentelemetry/sdk/version/version.h" OPENTELEMETRY_BEGIN_NAMESPACE @@ -24,6 +26,9 @@ 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 6af73f7e6b5..163cf7b57fe 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 json data - HttpRequestContentType content_type = HttpRequestContentType::kJson; + // By default, post binary data + HttpRequestContentType content_type = HttpRequestContentType::kBinary; // 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 a355b13162c..0cb39cb68e2 100644 --- a/exporters/otlp/src/otlp_environment.cc +++ b/exporters/otlp/src/otlp_environment.cc @@ -207,6 +207,78 @@ 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 new file mode 100644 index 00000000000..098de41c44e --- /dev/null +++ b/exporters/otlp/src/otlp_http.cc @@ -0,0 +1,26 @@ +// 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 3b8c6ad5b8e..ae768a0dc57 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 = HttpRequestContentType::kJson; + content_type = GetOtlpHttpProtocolFromString(GetOtlpDefaultHttpTracesProtocol()); 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 2f58bce768d..35b80f48ac5 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 = HttpRequestContentType::kJson; + content_type = GetOtlpHttpProtocolFromString(GetOtlpDefaultHttpLogsProtocol()); 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 ffcd502bd4d..ebed1c4c83c 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 = HttpRequestContentType::kJson; + content_type = GetOtlpHttpProtocolFromString(GetOtlpDefaultHttpMetricsProtocol()); 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 63f919c6cc8..26325142beb 100644 --- a/exporters/otlp/test/otlp_http_exporter_test.cc +++ b/exporters/otlp/test/otlp_http_exporter_test.cc @@ -522,6 +522,12 @@ 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) @@ -531,6 +537,7 @@ 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); @@ -557,11 +564,13 @@ 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) @@ -571,6 +580,7 @@ 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); @@ -597,11 +607,13 @@ 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 611bebb541f..3455b7cc364 100644 --- a/exporters/otlp/test/otlp_http_log_record_exporter_test.cc +++ b/exporters/otlp/test/otlp_http_log_record_exporter_test.cc @@ -642,6 +642,12 @@ 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) @@ -651,6 +657,7 @@ 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); @@ -677,11 +684,13 @@ 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) @@ -691,6 +700,7 @@ 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); @@ -717,11 +727,13 @@ 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 def56682d36..5812dddc252 100644 --- a/exporters/otlp/test/otlp_http_metric_exporter_test.cc +++ b/exporters/otlp/test/otlp_http_metric_exporter_test.cc @@ -862,6 +862,12 @@ 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) @@ -871,6 +877,7 @@ 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); @@ -897,11 +904,13 @@ 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) @@ -911,6 +920,7 @@ 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); @@ -937,11 +947,13 @@ 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)