Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Change OTLP HTTP content_type default to binary #2558

Merged
merged 3 commits into from
Feb 27, 2024
Merged

Change OTLP HTTP content_type default to binary #2558

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
a4b3c34
Change OTLP HTTP content_type default to binary
owent Feb 27, 2024
5994f42
Add changelog
owent Feb 27, 2024
69faeb5
Merge branch 'main' into change_otlp_http_content_type_default_to_binary
ThomsonTan Feb 27, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 3 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
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.1] 2024-02-23

* [SDK] Restore Recordable API compatibility with versions < 1.14.0
Expand Down
1 change: 1 addition & 0 deletions exporters/otlp/BUILD
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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;
}
Comment on lines +18 to +21
Copy link
Member

Choose a reason for hiding this comment

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

nit

Suggested change
else
{
return HttpRequestContentType::kBinary;
}
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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