From dd7787c67ec10cfe64cd4a81691deb7382e56676 Mon Sep 17 00:00:00 2001 From: Artur Shiriev Date: Thu, 29 Oct 2026 17:40:29 +0300 Subject: [PATCH] Default opentelemetry_insecure to None so OTEL_EXPORTER_OTLP_INSECURE takes effect --- docs/introduction/configuration.md | 6 +- .../instruments/opentelemetry_instrument.py | 30 +++++--- .../test_opentelemetry_instrument.py | 69 ++++++++++++++++++- tests/test_config_cascade.py | 2 +- 4 files changed, 93 insertions(+), 14 deletions(-) diff --git a/docs/introduction/configuration.md b/docs/introduction/configuration.md index e617f5e..12493bd 100644 --- a/docs/introduction/configuration.md +++ b/docs/introduction/configuration.md @@ -214,7 +214,7 @@ Additional parameters: - `opentelemetry_endpoint` - will be passed to `OTLPSpanExporter` as endpoint. Under `opentelemetry_exporter_protocol="http"` this is a full URL (e.g. `http://collector:4318/v1/traces`). - `opentelemetry_namespace` - will be passed to the `Resource`. - `opentelemetry_exporter_protocol` - OTLP exporter transport: `"grpc"` (default, needs the `otl` extra) or `"http"` (needs the `otl-http` extra, no `grpcio` - installable on free-threaded Python). -- `opentelemetry_insecure` - whether the gRPC OTLP connection is insecure (default: `True`; gRPC only; for `http` the endpoint URL scheme carries security). While it is `True`, an endpoint pointing at a non-local host triggers a warning that telemetry is sent unencrypted. +- `opentelemetry_insecure` - whether the gRPC OTLP connection is insecure (default: `None`; gRPC only; for `http` the endpoint URL scheme carries security). Left `None`, the exporter decides: `OTEL_EXPORTER_OTLP_TRACES_INSECURE` or `OTEL_EXPORTER_OTLP_METRICS_INSECURE` first, then `OTEL_EXPORTER_OTLP_INSECURE`, then the endpoint's scheme, where only `http://` connects in plaintext. Since 2.0 a schemeless endpoint such as `collector:4317` uses TLS, so write `http://collector:4317` for a plaintext collector, or set the field to `True`, which a `unix://` socket needs too. An `https://` endpoint uses TLS whatever the field says. When the connection is plaintext and the endpoint points at a non-local host, a warning says telemetry is sent unencrypted. - `opentelemetry_instrumentors` - a list of extra instrumentors. - `opentelemetry_log_traces` - traces will be logged to stdout. - `opentelemetry_sampler` - an `opentelemetry.sdk.trace.sampling.Sampler` deciding which traces are recorded. Unset, the SDK's own default applies: `parentbased_always_on`, which records every trace that is not the child of a non-recording remote parent, unless `OTEL_TRACES_SAMPLER` / `OTEL_TRACES_SAMPLER_ARG` say otherwise. @@ -244,8 +244,8 @@ counted by both. ```python config = FastAPIConfig( service_name="my-service", - opentelemetry_endpoint="localhost:4317", - opentelemetry_metrics_endpoint="localhost:4317", + opentelemetry_endpoint="http://localhost:4317", + opentelemetry_metrics_endpoint="http://localhost:4317", ) ``` diff --git a/lite_bootstrap/instruments/opentelemetry_instrument.py b/lite_bootstrap/instruments/opentelemetry_instrument.py index a275b61..c9218b0 100644 --- a/lite_bootstrap/instruments/opentelemetry_instrument.py +++ b/lite_bootstrap/instruments/opentelemetry_instrument.py @@ -74,7 +74,7 @@ class OpenTelemetryConfig(OpenTelemetryServiceFieldsConfig): ) opentelemetry_endpoint: str | None = None opentelemetry_metrics_endpoint: str | None = None - opentelemetry_insecure: bool = True + opentelemetry_insecure: bool | None = None opentelemetry_exporter_protocol: typing.Literal["grpc", "http"] = "grpc" opentelemetry_instrumentors: list[typing.Union[InstrumentorWithParams, "BaseInstrumentor"]] = dataclasses.field( default_factory=list @@ -96,17 +96,18 @@ def __post_init__(self) -> None: def _parse_remote_insecure_hosts(self) -> list[str]: """Return each distinct host an insecure, non-local endpoint points at, in declaration order.""" - endpoints = (self.opentelemetry_endpoint, self.opentelemetry_metrics_endpoint) - hosts = (self._parse_remote_insecure_host(endpoint) for endpoint in endpoints) + endpoints = ( + (self.opentelemetry_endpoint, "OTEL_EXPORTER_OTLP_TRACES_INSECURE"), + (self.opentelemetry_metrics_endpoint, "OTEL_EXPORTER_OTLP_METRICS_INSECURE"), + ) + hosts = (self._parse_remote_insecure_host(endpoint, env_var) for endpoint, env_var in endpoints) return list(dict.fromkeys(host for host in hosts if host is not None)) - def _parse_remote_insecure_host(self, endpoint: str | None) -> str | None: + def _parse_remote_insecure_host(self, endpoint: str | None, signal_insecure_env_var: str) -> str | None: """Return the host name if the endpoint is insecure AND non-local; else None.""" if self.opentelemetry_exporter_protocol != "grpc": return None - if not endpoint or not self.opentelemetry_insecure: - return None - if endpoint.startswith("unix://"): + if not endpoint or endpoint.startswith("unix://"): return None # urlparse treats schemeless input as `path`, misparsing `host:port` forms. # Prepend `//` so urlparse always sees a network-location-style input. @@ -114,14 +115,25 @@ def _parse_remote_insecure_host(self, endpoint: str | None) -> str | None: if "://" not in raw: raw = f"//{raw}" parsed = urllib.parse.urlparse(raw) - # The gRPC exporter forces TLS for https whatever insecure says. - if parsed.scheme == "https": + if not self._is_grpc_plaintext(parsed.scheme, signal_insecure_env_var): return None host = (parsed.hostname or "").lower() if host in _LOCAL_HOSTS: return None return host + def _is_grpc_plaintext(self, scheme: str, signal_insecure_env_var: str) -> bool: + """Decide insecure as the gRPC exporter does: https, then the field, the variables, the scheme.""" + if scheme == "https": + return False + if self.opentelemetry_insecure is not None: + return self.opentelemetry_insecure + for env_var in (signal_insecure_env_var, "OTEL_EXPORTER_OTLP_INSECURE"): + value = os.environ.get(env_var) + if value is not None: + return value.lower() == "true" + return scheme == "http" + @dataclasses.dataclass(kw_only=True, frozen=True) class OpenTelemetryAsgiConfig(OpenTelemetryConfig): diff --git a/tests/instruments/test_opentelemetry_instrument.py b/tests/instruments/test_opentelemetry_instrument.py index c80a80f..66cc0ad 100644 --- a/tests/instruments/test_opentelemetry_instrument.py +++ b/tests/instruments/test_opentelemetry_instrument.py @@ -293,6 +293,73 @@ def test_opentelemetry_config_no_warning_for_unix_socket_endpoint() -> None: ) +_INSECURE_ENV_VARS: typing.Final = ( + "OTEL_EXPORTER_OTLP_INSECURE", + "OTEL_EXPORTER_OTLP_TRACES_INSECURE", + "OTEL_EXPORTER_OTLP_METRICS_INSECURE", +) +_SIGNAL_INSECURE_ENV_VAR: typing.Final = "OTEL_EXPORTER_OTLP_{signal}_INSECURE" +_REMOTE_HTTP: typing.Final = "http://collector.example.com:4317" +_REMOTE_SCHEMELESS: typing.Final = "collector.example.com:4317" + + +@pytest.fixture +def without_insecure_env(monkeypatch: pytest.MonkeyPatch) -> None: + for env_var in _INSECURE_ENV_VARS: + monkeypatch.delenv(env_var, raising=False) + + +@pytest.mark.usefixtures("without_insecure_env") +@pytest.mark.parametrize("signal", ["traces", "metrics"]) +@pytest.mark.parametrize( + ("endpoint", "insecure", "env", "expected_plaintext"), + [ + pytest.param(_REMOTE_HTTP, None, {}, True, id="http-scheme"), + pytest.param(_REMOTE_SCHEMELESS, None, {}, False, id="schemeless"), + pytest.param("https://collector.example.com:4317", True, {}, False, id="https-forces-tls"), + pytest.param(_REMOTE_SCHEMELESS, True, {}, True, id="field-on"), + pytest.param(_REMOTE_HTTP, False, {}, False, id="field-off"), + pytest.param(_REMOTE_HTTP, None, {"OTEL_EXPORTER_OTLP_INSECURE": "false"}, False, id="generic-env-off"), + pytest.param(_REMOTE_SCHEMELESS, None, {"OTEL_EXPORTER_OTLP_INSECURE": "true"}, True, id="generic-env-on"), + pytest.param(_REMOTE_HTTP, None, {_SIGNAL_INSECURE_ENV_VAR: "false"}, False, id="signal-env-off"), + pytest.param(_REMOTE_SCHEMELESS, None, {_SIGNAL_INSECURE_ENV_VAR: "true"}, True, id="signal-env-on"), + pytest.param( + _REMOTE_SCHEMELESS, + None, + {_SIGNAL_INSECURE_ENV_VAR: "true", "OTEL_EXPORTER_OTLP_INSECURE": "false"}, + True, + id="signal-env-wins", + ), + pytest.param(_REMOTE_HTTP, True, {_SIGNAL_INSECURE_ENV_VAR: "false"}, True, id="field-wins"), + ], +) +def test_insecure_warning_fires_exactly_when_the_grpc_exporter_sends_plaintext( + signal: typing.Literal["traces", "metrics"], + endpoint: str, + insecure: bool | None, + env: dict[str, str], + expected_plaintext: bool, +) -> None: + signal_env = {env_var.format(signal=signal.upper()): value for env_var, value in env.items()} + endpoint_field = "opentelemetry_endpoint" if signal == "traces" else "opentelemetry_metrics_endpoint" + fields: dict[str, typing.Any] = {endpoint_field: endpoint} + if insecure is not None: + fields["opentelemetry_insecure"] = insecure + + with patch.dict(os.environ, signal_env): + with warnings.catch_warnings(record=True) as caught: + warnings.simplefilter("always") + config = OpenTelemetryConfig(**fields) + instrument = OpenTelemetryInstrument(bootstrap_config=config) + exporter = instrument._build_span_exporter() if signal == "traces" else instrument._build_metric_exporter() # noqa: SLF001 + assert exporter is not None + try: + assert exporter._insecure is expected_plaintext # noqa: SLF001 # ty: ignore[unresolved-attribute] + assert any("unencrypted" in str(w.message) for w in caught) is expected_plaintext + finally: + exporter.shutdown() + + def test_opentelemetry_teardown_aggregates_instrumentor_and_shutdown_errors() -> None: class BoomInstrumentor(BaseInstrumentor): def instrumentation_dependencies(self) -> typing.Collection[str]: @@ -401,7 +468,7 @@ def test_bootstrap_grpc_protocol_uses_grpc_exporter() -> None: try: with patch.object(otel_module, "OTLPGrpcSpanExporter") as mock_grpc: instrument.bootstrap() - mock_grpc.assert_called_once_with(endpoint="localhost:4317", insecure=True) + mock_grpc.assert_called_once_with(endpoint="localhost:4317", insecure=None) finally: instrument.teardown() diff --git a/tests/test_config_cascade.py b/tests/test_config_cascade.py index db45b3d..dbcf618 100644 --- a/tests/test_config_cascade.py +++ b/tests/test_config_cascade.py @@ -15,7 +15,7 @@ from lite_bootstrap.instruments.opentelemetry_instrument import OpenTelemetryConfig -_REMOTE_INSECURE_ENDPOINT: typing.Final = "collector.example.com:4317" +_REMOTE_INSECURE_ENDPOINT: typing.Final = "http://collector.example.com:4317" @pytest.mark.parametrize(