From 99c91986806e9abd765c7a8086fcc7b383e75285 Mon Sep 17 00:00:00 2001 From: Tiago Kochenborger Date: Tue, 26 May 2026 16:21:47 -0300 Subject: [PATCH 1/3] fix: record auditlog ng telemetry on client initialization --- pyproject.toml | 2 +- .../core/auditlog_ng/__init__.py | 52 ++++++++++++------- src/sap_cloud_sdk/core/auditlog_ng/client.py | 3 +- .../core/telemetry/metrics_decorator.py | 8 ++- .../core/unit/auditlog_ng/unit/test_client.py | 44 +++++++++++++++- .../auditlog_ng/unit/test_create_client.py | 51 +++++++++++++++++- .../unit/telemetry/test_metrics_decorator.py | 18 +++++++ uv.lock | 2 +- 8 files changed, 154 insertions(+), 26 deletions(-) diff --git a/pyproject.toml b/pyproject.toml index 23af4bf..eb77866 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [project] name = "sap-cloud-sdk" -version = "0.20.1" +version = "0.20.2" description = "SAP Cloud SDK for Python" readme = "README.md" license = "Apache-2.0" diff --git a/src/sap_cloud_sdk/core/auditlog_ng/__init__.py b/src/sap_cloud_sdk/core/auditlog_ng/__init__.py index 0fdfe7b..776e473 100644 --- a/src/sap_cloud_sdk/core/auditlog_ng/__init__.py +++ b/src/sap_cloud_sdk/core/auditlog_ng/__init__.py @@ -36,10 +36,9 @@ ValidationError, ) -from sap_cloud_sdk.core.telemetry import Module, Operation, record_metrics +from sap_cloud_sdk.core.telemetry import Module, Operation, record_error_metric -@record_metrics(Module.AUDITLOG_NG, Operation.AUDITLOG_CREATE_CLIENT) def create_client( *, config: Optional[AuditLogNGConfig] = None, @@ -86,24 +85,39 @@ def create_client( """ try: if config is None: - if not endpoint or not deployment_id or not namespace: - raise ValueError( - "endpoint, deployment_id, and namespace are required " - "when config is not provided" + try: + if not endpoint or not deployment_id or not namespace: + raise ValueError( + "endpoint, deployment_id, and namespace are required " + "when config is not provided" + ) + config = AuditLogNGConfig( + endpoint=endpoint, + deployment_id=deployment_id, + namespace=namespace, + cert_file=cert_file, + key_file=key_file, + ca_file=ca_file, + insecure=insecure, + service_name=service_name, + batch=batch, + compression=compression, + schema_url=schema_url, ) - config = AuditLogNGConfig( - endpoint=endpoint, - deployment_id=deployment_id, - namespace=namespace, - cert_file=cert_file, - key_file=key_file, - ca_file=ca_file, - insecure=insecure, - service_name=service_name, - batch=batch, - compression=compression, - schema_url=schema_url, - ) + except (ValueError, ValidationError): + record_error_metric( + Module.AUDITLOG_NG, + _telemetry_source, + Operation.AUDITLOG_CREATE_CLIENT, + ) + raise + except Exception: + record_error_metric( + Module.AUDITLOG_NG, + _telemetry_source, + Operation.AUDITLOG_CREATE_CLIENT, + ) + raise return AuditClient(config, _telemetry_source=_telemetry_source) diff --git a/src/sap_cloud_sdk/core/auditlog_ng/client.py b/src/sap_cloud_sdk/core/auditlog_ng/client.py index cea6054..53a30ed 100644 --- a/src/sap_cloud_sdk/core/auditlog_ng/client.py +++ b/src/sap_cloud_sdk/core/auditlog_ng/client.py @@ -35,7 +35,7 @@ _validate_source_arg, ) from sap_cloud_sdk.core.auditlog_ng.exceptions import ValidationError -from sap_cloud_sdk.core.telemetry import Module +from sap_cloud_sdk.core.telemetry import Module, Operation, record_metrics from sap_cloud_sdk.core.telemetry.config import ENV_OTLP_PROTOCOL @@ -102,6 +102,7 @@ class AuditClient: client.close() """ + @record_metrics(Module.AUDITLOG_NG, Operation.AUDITLOG_CREATE_CLIENT) def __init__( self, config: AuditLogNGConfig, _telemetry_source: Optional[Module] = None ) -> None: diff --git a/src/sap_cloud_sdk/core/telemetry/metrics_decorator.py b/src/sap_cloud_sdk/core/telemetry/metrics_decorator.py index b9d72a1..810db63 100644 --- a/src/sap_cloud_sdk/core/telemetry/metrics_decorator.py +++ b/src/sap_cloud_sdk/core/telemetry/metrics_decorator.py @@ -23,7 +23,8 @@ def record_metrics( instrumentation and should not be confused with general instrumentation or tracing. The decorator automatically detects the source of the call by checking the - `_telemetry_source` property on the client instance (self): + `_telemetry_source` property on the client instance (self), or the + `_telemetry_source` keyword argument before constructors assign it: - If the client has `_telemetry_source` set, it means it was created by another SDK module (e.g., objectstore → auditlog), and that module becomes the source - If `_telemetry_source` is None or not present, the call is from user code @@ -62,9 +63,14 @@ def decorator(func: Callable[P, R]) -> Callable[P, R]: @wraps(func) def wrapper(*args: P.args, **kwargs: P.kwargs) -> R: # Extract source from the client instance (self is the first argument) + # or from constructor kwargs before self._telemetry_source exists. source: Optional[Module] = None if args: source = getattr(args[0], "_telemetry_source", None) + if source is None: + source_kwarg = kwargs.get("_telemetry_source") + if isinstance(source_kwarg, Module): + source = source_kwarg try: result = func(*args, **kwargs) diff --git a/tests/core/unit/auditlog_ng/unit/test_client.py b/tests/core/unit/auditlog_ng/unit/test_client.py index 8707184..92b93c5 100644 --- a/tests/core/unit/auditlog_ng/unit/test_client.py +++ b/tests/core/unit/auditlog_ng/unit/test_client.py @@ -2,7 +2,6 @@ from __future__ import annotations -import json from typing import TypedDict, Unpack from unittest.mock import MagicMock, Mock, patch @@ -11,6 +10,7 @@ from sap_cloud_sdk.core.auditlog_ng.client import AuditClient 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.telemetry import Module, Operation class ConfigKwargs(TypedDict, total=False): @@ -34,7 +34,7 @@ def _make_config(**overrides: Unpack[ConfigKwargs]) -> AuditLogNGConfig: "namespace": "namespace-123", "insecure": True, } - defaults.update(overrides) # ty: ignore[invalid-argument-type] + defaults.update(overrides) return AuditLogNGConfig(**defaults) @@ -114,6 +114,46 @@ def test_sets_resource_attributes(self, mock_provider_cls, mock_exporter_cls): assert attrs["sap.ucl.deployment_id"] == "deployment-123" assert attrs["sap.ucl.system_namespace"] == "namespace-123" + @patch("sap_cloud_sdk.core.auditlog_ng.client.GRPCLogExporter") + @patch("sap_cloud_sdk.core.auditlog_ng.client.LoggerProvider") + def test_init_records_create_client_metric( + self, mock_provider_cls, mock_exporter_cls + ): + config = _make_config() + + with patch( + "sap_cloud_sdk.core.telemetry.metrics_decorator.record_request_metric" + ) as mock_metric: + AuditClient(config) + + mock_metric.assert_called_once_with( + Module.AUDITLOG_NG, + None, + Operation.AUDITLOG_CREATE_CLIENT, + False, + ) + + @patch("sap_cloud_sdk.core.auditlog_ng.client.GRPCLogExporter") + @patch("sap_cloud_sdk.core.auditlog_ng.client.LoggerProvider") + def test_init_records_error_metric_on_failure( + self, mock_provider_cls, mock_exporter_cls + ): + mock_provider_cls.side_effect = RuntimeError("provider failed") + config = _make_config() + + with patch( + "sap_cloud_sdk.core.telemetry.metrics_decorator.record_error_metric" + ) as mock_error_metric: + with pytest.raises(RuntimeError, match="provider failed"): + AuditClient(config, _telemetry_source=Module.DMS) + + mock_error_metric.assert_called_once_with( + Module.AUDITLOG_NG, + Module.DMS, + Operation.AUDITLOG_CREATE_CLIENT, + False, + ) + class TestAuditClientSend: def test_send_binary_success(self): diff --git a/tests/core/unit/auditlog_ng/unit/test_create_client.py b/tests/core/unit/auditlog_ng/unit/test_create_client.py index f59aa41..cfacabc 100644 --- a/tests/core/unit/auditlog_ng/unit/test_create_client.py +++ b/tests/core/unit/auditlog_ng/unit/test_create_client.py @@ -4,8 +4,9 @@ from unittest.mock import patch, Mock from sap_cloud_sdk.core.auditlog_ng import create_client, AuditClient -from sap_cloud_sdk.core.auditlog_ng.config import AuditLogNGConfig, SCHEMA_URL +from sap_cloud_sdk.core.auditlog_ng.config import AuditLogNGConfig from sap_cloud_sdk.core.auditlog_ng.exceptions import ClientCreationError +from sap_cloud_sdk.core.telemetry import Module, Operation class TestCreateClient: @@ -48,6 +49,26 @@ def test_create_client_missing_endpoint_raises(self): with pytest.raises(ValueError, match="endpoint, deployment_id, and namespace are required"): create_client(deployment_id="dep-1", namespace="ns-1") + def test_create_client_missing_endpoint_records_error_metric(self): + with patch( + "sap_cloud_sdk.core.auditlog_ng.record_error_metric" + ) as mock_error_metric: + with pytest.raises( + ValueError, + match="endpoint, deployment_id, and namespace are required", + ): + create_client( + deployment_id="dep-1", + namespace="ns-1", + _telemetry_source=Module.DMS, + ) + + mock_error_metric.assert_called_once_with( + Module.AUDITLOG_NG, + Module.DMS, + Operation.AUDITLOG_CREATE_CLIENT, + ) + def test_create_client_missing_deployment_id_raises(self): with pytest.raises(ValueError, match="endpoint, deployment_id, and namespace are required"): create_client(endpoint="localhost:4317", namespace="ns-1") @@ -103,3 +124,31 @@ def test_config_keyword_args_are_forwarded(self, mock_provider_cls, mock_exporte assert client._config.service_name == "my-svc" assert client._config.batch is True assert client._config.compression is False + + @patch("sap_cloud_sdk.core.auditlog_ng.client._create_log_exporter") + @patch("sap_cloud_sdk.core.auditlog_ng.client.LoggerProvider") + def test_create_client_records_metric_once_with_source( + self, mock_provider_cls, mock_exporter_fn + ): + mock_provider = Mock() + mock_provider.get_logger.return_value = Mock() + mock_provider_cls.return_value = mock_provider + + config = AuditLogNGConfig( + endpoint="localhost:4317", + deployment_id="dep-1", + namespace="ns-1", + insecure=True, + ) + + with patch( + "sap_cloud_sdk.core.telemetry.metrics_decorator.record_request_metric" + ) as mock_metric: + create_client(config=config, _telemetry_source=Module.DMS) + + mock_metric.assert_called_once_with( + Module.AUDITLOG_NG, + Module.DMS, + Operation.AUDITLOG_CREATE_CLIENT, + False, + ) diff --git a/tests/core/unit/telemetry/test_metrics_decorator.py b/tests/core/unit/telemetry/test_metrics_decorator.py index 1928669..968f906 100644 --- a/tests/core/unit/telemetry/test_metrics_decorator.py +++ b/tests/core/unit/telemetry/test_metrics_decorator.py @@ -99,6 +99,24 @@ def test_method(self): False # deprecated parameter ) + def test_decorator_reads_source_from_kwargs_when_self_source_missing(self): + """Test source detection before constructors assign _telemetry_source.""" + + class TestClient: + @record_metrics(Module.AUDITLOG_NG, Operation.AUDITLOG_CREATE_CLIENT) + def __init__(self, _telemetry_source=None): + self._telemetry_source = _telemetry_source + + with patch('sap_cloud_sdk.core.telemetry.metrics_decorator.record_request_metric') as mock_metric: + TestClient(_telemetry_source=Module.DMS) + + mock_metric.assert_called_once_with( + Module.AUDITLOG_NG, + Module.DMS, + Operation.AUDITLOG_CREATE_CLIENT, + False # deprecated parameter + ) + def test_decorator_records_error_metric_on_exception(self): """Test that decorator records error metric when function raises exception.""" diff --git a/uv.lock b/uv.lock index 0e5cebd..4b3a819 100644 --- a/uv.lock +++ b/uv.lock @@ -2924,7 +2924,7 @@ wheels = [ [[package]] name = "sap-cloud-sdk" -version = "0.19.3" +version = "0.20.2" source = { editable = "." } dependencies = [ { name = "grpcio" }, From 1783bb58bb685fa3f1c33c5fd47fbce45968960c Mon Sep 17 00:00:00 2001 From: Tiago Kochenborger Date: Tue, 26 May 2026 16:27:30 -0300 Subject: [PATCH 2/3] test: cover auditlog ng factory error telemetry --- CHANGELOG.md | 10 ++++ .../core/auditlog_ng/__init__.py | 7 --- .../auditlog_ng/unit/test_create_client.py | 53 +++++++++++++++---- 3 files changed, 53 insertions(+), 17 deletions(-) create mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md new file mode 100644 index 0000000..71da524 --- /dev/null +++ b/CHANGELOG.md @@ -0,0 +1,10 @@ +# Changelog + +## [0.20.2] - 2026-05-26 + +### Fixed + +- Record ALS v3 (`auditlog_ng`) capability telemetry when `AuditClient` is + initialized, including direct `AuditClient(config)` construction. The + `send()` and `send_json()` APIs remain uninstrumented so the metric continues + to represent client initialization rather than audit event volume. diff --git a/src/sap_cloud_sdk/core/auditlog_ng/__init__.py b/src/sap_cloud_sdk/core/auditlog_ng/__init__.py index 776e473..5332f36 100644 --- a/src/sap_cloud_sdk/core/auditlog_ng/__init__.py +++ b/src/sap_cloud_sdk/core/auditlog_ng/__init__.py @@ -104,13 +104,6 @@ def create_client( compression=compression, schema_url=schema_url, ) - except (ValueError, ValidationError): - record_error_metric( - Module.AUDITLOG_NG, - _telemetry_source, - Operation.AUDITLOG_CREATE_CLIENT, - ) - raise except Exception: record_error_metric( Module.AUDITLOG_NG, diff --git a/tests/core/unit/auditlog_ng/unit/test_create_client.py b/tests/core/unit/auditlog_ng/unit/test_create_client.py index cfacabc..2180ebc 100644 --- a/tests/core/unit/auditlog_ng/unit/test_create_client.py +++ b/tests/core/unit/auditlog_ng/unit/test_create_client.py @@ -49,18 +49,34 @@ def test_create_client_missing_endpoint_raises(self): with pytest.raises(ValueError, match="endpoint, deployment_id, and namespace are required"): create_client(deployment_id="dep-1", namespace="ns-1") - def test_create_client_missing_endpoint_records_error_metric(self): + @pytest.mark.parametrize( + ("kwargs", "match"), + [ + ( + {"deployment_id": "dep-1", "namespace": "ns-1"}, + "endpoint, deployment_id, and namespace are required", + ), + ( + { + "endpoint": "localhost:4317", + "deployment_id": "bad value", + "namespace": "ns-1", + }, + "deployment_id", + ), + ], + ) + def test_create_client_config_errors_record_error_metric(self, kwargs, match): with patch( "sap_cloud_sdk.core.auditlog_ng.record_error_metric" ) as mock_error_metric: with pytest.raises( ValueError, - match="endpoint, deployment_id, and namespace are required", + match=match, ): create_client( - deployment_id="dep-1", - namespace="ns-1", _telemetry_source=Module.DMS, + **kwargs, ) mock_error_metric.assert_called_once_with( @@ -96,13 +112,30 @@ def test_create_client_unexpected_exception_wraps_in_client_creation_error( ): mock_provider_cls.side_effect = RuntimeError("Unexpected failure") - with pytest.raises(ClientCreationError, match="Failed to create audit log NG client"): - create_client( - endpoint="localhost:4317", - deployment_id="dep-1", - namespace="ns-1", - insecure=True, + with patch( + "sap_cloud_sdk.core.telemetry.metrics_decorator.record_error_metric" + ) as mock_error_metric: + with patch( + "sap_cloud_sdk.core.telemetry.metrics_decorator.record_request_metric" + ) as mock_request_metric: + with pytest.raises( + ClientCreationError, match="Failed to create audit log NG client" + ): + create_client( + endpoint="localhost:4317", + deployment_id="dep-1", + namespace="ns-1", + insecure=True, + _telemetry_source=Module.DMS, + ) + + mock_error_metric.assert_called_once_with( + Module.AUDITLOG_NG, + Module.DMS, + Operation.AUDITLOG_CREATE_CLIENT, + False, ) + mock_request_metric.assert_not_called() @patch("sap_cloud_sdk.core.auditlog_ng.client._create_log_exporter") @patch("sap_cloud_sdk.core.auditlog_ng.client.LoggerProvider") From 1c5504cefd75b5d4de5dd0e18cc4dc785f0acdd4 Mon Sep 17 00:00:00 2001 From: Tiago Kochenborger Date: Tue, 26 May 2026 23:16:03 -0300 Subject: [PATCH 3/3] chore: address auditlog ng telemetry review comments --- CHANGELOG.md | 10 ---------- src/sap_cloud_sdk/core/auditlog_ng/__init__.py | 8 ++++++-- tests/core/unit/auditlog_ng/unit/test_create_client.py | 2 +- 3 files changed, 7 insertions(+), 13 deletions(-) delete mode 100644 CHANGELOG.md diff --git a/CHANGELOG.md b/CHANGELOG.md deleted file mode 100644 index 71da524..0000000 --- a/CHANGELOG.md +++ /dev/null @@ -1,10 +0,0 @@ -# Changelog - -## [0.20.2] - 2026-05-26 - -### Fixed - -- Record ALS v3 (`auditlog_ng`) capability telemetry when `AuditClient` is - initialized, including direct `AuditClient(config)` construction. The - `send()` and `send_json()` APIs remain uninstrumented so the metric continues - to represent client initialization rather than audit event volume. diff --git a/src/sap_cloud_sdk/core/auditlog_ng/__init__.py b/src/sap_cloud_sdk/core/auditlog_ng/__init__.py index 5332f36..321833d 100644 --- a/src/sap_cloud_sdk/core/auditlog_ng/__init__.py +++ b/src/sap_cloud_sdk/core/auditlog_ng/__init__.py @@ -36,7 +36,11 @@ ValidationError, ) -from sap_cloud_sdk.core.telemetry import Module, Operation, record_error_metric +from sap_cloud_sdk.core.telemetry import ( + Module, + Operation, + record_error_metric as _record_error_metric, +) def create_client( @@ -105,7 +109,7 @@ def create_client( schema_url=schema_url, ) except Exception: - record_error_metric( + _record_error_metric( Module.AUDITLOG_NG, _telemetry_source, Operation.AUDITLOG_CREATE_CLIENT, diff --git a/tests/core/unit/auditlog_ng/unit/test_create_client.py b/tests/core/unit/auditlog_ng/unit/test_create_client.py index 2180ebc..9da5b80 100644 --- a/tests/core/unit/auditlog_ng/unit/test_create_client.py +++ b/tests/core/unit/auditlog_ng/unit/test_create_client.py @@ -68,7 +68,7 @@ def test_create_client_missing_endpoint_raises(self): ) def test_create_client_config_errors_record_error_metric(self, kwargs, match): with patch( - "sap_cloud_sdk.core.auditlog_ng.record_error_metric" + "sap_cloud_sdk.core.auditlog_ng._record_error_metric" ) as mock_error_metric: with pytest.raises( ValueError,