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.

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

[EXPORTER] GRPC endpoint scheme should take precedence over OTEL_EXPORTER_OTLP_TRACES_INSECURE #2060

Merged
merged 7 commits into from
Mar 16, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
25 changes: 25 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,31 @@ Increment the:

* [RESOURCE SDK] Fix schema URL precedence bug in `Resource::Merge`.
[#2036](https://github.com/open-telemetry/opentelemetry-cpp/pull/2036)
* [EXPORTER] GRPC endpoint scheme should take precedence over OTEL_EXPORTER_OTLP_TRACES_INSECURE
[#2060](https://github.com/open-telemetry/opentelemetry-cpp/pull/2060)

Important changes:

* [EXPORTER] GRPC endpoint scheme should take precedence over OTEL_EXPORTER_OTLP_TRACES_INSECURE
[#2060](https://github.com/open-telemetry/opentelemetry-cpp/pull/2060)
* The logic to decide whether or not an OTLP GRPC exporter uses SSL has
changed to comply with the specification:
* Before this change, the following settings were evaluated, in order:
* OTEL_EXPORTER_OTLP_TRACES_INSECURE (starting with 1.8.3)
* OTEL_EXPORTER_OTLP_INSECURE (starting with 1.8.3)
* OTEL_EXPORTER_OTLP_TRACES_SSL_ENABLE
* OTEL_EXPORTER_OTLP_SSL_ENABLE
* With this change, the following settings are evaluated, in order:
* The GRPC endpoint scheme, if provided:
* "https" imply with SSL,
* "http" imply without ssl.
* OTEL_EXPORTER_OTLP_TRACES_INSECURE
* OTEL_EXPORTER_OTLP_INSECURE
* OTEL_EXPORTER_OTLP_TRACES_SSL_ENABLE
* OTEL_EXPORTER_OTLP_SSL_ENABLE
* As a result, a behavior change for GRPC SSL is possible,
because the endpoint scheme now takes precedence.
Please verify configuration settings for the GRPC endpoint.

## [1.8.3] 2023-03-06

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,14 +53,14 @@ inline std::string GetOtlpDefaultMetricsEndpoint()
return GetOtlpDefaultHttpMetricsEndpoint();
}

bool GetOtlpDefaultTracesIsInsecure();
bool GetOtlpDefaultMetricsIsInsecure();
bool GetOtlpDefaultLogsIsInsecure();
bool GetOtlpDefaultGrpcTracesIsInsecure();
bool GetOtlpDefaultGrpcMetricsIsInsecure();
bool GetOtlpDefaultGrpcLogsIsInsecure();

// Compatibility with OTELCPP 1.8.2
inline bool GetOtlpDefaultIsSslEnable()
{
return (!GetOtlpDefaultTracesIsInsecure());
return (!GetOtlpDefaultGrpcTracesIsInsecure());
}

std::string GetOtlpDefaultTracesSslCertificatePath();
Expand Down
48 changes: 45 additions & 3 deletions exporters/otlp/src/otlp_environment.cc
Original file line number Diff line number Diff line change
Expand Up @@ -207,8 +207,22 @@ std::string GetOtlpDefaultHttpLogsEndpoint()
return kDefault;
}

bool GetOtlpDefaultTracesIsInsecure()
bool GetOtlpDefaultGrpcTracesIsInsecure()
{
std::string endpoint = GetOtlpDefaultGrpcTracesEndpoint();

/* The trace endpoint, when providing a scheme, takes precedence. */

if (endpoint.substr(0, 6) == "https:")
{
return false;
}

if (endpoint.substr(0, 5) == "http:")
{
return true;
}

constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_TRACES_INSECURE";
constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_INSECURE";
constexpr char kOldSignalEnv[] = "OTEL_EXPORTER_OTLP_TRACES_SSL_ENABLE";
Expand Down Expand Up @@ -251,8 +265,22 @@ bool GetOtlpDefaultTracesIsInsecure()
return false;
}

bool GetOtlpDefaultMetricsIsInsecure()
bool GetOtlpDefaultGrpcMetricsIsInsecure()
{
std::string endpoint = GetOtlpDefaultGrpcMetricsEndpoint();

/* The metrics endpoint, when providing a scheme, takes precedence. */

if (endpoint.substr(0, 6) == "https:")
{
return false;
}

if (endpoint.substr(0, 5) == "http:")
{
return true;
}

constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_METRICS_INSECURE";
constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_INSECURE";
constexpr char kOldSignalEnv[] = "OTEL_EXPORTER_OTLP_METRICS_SSL_ENABLE";
Expand Down Expand Up @@ -295,8 +323,22 @@ bool GetOtlpDefaultMetricsIsInsecure()
return false;
}

bool GetOtlpDefaultLogsIsInsecure()
bool GetOtlpDefaultGrpcLogsIsInsecure()
{
std::string endpoint = GetOtlpDefaultGrpcLogsEndpoint();

/* The logs endpoint, when providing a scheme, takes precedence. */

if (endpoint.substr(0, 6) == "https:")
{
return false;
}

if (endpoint.substr(0, 5) == "http:")
{
return true;
}

constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_LOGS_INSECURE";
constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_INSECURE";

Expand Down
72 changes: 71 additions & 1 deletion exporters/otlp/test/otlp_grpc_exporter_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -168,7 +168,7 @@ TEST_F(OtlpGrpcExporterTestPeer, ConfigFromEnv)
const std::string cacert_str = "--begin and end fake cert--";
setenv("OTEL_EXPORTER_OTLP_CERTIFICATE_STRING", cacert_str.c_str(), 1);
setenv("OTEL_EXPORTER_OTLP_SSL_ENABLE", "True", 1);
const std::string endpoint = "http://localhost:9999";
const std::string endpoint = "https://localhost:9999";
setenv("OTEL_EXPORTER_OTLP_ENDPOINT", endpoint.c_str(), 1);
setenv("OTEL_EXPORTER_OTLP_TIMEOUT", "20050ms", 1);
setenv("OTEL_EXPORTER_OTLP_HEADERS", "k1=v1,k2=v2", 1);
Expand Down Expand Up @@ -211,6 +211,76 @@ TEST_F(OtlpGrpcExporterTestPeer, ConfigFromEnv)
}
# endif

# ifndef NO_GETENV
// Test exporter configuration options with use_ssl_credentials
TEST_F(OtlpGrpcExporterTestPeer, ConfigHttpsSecureFromEnv)
{
// https takes precedence over insecure
const std::string endpoint = "https://localhost:9999";
setenv("OTEL_EXPORTER_OTLP_ENDPOINT", endpoint.c_str(), 1);
setenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE", "true", 1);

std::unique_ptr<OtlpGrpcExporter> exporter(new OtlpGrpcExporter());
EXPECT_EQ(GetOptions(exporter).use_ssl_credentials, true);
EXPECT_EQ(GetOptions(exporter).endpoint, endpoint);

unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT");
unsetenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE");
}
# endif

# ifndef NO_GETENV
// Test exporter configuration options with use_ssl_credentials
TEST_F(OtlpGrpcExporterTestPeer, ConfigHttpInsecureFromEnv)
{
// http takes precedence over secure
const std::string endpoint = "http://localhost:9999";
setenv("OTEL_EXPORTER_OTLP_ENDPOINT", endpoint.c_str(), 1);
setenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE", "false", 1);

std::unique_ptr<OtlpGrpcExporter> exporter(new OtlpGrpcExporter());
EXPECT_EQ(GetOptions(exporter).use_ssl_credentials, false);
EXPECT_EQ(GetOptions(exporter).endpoint, endpoint);

unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT");
unsetenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE");
}
# endif

# ifndef NO_GETENV
// Test exporter configuration options with use_ssl_credentials
TEST_F(OtlpGrpcExporterTestPeer, ConfigUnknownSecureFromEnv)
{
const std::string endpoint = "localhost:9999";
setenv("OTEL_EXPORTER_OTLP_ENDPOINT", endpoint.c_str(), 1);
setenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE", "false", 1);

std::unique_ptr<OtlpGrpcExporter> exporter(new OtlpGrpcExporter());
EXPECT_EQ(GetOptions(exporter).use_ssl_credentials, true);
EXPECT_EQ(GetOptions(exporter).endpoint, endpoint);

unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT");
unsetenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE");
}
# endif

# ifndef NO_GETENV
// Test exporter configuration options with use_ssl_credentials
TEST_F(OtlpGrpcExporterTestPeer, ConfigUnknownInsecureFromEnv)
{
const std::string endpoint = "localhost:9999";
setenv("OTEL_EXPORTER_OTLP_ENDPOINT", endpoint.c_str(), 1);
setenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE", "true", 1);

std::unique_ptr<OtlpGrpcExporter> exporter(new OtlpGrpcExporter());
EXPECT_EQ(GetOptions(exporter).use_ssl_credentials, false);
EXPECT_EQ(GetOptions(exporter).endpoint, endpoint);

unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT");
unsetenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE");
}
# endif

} // namespace otlp
} // namespace exporter
OPENTELEMETRY_END_NAMESPACE
Expand Down