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..321833d 100644 --- a/src/sap_cloud_sdk/core/auditlog_ng/__init__.py +++ b/src/sap_cloud_sdk/core/auditlog_ng/__init__.py @@ -36,10 +36,13 @@ ValidationError, ) -from sap_cloud_sdk.core.telemetry import Module, Operation, record_metrics +from sap_cloud_sdk.core.telemetry import ( + Module, + Operation, + record_error_metric as _record_error_metric, +) -@record_metrics(Module.AUDITLOG_NG, Operation.AUDITLOG_CREATE_CLIENT) def create_client( *, config: Optional[AuditLogNGConfig] = None, @@ -86,24 +89,32 @@ 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, + ) + except Exception: + _record_error_metric( + Module.AUDITLOG_NG, + _telemetry_source, + Operation.AUDITLOG_CREATE_CLIENT, ) - 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, - ) + 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..9da5b80 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,42 @@ 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") + @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=match, + ): + create_client( + _telemetry_source=Module.DMS, + **kwargs, + ) + + 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") @@ -75,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") @@ -103,3 +157,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" },