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)