Skip to content

Commit 211d9c9

Browse files
marcalffmarcalff
authored
[EXPORTER] GRPC endpoint scheme should take precedence over OTEL_EXPORTER_OTLP_TRACES_INSECURE (#2060)
1 parent e0a85f2 commit 211d9c9

File tree

4 files changed

+145
-8
lines changed

4 files changed

+145
-8
lines changed

‎CHANGELOG.md‎

Lines changed: 25 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,31 @@ Increment the:
1717

1818
* [RESOURCE SDK] Fix schema URL precedence bug in `Resource::Merge`.
1919
[#2036](https://github.com/open-telemetry/opentelemetry-cpp/pull/2036)
20+
* [EXPORTER] GRPC endpoint scheme should take precedence over OTEL_EXPORTER_OTLP_TRACES_INSECURE
21+
[#2060](https://github.com/open-telemetry/opentelemetry-cpp/pull/2060)
22+
23+
Important changes:
24+
25+
* [EXPORTER] GRPC endpoint scheme should take precedence over OTEL_EXPORTER_OTLP_TRACES_INSECURE
26+
[#2060](https://github.com/open-telemetry/opentelemetry-cpp/pull/2060)
27+
* The logic to decide whether or not an OTLP GRPC exporter uses SSL has
28+
changed to comply with the specification:
29+
* Before this change, the following settings were evaluated, in order:
30+
* OTEL_EXPORTER_OTLP_TRACES_INSECURE (starting with 1.8.3)
31+
* OTEL_EXPORTER_OTLP_INSECURE (starting with 1.8.3)
32+
* OTEL_EXPORTER_OTLP_TRACES_SSL_ENABLE
33+
* OTEL_EXPORTER_OTLP_SSL_ENABLE
34+
* With this change, the following settings are evaluated, in order:
35+
* The GRPC endpoint scheme, if provided:
36+
* "https" imply with SSL,
37+
* "http" imply without ssl.
38+
* OTEL_EXPORTER_OTLP_TRACES_INSECURE
39+
* OTEL_EXPORTER_OTLP_INSECURE
40+
* OTEL_EXPORTER_OTLP_TRACES_SSL_ENABLE
41+
* OTEL_EXPORTER_OTLP_SSL_ENABLE
42+
* As a result, a behavior change for GRPC SSL is possible,
43+
because the endpoint scheme now takes precedence.
44+
Please verify configuration settings for the GRPC endpoint.
2045

2146
## [1.8.3] 2023-03-06
2247

‎exporters/otlp/include/opentelemetry/exporters/otlp/otlp_environment.h‎

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,14 +53,14 @@ inline std::string GetOtlpDefaultMetricsEndpoint()
5353
return GetOtlpDefaultHttpMetricsEndpoint();
5454
}
5555

56-
bool GetOtlpDefaultTracesIsInsecure();
57-
bool GetOtlpDefaultMetricsIsInsecure();
58-
bool GetOtlpDefaultLogsIsInsecure();
56+
bool GetOtlpDefaultGrpcTracesIsInsecure();
57+
bool GetOtlpDefaultGrpcMetricsIsInsecure();
58+
bool GetOtlpDefaultGrpcLogsIsInsecure();
5959

6060
// Compatibility with OTELCPP 1.8.2
6161
inline bool GetOtlpDefaultIsSslEnable()
6262
{
63-
return (!GetOtlpDefaultTracesIsInsecure());
63+
return (!GetOtlpDefaultGrpcTracesIsInsecure());
6464
}
6565

6666
std::string GetOtlpDefaultTracesSslCertificatePath();

‎exporters/otlp/src/otlp_environment.cc‎

Lines changed: 45 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -207,8 +207,22 @@ std::string GetOtlpDefaultHttpLogsEndpoint()
207207
return kDefault;
208208
}
209209

210-
bool GetOtlpDefaultTracesIsInsecure()
210+
bool GetOtlpDefaultGrpcTracesIsInsecure()
211211
{
212+
std::string endpoint = GetOtlpDefaultGrpcTracesEndpoint();
213+
214+
/* The trace endpoint, when providing a scheme, takes precedence. */
215+
216+
if (endpoint.substr(0, 6) == "https:")
217+
{
218+
return false;
219+
}
220+
221+
if (endpoint.substr(0, 5) == "http:")
222+
{
223+
return true;
224+
}
225+
212226
constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_TRACES_INSECURE";
213227
constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_INSECURE";
214228
constexpr char kOldSignalEnv[] = "OTEL_EXPORTER_OTLP_TRACES_SSL_ENABLE";
@@ -251,8 +265,22 @@ bool GetOtlpDefaultTracesIsInsecure()
251265
return false;
252266
}
253267

254-
bool GetOtlpDefaultMetricsIsInsecure()
268+
bool GetOtlpDefaultGrpcMetricsIsInsecure()
255269
{
270+
std::string endpoint = GetOtlpDefaultGrpcMetricsEndpoint();
271+
272+
/* The metrics endpoint, when providing a scheme, takes precedence. */
273+
274+
if (endpoint.substr(0, 6) == "https:")
275+
{
276+
return false;
277+
}
278+
279+
if (endpoint.substr(0, 5) == "http:")
280+
{
281+
return true;
282+
}
283+
256284
constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_METRICS_INSECURE";
257285
constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_INSECURE";
258286
constexpr char kOldSignalEnv[] = "OTEL_EXPORTER_OTLP_METRICS_SSL_ENABLE";
@@ -295,8 +323,22 @@ bool GetOtlpDefaultMetricsIsInsecure()
295323
return false;
296324
}
297325

298-
bool GetOtlpDefaultLogsIsInsecure()
326+
bool GetOtlpDefaultGrpcLogsIsInsecure()
299327
{
328+
std::string endpoint = GetOtlpDefaultGrpcLogsEndpoint();
329+
330+
/* The logs endpoint, when providing a scheme, takes precedence. */
331+
332+
if (endpoint.substr(0, 6) == "https:")
333+
{
334+
return false;
335+
}
336+
337+
if (endpoint.substr(0, 5) == "http:")
338+
{
339+
return true;
340+
}
341+
300342
constexpr char kSignalEnv[] = "OTEL_EXPORTER_OTLP_LOGS_INSECURE";
301343
constexpr char kGenericEnv[] = "OTEL_EXPORTER_OTLP_INSECURE";
302344

‎exporters/otlp/test/otlp_grpc_exporter_test.cc‎

Lines changed: 71 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -168,7 +168,7 @@ TEST_F(OtlpGrpcExporterTestPeer, ConfigFromEnv)
168168
const std::string cacert_str = "--begin and end fake cert--";
169169
setenv("OTEL_EXPORTER_OTLP_CERTIFICATE_STRING", cacert_str.c_str(), 1);
170170
setenv("OTEL_EXPORTER_OTLP_SSL_ENABLE", "True", 1);
171-
const std::string endpoint = "http://localhost:9999";
171+
const std::string endpoint = "https://localhost:9999";
172172
setenv("OTEL_EXPORTER_OTLP_ENDPOINT", endpoint.c_str(), 1);
173173
setenv("OTEL_EXPORTER_OTLP_TIMEOUT", "20050ms", 1);
174174
setenv("OTEL_EXPORTER_OTLP_HEADERS", "k1=v1,k2=v2", 1);
@@ -211,6 +211,76 @@ TEST_F(OtlpGrpcExporterTestPeer, ConfigFromEnv)
211211
}
212212
# endif
213213

214+
# ifndef NO_GETENV
215+
// Test exporter configuration options with use_ssl_credentials
216+
TEST_F(OtlpGrpcExporterTestPeer, ConfigHttpsSecureFromEnv)
217+
{
218+
// https takes precedence over insecure
219+
const std::string endpoint = "https://localhost:9999";
220+
setenv("OTEL_EXPORTER_OTLP_ENDPOINT", endpoint.c_str(), 1);
221+
setenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE", "true", 1);
222+
223+
std::unique_ptr<OtlpGrpcExporter> exporter(new OtlpGrpcExporter());
224+
EXPECT_EQ(GetOptions(exporter).use_ssl_credentials, true);
225+
EXPECT_EQ(GetOptions(exporter).endpoint, endpoint);
226+
227+
unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT");
228+
unsetenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE");
229+
}
230+
# endif
231+
232+
# ifndef NO_GETENV
233+
// Test exporter configuration options with use_ssl_credentials
234+
TEST_F(OtlpGrpcExporterTestPeer, ConfigHttpInsecureFromEnv)
235+
{
236+
// http takes precedence over secure
237+
const std::string endpoint = "http://localhost:9999";
238+
setenv("OTEL_EXPORTER_OTLP_ENDPOINT", endpoint.c_str(), 1);
239+
setenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE", "false", 1);
240+
241+
std::unique_ptr<OtlpGrpcExporter> exporter(new OtlpGrpcExporter());
242+
EXPECT_EQ(GetOptions(exporter).use_ssl_credentials, false);
243+
EXPECT_EQ(GetOptions(exporter).endpoint, endpoint);
244+
245+
unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT");
246+
unsetenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE");
247+
}
248+
# endif
249+
250+
# ifndef NO_GETENV
251+
// Test exporter configuration options with use_ssl_credentials
252+
TEST_F(OtlpGrpcExporterTestPeer, ConfigUnknownSecureFromEnv)
253+
{
254+
const std::string endpoint = "localhost:9999";
255+
setenv("OTEL_EXPORTER_OTLP_ENDPOINT", endpoint.c_str(), 1);
256+
setenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE", "false", 1);
257+
258+
std::unique_ptr<OtlpGrpcExporter> exporter(new OtlpGrpcExporter());
259+
EXPECT_EQ(GetOptions(exporter).use_ssl_credentials, true);
260+
EXPECT_EQ(GetOptions(exporter).endpoint, endpoint);
261+
262+
unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT");
263+
unsetenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE");
264+
}
265+
# endif
266+
267+
# ifndef NO_GETENV
268+
// Test exporter configuration options with use_ssl_credentials
269+
TEST_F(OtlpGrpcExporterTestPeer, ConfigUnknownInsecureFromEnv)
270+
{
271+
const std::string endpoint = "localhost:9999";
272+
setenv("OTEL_EXPORTER_OTLP_ENDPOINT", endpoint.c_str(), 1);
273+
setenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE", "true", 1);
274+
275+
std::unique_ptr<OtlpGrpcExporter> exporter(new OtlpGrpcExporter());
276+
EXPECT_EQ(GetOptions(exporter).use_ssl_credentials, false);
277+
EXPECT_EQ(GetOptions(exporter).endpoint, endpoint);
278+
279+
unsetenv("OTEL_EXPORTER_OTLP_ENDPOINT");
280+
unsetenv("OTEL_EXPORTER_OTLP_TRACES_INSECURE");
281+
}
282+
# endif
283+
214284
} // namespace otlp
215285
} // namespace exporter
216286
OPENTELEMETRY_END_NAMESPACE

0 commit comments

Comments
 (0)