refactor(observability): infer OTLP TLS from endpoint scheme

[stwilt]

📋 What does this PR do?

Replaces the always-insecure=True default for the OTLP gRPC span exporter and metrics exporter with TLS inferred from the endpoint scheme via urllib.parse.urlparse:

• http://... → insecure=True (plaintext)
• https://... → insecure=False (TLS)
• bare host:port → insecure=False (TLS — secure default, matches the OTel SDK)
• Explicit insecure= in exporter_config / kwargs always wins

Also drops the broken OTLP_GRPC_ENDPOINT_SECURITY env-var lookup in the metrics factory, which never bool-parsed strings and so was effectively insecure=True regardless of the env var's value.

🔍 Why is this needed?

The previous default would silently send telemetry over plaintext gRPC even when the configured endpoint was an https:// URL — a quiet security footgun. Inferring from the URL scheme matches what most OTel SDKs (and the Go exporters this code originally mirrored) do, and gives https:// and bare-host endpoints a secure-by-default behavior. The default http://localhost:4317 still infers insecure=True, so local collectors keep working with no config change.

🧪 How was it tested?

• pytest tests/test_tracing.py — 24/24 pass
• New cases cover http://, https://, uppercase scheme, bare host:port, empty/None, and explicit insecure=True override against a bare-host endpoint
• python -c "from openfilter.filter_runtime.open_telemetry.open_telemetry_exporter_factory import ExporterFactory" — imports clean

🔗 Related Issues

None — no ticket for this change.

✅ Checklist

• I have signed all commits in compliance with the DCO (git commit -s)
• I have added or updated tests as needed
• I have added or updated documentation as needed (no user-facing docs touched — internal exporter default)

[lucasmundim]

Solid security fix and the bug call-out on the env-var lookup is correct. A few items before merging — flagged inline. Two non-inline items not in the diff that also want attention:

• RELEASE.md:352 and docs/overview.md:935-952 still document OTLP_GRPC_ENDPOINT_SECURITY / don't mention the new scheme-based inference. Worth a docs pass in this PR or a follow-up.

[shingonoide]

LGTM on the security direction; +1 to Mundão's metrics-factory regression — that's the merge blocker.

Two complementary notes:

• OTLP_GRPC_ENDPOINT_SECURITY was a full no-op, not a half-working knob — worth being precise in RELEASE.md beyond just deleting the stale doc. os.getenv(name, True) returns the literal True only when unset; any set value came back as a string, and every non-empty string is truthy, so insecure=True regardless of what an operator wrote. No one was ever getting TLS through this var. The actual behavior change is narrower: operators with bare host:port endpoints flip from plaintext to TLS. That's the line worth in the changelog.
• New hard import coupling. from openfilter.observability.tracing import infer_otlp_insecure at the top of the metrics factory means the metrics module now fails to import if tracing does. Low-probability but easy to avoid with a lazy import inside the otlp_grpc branch, or by moving the helper to a small openfilter/observability/_otlp.py so neither side imports the other's heavy module.

Otherwise clean — urlparse over regex is the right call, and the gRPC path now infers transport the same way otlp_http already does, which is good consistency.

[stwilt]

Updated in b9405dd — see inline thread replies for the per-comment items.

@lucasmundim — five inline points addressed; on the docs you flagged: the dead OTLP_GRPC_ENDPOINT_SECURITY line under v0.1.6 of RELEASE.md is gone, and the new v0.1.31 entry incorporates @shingonoide's precision wording. docs/overview.md got a TLS-inference note + an annotation on the gRPC endpoint row in the env-var table.

@shingonoide — both points taken:

• Changelog precision: the v0.1.31 entry now spells out that os.getenv(name, True) only ever returned the literal True when unset, every set value came back as a truthy string, and the actual operator-visible change is narrow (bare host:port endpoints flip from plaintext to TLS).
• Import decoupling: helper moved to openfilter/observability/_otlp.py; both factories import from there. tracing.py re-exports infer_otlp_insecure for backward compat with any existing importers (the test file was the only in-tree caller).

[lucasmundim]

All five inline points addressed and the two follow-ups from @shingonoide too. Verified locally:

