fix(auditlog_ng): infer OTLP protocol from endpoint when env unset

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,

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)