From a3016788620d9e11f213ebe5320dd069746af02f Mon Sep 17 00:00:00 2001 From: Jean Scherf Date: Mon, 19 Oct 2026 10:41:12 -0300 Subject: [PATCH] fix(auditlog_ng): infer OTLP protocol from endpoint when env unset The audit client chose its OTLP protocol from OTEL_EXPORTER_OTLP_PROTOCOL, defaulting to gRPC when it was unset. The platform instrumentation never injects that variable, so an HTTP audit destination (http://host:4318/v1/logs) was always dialed with a gRPC exporter, breaking delivery. Derive the protocol from the resolved endpoint shape instead: an http(s) URL with a path -> http/protobuf, a bare host:port -> grpc. An explicit OTEL_EXPORTER_OTLP_PROTOCOL still wins when set, and gRPC remains the fallback when both the env and the endpoint shape are inconclusive. Add unit tests covering every branch of the shape inference and the resolver. --- src/sap_cloud_sdk/core/auditlog_ng/client.py | 57 +++++++++- .../core/unit/auditlog_ng/unit/test_client.py | 106 +++++++++++++++++- 2 files changed, 156 insertions(+), 7 deletions(-) diff --git a/src/sap_cloud_sdk/core/auditlog_ng/client.py b/src/sap_cloud_sdk/core/auditlog_ng/client.py index 0de95f17..3835e2b3 100644 --- a/src/sap_cloud_sdk/core/auditlog_ng/client.py +++ b/src/sap_cloud_sdk/core/auditlog_ng/client.py @@ -5,10 +5,12 @@ """ import json +import logging import os import uuid from datetime import datetime, timezone from typing import Any, Optional, cast +from urllib.parse import urlparse import protovalidate from protovalidate import ValidationError as ProtoValidationError @@ -42,6 +44,8 @@ from sap_cloud_sdk.core.runtime_context.providers import APP_TENANT_ID, USER_ID from sap_cloud_sdk.ias._context import get_auth_context +logger = logging.getLogger(__name__) + def _fill_common_from_auth_context(event: Message) -> None: """Back-fill common fields from the request auth context. @@ -72,12 +76,61 @@ def _fill_common_from_auth_context(event: Message) -> None: common.timestamp.FromDatetime(datetime.now(timezone.utc)) +def _infer_protocol_from_endpoint(endpoint: Optional[str]) -> Optional[str]: + """Infer the OTLP protocol from the shape of *endpoint*. + + An OTLP/HTTP endpoint is a URL with an http(s) scheme and a path, for example + ``http://host:4318/v1/logs``. An OTLP/gRPC endpoint is a bare ``host:port`` (optionally + with a scheme) and no path, for example ``audit.example.com:443``. Returns + ``"http/protobuf"``, ``"grpc"``, or ``None`` when the shape is inconclusive. + """ + ep = (endpoint or "").strip() + if not ep: + return None + parsed = urlparse(ep if "://" in ep else f"//{ep}") + if not parsed.netloc: + return None + has_path = bool(parsed.path.strip("/")) + if parsed.scheme.lower() in ("http", "https") and has_path: + return "http/protobuf" + if not has_path: + return "grpc" + return None + + +def _resolve_protocol(endpoint: str) -> str: + """Resolve the OTLP protocol for the audit exporter. + + An explicit ``OTEL_EXPORTER_OTLP_PROTOCOL`` wins when set, so deliberate overrides keep + working. When it is unset, the protocol is inferred from the endpoint shape rather than + blindly defaulting to gRPC. The old default previously produced a gRPC exporter aimed at + an HTTP endpoint whenever the environment did not inject the protocol (for example under + an env-only instrumentation setup), which broke delivery. Falls back to ``grpc`` when the + env is unset and the endpoint shape is inconclusive. + """ + explicit = os.getenv(ENV_OTLP_PROTOCOL) + if explicit: + return explicit.lower() + inferred = _infer_protocol_from_endpoint(endpoint) + if inferred is not None: + logger.debug( + "OTEL_EXPORTER_OTLP_PROTOCOL unset; inferred %r from the audit endpoint shape.", + inferred, + ) + return inferred + return "grpc" + + def _create_log_exporter( config: AuditLogNGConfig, credentials: Optional[grpc.ChannelCredentials], ): - """Create an OTLP log exporter based on OTEL_EXPORTER_OTLP_PROTOCOL.""" - protocol = os.getenv(ENV_OTLP_PROTOCOL, "grpc").lower() + """Create an OTLP log exporter, choosing gRPC vs HTTP from the endpoint shape. + + The protocol is derived from ``config.endpoint`` (see :func:`_resolve_protocol`) so the + client is not broken by a missing or mismatched ``OTEL_EXPORTER_OTLP_PROTOCOL``. + """ + protocol = _resolve_protocol(config.endpoint) if protocol == "grpc": return GRPCLogExporter( endpoint=config.endpoint, diff --git a/tests/core/unit/auditlog_ng/unit/test_client.py b/tests/core/unit/auditlog_ng/unit/test_client.py index 8d22a882..422a9021 100644 --- a/tests/core/unit/auditlog_ng/unit/test_client.py +++ b/tests/core/unit/auditlog_ng/unit/test_client.py @@ -8,10 +8,17 @@ import pytest -from sap_cloud_sdk.core.auditlog_ng.client import AuditClient, _fill_common_from_auth_context +from sap_cloud_sdk.core.auditlog_ng.client import ( + AuditClient, + _fill_common_from_auth_context, + _infer_protocol_from_endpoint, + _resolve_protocol, +) from sap_cloud_sdk.core.auditlog_ng.config import AuditLogNGConfig, SCHEMA_URL from sap_cloud_sdk.core.auditlog_ng.exceptions import ValidationError -from sap_cloud_sdk.core.auditlog_ng.gen.sap.auditlog.auditevent.v2 import auditevent_pb2 as pb +from sap_cloud_sdk.core.auditlog_ng.gen.sap.auditlog.auditevent.v2 import ( + auditevent_pb2 as pb, +) from sap_cloud_sdk.core.telemetry import Module, Operation from sap_cloud_sdk.ias._context import set_auth_context @@ -70,7 +77,9 @@ def _make_mocked_client( ) -def _make_mock_event(tenant_id="tenant-123", descriptor_full_name="sap.auditlog.auditevent.v2.DataAccess"): +def _make_mock_event( + tenant_id="tenant-123", descriptor_full_name="sap.auditlog.auditevent.v2.DataAccess" +): event = MagicMock() event.common.tenant_id = tenant_id event.DESCRIPTOR.full_name = descriptor_full_name @@ -201,11 +210,15 @@ def test_send_json_success(self): def test_send_uses_descriptor_name_when_event_type_missing(self): client, mock_logger, _, _, patcher, _ = _make_mocked_client() try: - event = _make_mock_event(descriptor_full_name="sap.auditlog.auditevent.v2.ConfigurationChange") + event = _make_mock_event( + descriptor_full_name="sap.auditlog.auditevent.v2.ConfigurationChange" + ) client.send(event) _, kwargs = mock_logger.emit.call_args - assert kwargs["event_name"] == "sap.auditlog.auditevent.v2.ConfigurationChange" + assert ( + kwargs["event_name"] == "sap.auditlog.auditevent.v2.ConfigurationChange" + ) finally: patcher.stop() @@ -340,6 +353,89 @@ def test_unsupported_protocol_raises( AuditClient(_make_config()) +class TestInferProtocolFromEndpoint: + @pytest.mark.parametrize( + ("endpoint", "expected"), + [ + # HTTP: http(s) scheme with a path (the OTLP/HTTP signal endpoint shape). + ("http://collector:4318/v1/logs", "http/protobuf"), + ("https://collector.example.com/v1/logs", "http/protobuf"), + # gRPC: bare host:port, no path (the OTLP/gRPC shape), with or without scheme. + ("audit.example.com:443", "grpc"), + ("localhost:4317", "grpc"), + ("https://audit.example.com:443", "grpc"), + ("grpc://audit.example.com:443", "grpc"), + # gRPC: host without a port and without a path still has the bare-host shape. + ("audit.example.com", "grpc"), + # No path means gRPC even with an http scheme and a port: a path is the only + # positive HTTP signal, and OTLP/gRPC endpoints may carry an http(s) scheme to + # indicate (in)secure transport. An explicit env override covers the rare + # http-base-without-path case. + ("http://collector:4318", "grpc"), + # Inconclusive shapes return None so the caller can fall back. + ("", None), + (" ", None), + (None, None), + ("/v1/logs", None), + ("grpc://audit.example.com:443/v1/logs", None), + ], + ) + def test_infers_shape(self, endpoint, expected): + assert _infer_protocol_from_endpoint(endpoint) == expected + + +class TestResolveProtocol: + def test_explicit_env_wins_over_endpoint_shape(self, monkeypatch): + # A deliberate override is honoured even if it disagrees with the endpoint shape. + monkeypatch.setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "http/protobuf") + assert _resolve_protocol("audit.example.com:443") == "http/protobuf" + + def test_explicit_env_is_lowercased(self, monkeypatch): + monkeypatch.setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "GRPC") + assert _resolve_protocol("http://collector:4318/v1/logs") == "grpc" + + def test_infers_http_when_env_unset(self, monkeypatch): + monkeypatch.delenv("OTEL_EXPORTER_OTLP_PROTOCOL", raising=False) + assert _resolve_protocol("http://collector:4318/v1/logs") == "http/protobuf" + + def test_infers_grpc_when_env_unset(self, monkeypatch): + monkeypatch.delenv("OTEL_EXPORTER_OTLP_PROTOCOL", raising=False) + assert _resolve_protocol("audit.example.com:443") == "grpc" + + def test_falls_back_to_grpc_when_env_unset_and_shape_inconclusive( + self, monkeypatch + ): + monkeypatch.delenv("OTEL_EXPORTER_OTLP_PROTOCOL", raising=False) + assert _resolve_protocol("") == "grpc" + + def test_empty_env_falls_through_to_endpoint_inference(self, monkeypatch): + # An env var set to "" is not a deliberate choice; infer from the endpoint instead. + monkeypatch.setenv("OTEL_EXPORTER_OTLP_PROTOCOL", "") + assert _resolve_protocol("http://collector:4318/v1/logs") == "http/protobuf" + + +class TestProtocolInferredByClient: + """End-to-end: the exporter the client builds follows the endpoint when env is unset.""" + + @patch("sap_cloud_sdk.core.auditlog_ng.client.HTTPLogExporter") + @patch("sap_cloud_sdk.core.auditlog_ng.client.LoggerProvider") + def test_http_endpoint_without_env_builds_http_exporter( + self, mock_provider_cls, mock_http_exporter_cls, monkeypatch + ): + monkeypatch.delenv("OTEL_EXPORTER_OTLP_PROTOCOL", raising=False) + AuditClient(_make_config(endpoint="http://collector:4318/v1/logs")) + mock_http_exporter_cls.assert_called_once() + + @patch("sap_cloud_sdk.core.auditlog_ng.client.GRPCLogExporter") + @patch("sap_cloud_sdk.core.auditlog_ng.client.LoggerProvider") + def test_grpc_endpoint_without_env_builds_grpc_exporter( + self, mock_provider_cls, mock_grpc_exporter_cls, monkeypatch + ): + monkeypatch.delenv("OTEL_EXPORTER_OTLP_PROTOCOL", raising=False) + AuditClient(_make_config(endpoint="audit.example.com:443")) + mock_grpc_exporter_cls.assert_called_once() + + class TestFillCommonFromAuthContext: def setup_method(self): set_auth_context(None)