• Metrics-factory or "http://localhost:4317" fallback lands correctly — with empty env and no kwargs, the built exporter opens an insecure channel against localhost:4317. The original regression I flagged would have produced a secure channel against the SDK's default endpoint here.
• End-to-end TLS behavior verified by intercepting grpc.{secure,insecure}_channel: http:// → insecure, https:// → TLS, bare host:port → TLS, and explicit insecure=True against bare host correctly overrides to insecure.
• 27/27 tests pass across tests/test_tracing.py and the new tests/test_open_telemetry_exporter_factory.py.
• Decoupling clean: openfilter/observability/_otlp.py is the single source, metrics factory imports from there (no tracing import), and tracing.py re-exports infer_otlp_insecure in __all__ for any external callers.
• RELEASE.md v0.1.31 entry is precise about the actual behavior change (only bare host:port operators flip plaintext → TLS), and the stale v0.1.6 line is gone. docs/overview.md got the TLS-inference callout.

One minor observation, not blocking: https://... + explicit insecure=True ends up as a secure channel — the underlying OTel SDK preserves TLS when the URL scheme is https:// regardless of the kwarg. The factory passes the kwarg through faithfully; this is SDK behavior, not a PR bug, and the realistic bare-host override case works correctly.

LGTM. Thanks for the careful follow-through on every point.

[lucasmundim]

Code review

Found 1 issue:

1. The "Mirror the tracing factory's localhost fallback" comment overstates parity — only the fallback is mirrored, not the env-var precedence chain.

[lucasmundim]

Re-review on 3cecd64.

The env-var precedence chain is the right shape and the comment block at lines 25-33 clearly documents the ordering. Two items inline — neither blocking, but the test one is the same shape as the assertIsNotNone weakness we dropped in the prior round and worth tightening before this merges.

[lucasmundim]

Re-review on 20c0c65.

Both inline items from the prior round are addressed:

• Test assertions are now load-bearing. Every case patches OTLPGrpcExporter in the factory module's namespace and asserts the exact (endpoint, insecure) kwargs. test_unset_endpoint_falls_back_to_localhost_plaintext pins ("http://localhost:4317", True); the two precedence tests pin the winning endpoint and inferred insecure. Reversing precedence or letting endpoint=None reach the SDK would now fail loudly.
• otlp_http asymmetry documented. The new comment at L55-60 makes the intentional divergence vs. the gRPC branch explicit, and the SDK-default claim checks out — OTLPMetricExporter(endpoint=None)._endpoint == "http://localhost:4318/v1/metrics", so a bare-host fallback would have silently dropped the path suffix.

29/29 tests pass locally. LGTM.

One non-blocking follow-up worth noting: the otlp_http branch grew the same env-var precedence chain but the new test file only covers otlp_grpc. Reasonable scope cut for this PR; if symmetric coverage is wanted later, the existing tests are a cheap template.

[lucasmundim]

Re-review on 440a52f. The new commit closes the third call site — client.py:build_exporter is the metrics builder OpenTelemetryClient wires up in the live runtime (line 157), so this is the path that was producing the WRONG_VERSION_NUMBER handshake error in filter pods against the in-cluster collector.

35/35 tests pass locally across the three relevant files. The new test file pins exact (endpoint, insecure) kwargs — same load-bearing shape as test_open_telemetry_exporter_factory.py from the prior round.

LGTM. Two non-blocking notes inline, both about scope-symmetry vs. the other two builders, neither introduced by this PR.

[lucasmundim]

Pre-existing, not introduced here, but worth a follow-up: this env-var chain is shorter than the other two builders we aligned in 3cecd64. ExporterFactory.otlp_grpc and tracing.build_span_exporter.otlp_grpc both also consult OTEL_EXPORTER_OTLP_ENDPOINT and OTEL_EXPORTER_OTLP_GRPC_ENDPOINT before falling back to localhost. An operator setting only the standard OTel env vars would get them honored by traces and by filter_runtime's metrics factory, but not by this builder. Out of scope for this PR (the diff doesn't touch the chain) — flagging for a follow-up that fans out the same 4-step chain here.

[lucasmundim]

Worth a small tightening: this entry says "the exporter factory and tracing builder," but 440a52f adds the same inference to a third site — observability/client.py:build_exporter — which is the metrics path OpenTelemetryClient uses in the live runtime and the one that produced the WRONG_VERSION_NUMBER symptom operators would search the changelog for. Suggest something like "the exporter factory, tracing builder, and OpenTelemetryClient's metrics builder now use urllib.parse.urlparse..." so the changelog matches symptom to fix.