Skip to content

Commit

Permalink
Reapply "Change OTLP HTTP content_type default to binary (#2558)" (#2564
Browse files Browse the repository at this point in the history 
)
  • Loading branch information
@ThomsonTan
ThomsonTan committed Feb 27, 2024
1 parent a799f4a commit d036d82
Show file tree
Hide file tree
Showing 14 changed files with 154 additions and 6 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 1 addition & 0 deletions exporters/otlp/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ cc_library(
cc_library(
name = "otlp_http_client",
srcs = [
"src/otlp_http.cc",
"src/otlp_http_client.cc",
],
hdrs = [
Expand Down
3 changes: 2 additions & 1 deletion exporters/otlp/CMakeLists.txt
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 4 additions & 0 deletions exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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()
{
Expand Down
5 changes: 5 additions & 0 deletions exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand All @@ -24,6 +26,9 @@ enum class HttpRequestContentType
kBinary,
};

OPENTELEMETRY_EXPORT HttpRequestContentType
GetOtlpHttpProtocolFromString(nostd::string_view name) noexcept;

} // namespace otlp
} // namespace exporter
OPENTELEMETRY_END_NAMESPACE
4 changes: 2 additions & 2 deletions exporters/otlp/include/opentelemetry/exporters/otlp/otlp_http_client.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
72 changes: 72 additions & 0 deletions exporters/otlp/src/otlp_environment.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
26 changes: 26 additions & 0 deletions exporters/otlp/src/otlp_http.cc
View file
Original file line number Diff line number Diff line change
@@ -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
2 changes: 1 addition & 1 deletion exporters/otlp/src/otlp_http_exporter_options.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion exporters/otlp/src/otlp_http_log_record_exporter_options.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
2 changes: 1 addition & 1 deletion exporters/otlp/src/otlp_http_metric_exporter_options.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
12 changes: 12 additions & 0 deletions exporters/otlp/test/otlp_http_exporter_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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<OtlpHttpExporter> exporter(new OtlpHttpExporter());
EXPECT_EQ(GetOptions(exporter).url, url);
Expand All @@ -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)
Expand All @@ -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<OtlpHttpExporter> exporter(new OtlpHttpExporter());
EXPECT_EQ(GetOptions(exporter).url, url);
Expand All @@ -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

Expand Down
12 changes: 12 additions & 0 deletions exporters/otlp/test/otlp_http_log_record_exporter_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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<OtlpHttpLogRecordExporter> exporter(new OtlpHttpLogRecordExporter());
EXPECT_EQ(GetOptions(exporter).url, url);
Expand All @@ -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)
Expand All @@ -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<OtlpHttpLogRecordExporter> exporter(new OtlpHttpLogRecordExporter());
EXPECT_EQ(GetOptions(exporter).url, url);
Expand All @@ -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)
Expand Down
12 changes: 12 additions & 0 deletions exporters/otlp/test/otlp_http_metric_exporter_test.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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<OtlpHttpMetricExporter> exporter(new OtlpHttpMetricExporter());
EXPECT_EQ(GetOptions(exporter).url, url);
Expand All @@ -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)
Expand All @@ -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<OtlpHttpMetricExporter> exporter(new OtlpHttpMetricExporter());
EXPECT_EQ(GetOptions(exporter).url, url);
Expand All @@ -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)
Expand Down

2 comments on commit d036d82

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'OpenTelemetry-cpp api Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: d036d82 Previous: a799f4a Ratio
BM_ProcYieldSpinLockThrashing/2/process_time/real_time 2.3287200927734375 ms/iter 0.2392931814301961 ms/iter 9.73

This comment was automatically generated by workflow using github-action-benchmark.

@github-actions
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Performance Alert ⚠️

Possible performance regression was detected for benchmark 'OpenTelemetry-cpp sdk Benchmark'.
Benchmark result of this commit is worse than the previous benchmark result exceeding threshold 2.

Benchmark suite Current: d036d82 Previous: a799f4a Ratio
BM_NoopSpanCreation 257.80486153446697 ns/iter 125.23904724441108 ns/iter 2.06
BM_BaselineBuffer/1 6093165.874481201 ns/iter 546411.0374450684 ns/iter 11.15
BM_LockFreeBuffer/1 2128296.6136932373 ns/iter 685625.5531311035 ns/iter 3.10

This comment was automatically generated by workflow using github-action-benchmark.

Please sign in to comment.