feat(otlp): support INSECURE environment variables for gRPC#3365
Conversation
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #3365 +/- ##
======================================
Coverage 82.9% 82.9%
======================================
Files 130 130
Lines 27350 27463 +113
======================================
+ Hits 22675 22787 +112
- Misses 4675 4676 +1 ☔ View full report in Codecov by Harness. 🚀 New features to boost your workflow:
|
c473874 to
d638639
Compare
a4a54f1 to
e976192
Compare
4f926e3 to
21c6118
Compare
ba6c412 to
c5d919d
Compare
scottgerring
left a comment
There was a problem hiding this comment.
Hey @bryantbiggs thanks for this one and sorry for very slow turnaround - it'll help with OTLP stabilisation!
Some comments inline but I reckon it's almost there 🚢
Add support for 4 INSECURE environment variables per the OTLP exporter specification (SHOULD-level): - OTEL_EXPORTER_OTLP_INSECURE (generic) - OTEL_EXPORTER_OTLP_TRACES_INSECURE - OTEL_EXPORTER_OTLP_METRICS_INSECURE - OTEL_EXPORTER_OTLP_LOGS_INSECURE Per the spec, INSECURE only applies to gRPC (not HTTP - HTTP security is determined by URL scheme). There is intentionally no programmatic builder method - this is env-var-only per spec. When an endpoint has no explicit scheme: - INSECURE=true: connect without TLS (prepend http://) - INSECURE=false (default): connect with TLS (prepend https://, auto- apply ClientTlsConfig::new() when TLS features are enabled) Endpoints with explicit http:// or https:// scheme ignore the INSECURE flag entirely. Scheme detection is case-insensitive. Note: Schemeless endpoints (e.g., collector.example.com:4317) now default to https:// instead of being passed as-is. Set OTEL_EXPORTER_OTLP_INSECURE=true for plaintext connections. Refs: open-telemetry#774, open-telemetry#984
- Treat any explicit endpoint scheme (http, https, unix, ...) as-is instead of only http/https, fixing `unix:///path` being rewritten to `https://unix:///path`. The scheme logic is extracted into apply_insecure_scheme() with feature-agnostic unit tests so it is exercised under --all-features (the prior TLS-gated tests never compiled in CI). - Run the new schemeless/insecure integration tests under a tokio runtime and gate the new schemeless and resolve_insecure tests on `trace` so they no longer reach trace-gated re-exports under per-feature builds. - Reword the INSECURE const docs to the clearer "disable TLS" phrasing. - Move the INSECURE changelog entry to vNext.
opentelemetry-otlp's tests only ran under --all-features in CI, so test code that is compiled out when other features are enabled was never exercised, and latent per-feature breakage went unnoticed. - Gate every test that references the http-proto-only `Protocol::HttpBinary` variant on `http-proto` (http exporter mod/trace/metrics/logs tests). - Run `build_with_default_transport` (span/metric/logs) under a tokio runtime; under a gRPC-only build the auto-selected tonic transport needs a reactor. - Fix a latent typo: `test_default_protocol` called a non-existent `crate::exporter::default_protocol()`; use `crate::Protocol::default()`. - Run a curated set of real-world feature combinations in scripts/test.sh (gRPC no-TLS, http-proto, http-json) so feature-gated tests compile and execute in CI. No production behavior change; test and CI only.
c5d919d to
2307ea7
Compare
Co-authored-by: Scott Gerring <scottgerring@users.noreply.github.com>
scottgerring
left a comment
There was a problem hiding this comment.
LGTM pending build completing!
@cijothomas it would be great to get this in in the quest for OTLP stabilisation
There was a problem hiding this comment.
Pull request overview
This PR adds OTLP/gRPC support for the spec-defined OTEL_EXPORTER_OTLP*_INSECURE environment variables by interpreting schemeless endpoints as either http:// (plaintext) or https:// (TLS) based on env var values, and updates tests/docs/scripts to validate behavior across feature combinations.
Changes:
- Add generic + signal-specific
*_INSECUREenv vars, and apply them when building tonic (gRPC) channels for schemeless endpoints. - Update docs/exports/changelog to surface the new env vars and the (intentional) default-to-
https://breaking behavior for schemeless endpoints. - Strengthen test execution across feature combinations and adjust test cfg/tokio usage to avoid feature-matrix blind spots.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| scripts/test.sh | Adds curated opentelemetry-otlp feature-matrix test runs to catch feature-gated compilation/execution gaps. |
| opentelemetry-otlp/src/span.rs | Introduces OTEL_EXPORTER_OTLP_TRACES_INSECURE const and switches default-transport test to #[tokio::test]. |
| opentelemetry-otlp/src/metric.rs | Introduces OTEL_EXPORTER_OTLP_METRICS_INSECURE const and switches default-transport test to #[tokio::test]. |
| opentelemetry-otlp/src/logs.rs | Introduces OTEL_EXPORTER_OTLP_LOGS_INSECURE const and switches default-transport test to #[tokio::test]. |
| opentelemetry-otlp/src/lib.rs | Documents and re-exports new INSECURE env vars (generic + per-signal). |
| opentelemetry-otlp/src/exporter/tonic/mod.rs | Applies INSECURE-driven scheme prefixing for schemeless gRPC endpoints and adds related unit/integration tests. |
| opentelemetry-otlp/src/exporter/mod.rs | Adds OTEL_EXPORTER_OTLP_INSECURE + resolve_insecure() and associated unit tests. |
| opentelemetry-otlp/src/exporter/http/trace.rs | Gating of protobuf-specific tests to http-proto feature. |
| opentelemetry-otlp/src/exporter/http/mod.rs | Tightens test cfg gating to avoid compiling tests under incompatible HTTP feature sets. |
| opentelemetry-otlp/src/exporter/http/metrics.rs | Gating of protobuf-specific tests to http-proto feature. |
| opentelemetry-otlp/src/exporter/http/logs.rs | Gating of protobuf-specific tests to http-proto feature. |
| opentelemetry-otlp/CHANGELOG.md | Adds vNext entry documenting new env vars and breaking default scheme behavior for schemeless endpoints. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Detect an explicit scheme via split_once("://") and require the
preceding segment to contain no path/query/fragment delimiter, rather
than treating any "://" substring as a scheme. A schemeless endpoint
whose path happens to contain "://" (e.g. collector:4317/path://foo)
is now correctly prefixed instead of being passed through and failing
URI parsing.
a0a9305
Summary
Add support for 4 INSECURE environment variables per the OTLP exporter specification (SHOULD-level):
OTEL_EXPORTER_OTLP_INSECURE(generic)OTEL_EXPORTER_OTLP_TRACES_INSECUREOTEL_EXPORTER_OTLP_METRICS_INSECUREOTEL_EXPORTER_OTLP_LOGS_INSECUREPer the spec, INSECURE only applies to gRPC (not HTTP — HTTP security is determined by URL scheme). There is intentionally no programmatic builder method — this is env-var-only per spec.
Behavior
When an endpoint has no explicit scheme (e.g.,
collector.example.com:4317):INSECURE=true→ prependshttp://(plaintext)INSECURE=false(default) → prependshttps://and auto-appliesClientTlsConfig::new()when TLS features are enabledEndpoints with an explicit
http://orhttps://scheme are unaffected (scheme detection is case-insensitive).Signal-specific env var takes precedence over generic;
"true"(case-insensitive) = insecure, anything else = secure.Breaking behavior change
Schemeless endpoints (e.g.,
collector.example.com:4317) now default tohttps://instead of being passed as-is toChannel::from_shared. Users with schemeless endpoints connecting to plaintext gRPC servers should setOTEL_EXPORTER_OTLP_INSECURE=true. Endpoints with an explicithttp://scheme (including the defaulthttp://localhost:4317) are unaffected.Test plan
cargo clippy -p opentelemetry-otlp --all-targets --all-features -- -D warningspassescargo test -p opentelemetry-otlp --all-features— 117 unit + 2 integration + 21 doc-tests passresolve_insecure(): signal overrides generic, default is false, case-insensitive, non-true values return false, falls back to genericbuild_channelwith INSECURE: schemeless+secure errors without TLS, schemeless+insecure succeeds, schemeless defaults to https, explicit http:// ignores INSECURE envtemp_env::with_var_unsetfor signal-specific var to prevent parallel test interferenceResolves: #774
Resolves: #984