diff --git a/sagemaker-core/src/sagemaker/core/telemetry/telemetry_logging.py b/sagemaker-core/src/sagemaker/core/telemetry/telemetry_logging.py index 49ba16c543..0e35a47b58 100644 --- a/sagemaker-core/src/sagemaker/core/telemetry/telemetry_logging.py +++ b/sagemaker-core/src/sagemaker/core/telemetry/telemetry_logging.py @@ -471,6 +471,65 @@ def _run(): return thread +def _emit_failure_telemetry( + feature: str, + func_name: str, + exc: Exception, + sagemaker_session: Session = None, +) -> None: + """Emit a single FAILURE telemetry event for a client-side failure. + + Unlike the ``@_telemetry_emitter`` decorator -- which wraps a call and emits on + both success and failure for every invocation -- this helper emits only when a + caller has explicitly hit a failure it wants recorded. Use it to capture a class + of client-side failure (e.g. invalid user input) without adding any happy-path + telemetry or per-call overhead to the surrounding code. + + Best-effort: it resolves a session (falling back to the default), honors the + telemetry opt-out configuration, and swallows any error while emitting, so it can + never mask or replace the caller's own exception. + + Args: + feature: The Feature enum value to attribute this event to. + func_name: Human-readable name of the failing operation, for tracking. + exc: The exception representing the failure (used for reason/type/category). + sagemaker_session: Optional session; the default session is used if omitted. + """ + try: + session = sagemaker_session or _get_default_sagemaker_session() + if not session: + return + # Honor the same telemetry opt-out contract as @_telemetry_emitter: a user + # who has opted out must not have these events emitted. + if resolve_value_from_config( + direct_input=None, + config_path=TELEMETRY_OPT_OUT_PATH, + default_value=False, + sagemaker_session=session, + ): + return + # Mirror the decorator's platform/env dimensions so these events can be + # sliced consistently alongside decorator-emitted ones. + extra = ( + f"{func_name}" + f"&x-sdkVersion={SDK_VERSION}" + f"&x-env={PYTHON_VERSION}" + f"&x-sys={OS_NAME_VERSION}" + f"&x-platform={process_studio_metadata_file()}" + f"&x-errorCategory={_classify_error(exc)}" + ) + _send_telemetry_request( + STATUS_TO_CODE[str(Status.FAILURE)], + [FEATURE_TO_CODE[str(feature)]], + session, + str(exc), + exc.__class__.__name__, + extra, + ) + except Exception: # pragma: no cover - telemetry must never break the caller + pass + + def _send_telemetry_request_sync( status: int, feature_list: List[int], diff --git a/sagemaker-core/tests/unit/telemetry/test_telemetry_logging.py b/sagemaker-core/tests/unit/telemetry/test_telemetry_logging.py index e1e55d4241..080a64b162 100644 --- a/sagemaker-core/tests/unit/telemetry/test_telemetry_logging.py +++ b/sagemaker-core/tests/unit/telemetry/test_telemetry_logging.py @@ -25,16 +25,19 @@ from sagemaker.core.telemetry.telemetry_logging import ( _send_telemetry_request, _send_telemetry_request_sync, + _emit_failure_telemetry, _telemetry_emitter, _construct_url, _get_accountId, _requests_helper, _get_region_or_default, _get_default_sagemaker_session, + STATUS_TO_CODE, OS_NAME_VERSION, PYTHON_VERSION, TELEMETRY_REQUEST_TIMEOUT, ) +from sagemaker.core.telemetry.constants import Status from sagemaker.core.user_agent import SDK_VERSION, process_studio_metadata_file # Try to import sagemaker-serve exceptions, skip tests if not available @@ -885,3 +888,69 @@ def test_falls_back_to_default_region_when_none_resolved(self, mock_boto_session mock_boto_session.call_args_list[-1], unittest.mock.call(region_name=DEFAULT_AWS_REGION), ) + + +class TestEmitFailureTelemetry(unittest.TestCase): + """Tests for the failure-only _emit_failure_telemetry helper.""" + + @patch("sagemaker.core.telemetry.telemetry_logging.resolve_value_from_config", return_value=False) + @patch("sagemaker.core.telemetry.telemetry_logging._get_default_sagemaker_session") + @patch("sagemaker.core.telemetry.telemetry_logging._send_telemetry_request") + def test_emits_failure_event(self, mock_send, mock_default_session, mock_optout): + mock_default_session.return_value = Mock() + exc = ValueError("bad value") + + _emit_failure_telemetry(Feature.MODEL_CUSTOMIZATION, "MyClass.method", exc) + + mock_send.assert_called_once() + args = mock_send.call_args.args + assert args[0] == STATUS_TO_CODE[str(Status.FAILURE)] # status code + assert args[3] == "bad value" # failure_reason + assert args[4] == "ValueError" # failure_type + assert "MyClass.method" in args[5] # extra_info carries func_name + + @patch("sagemaker.core.telemetry.telemetry_logging.resolve_value_from_config", return_value=True) + @patch("sagemaker.core.telemetry.telemetry_logging._get_default_sagemaker_session") + @patch("sagemaker.core.telemetry.telemetry_logging._send_telemetry_request") + def test_opt_out_suppresses_emit(self, mock_send, mock_default_session, mock_optout): + # A user who has opted out of telemetry must not have these events emitted. + mock_default_session.return_value = Mock() + + _emit_failure_telemetry(Feature.MODEL_CUSTOMIZATION, "MyClass.method", ValueError("x")) + + mock_send.assert_not_called() + + @patch("sagemaker.core.telemetry.telemetry_logging.resolve_value_from_config", return_value=False) + @patch("sagemaker.core.telemetry.telemetry_logging._get_default_sagemaker_session") + @patch("sagemaker.core.telemetry.telemetry_logging._send_telemetry_request") + def test_no_session_does_not_emit(self, mock_send, mock_default_session, mock_optout): + mock_default_session.return_value = None + + _emit_failure_telemetry(Feature.MODEL_CUSTOMIZATION, "MyClass.method", ValueError("x")) + + mock_send.assert_not_called() + + @patch("sagemaker.core.telemetry.telemetry_logging.resolve_value_from_config", return_value=False) + @patch("sagemaker.core.telemetry.telemetry_logging._get_default_sagemaker_session") + @patch("sagemaker.core.telemetry.telemetry_logging._send_telemetry_request") + def test_uses_provided_session_without_default_lookup( + self, mock_send, mock_default_session, mock_optout + ): + _emit_failure_telemetry( + Feature.MODEL_CUSTOMIZATION, "MyClass.method", ValueError("x"), + sagemaker_session=Mock(), + ) + + mock_default_session.assert_not_called() + mock_send.assert_called_once() + + @patch("sagemaker.core.telemetry.telemetry_logging.resolve_value_from_config", return_value=False) + @patch("sagemaker.core.telemetry.telemetry_logging._get_default_sagemaker_session") + @patch("sagemaker.core.telemetry.telemetry_logging._send_telemetry_request") + def test_backend_error_is_swallowed(self, mock_send, mock_default_session, mock_optout): + # Telemetry is best-effort: an error while emitting must never propagate. + mock_default_session.return_value = Mock() + mock_send.side_effect = RuntimeError("telemetry backend down") + + # Should not raise. + _emit_failure_telemetry(Feature.MODEL_CUSTOMIZATION, "MyClass.method", ValueError("x")) diff --git a/sagemaker-train/src/sagemaker/train/common.py b/sagemaker-train/src/sagemaker/train/common.py index be0c301e51..feb1fd6b06 100644 --- a/sagemaker-train/src/sagemaker/train/common.py +++ b/sagemaker-train/src/sagemaker/train/common.py @@ -1,6 +1,9 @@ from typing import Dict, Any from enum import Enum -from sagemaker.core.telemetry.telemetry_logging import _telemetry_emitter +from sagemaker.core.telemetry.telemetry_logging import ( + _telemetry_emitter, + _emit_failure_telemetry, +) from sagemaker.core.telemetry.constants import Feature JOB_TYPE = "FineTuning" @@ -93,11 +96,24 @@ def __setattr__(self, name: str, value: Any): if getattr(self, '_initialized', False): spec = self._specs[name] if isinstance(spec, dict): - self._validate_value(name, value, spec) + try: + self._validate_value(name, value, spec) + except Exception as exc: + _emit_failure_telemetry( + Feature.MODEL_CUSTOMIZATION, "FineTuningOptions.__setattr__", exc + ) + raise self._user_set.add(name) super().__setattr__(name, value) elif hasattr(self, '_specs'): - raise AttributeError(f"'{name}' is not a valid fine-tuning option. Valid options: {list(self._specs.keys())}") + exc = AttributeError( + f"'{name}' is not a valid fine-tuning option. " + f"Valid options: {list(self._specs.keys())}" + ) + _emit_failure_telemetry( + Feature.MODEL_CUSTOMIZATION, "FineTuningOptions.__setattr__", exc + ) + raise exc else: super().__setattr__(name, value) diff --git a/sagemaker-train/tests/unit/train/test_common.py b/sagemaker-train/tests/unit/train/test_common.py index 74230cad60..fc662d55a3 100644 --- a/sagemaker-train/tests/unit/train/test_common.py +++ b/sagemaker-train/tests/unit/train/test_common.py @@ -1,3 +1,7 @@ +from unittest.mock import patch + +import pytest + from sagemaker.train.common import FineTuningOptions @@ -129,3 +133,69 @@ def test_framework_agnostic_gated_on_sequence_length_metadata_only(self): ) object.__setattr__(opts, "dataset_max_len", 999999) opts.validate_length_constraints() # unknown ceiling -> no raise + + +class TestFineTuningOptionsValidationTelemetry: + """Failure-only telemetry emitted from FineTuningOptions.__setattr__. + + A single FAILURE event (via the core _emit_failure_telemetry helper) is emitted + when a caller sets an invalid option name or an out-of-spec value, and NOTHING is + emitted on the happy path (valid sets, internal sets, or construction). + """ + + _SPECS = { + "learning_rate": {"default": 1e-4, "type": "float", "min": 1e-7, "max": 1.0}, + "num_epochs": {"default": 3, "type": "integer", "min": 1, "max": 100}, + } + + @patch("sagemaker.train.common._emit_failure_telemetry") + def test_invalid_option_name_emits_failure(self, mock_emit): + options = FineTuningOptions(self._SPECS) + with pytest.raises(AttributeError): + options.not_a_real_param = 5 + + mock_emit.assert_called_once() + args = mock_emit.call_args[0] + assert args[1] == "FineTuningOptions.__setattr__" # func_name + assert isinstance(args[2], AttributeError) # exc + + @patch("sagemaker.train.common._emit_failure_telemetry") + def test_out_of_range_value_emits_failure(self, mock_emit): + options = FineTuningOptions(self._SPECS) + with pytest.raises(ValueError): + options.learning_rate = 999.0 # exceeds max=1.0 + + mock_emit.assert_called_once() + assert isinstance(mock_emit.call_args[0][2], ValueError) + + @patch("sagemaker.train.common._emit_failure_telemetry") + def test_wrong_type_value_emits_failure(self, mock_emit): + options = FineTuningOptions(self._SPECS) + with pytest.raises(ValueError): + options.num_epochs = "three" # not an integer + + mock_emit.assert_called_once() + assert isinstance(mock_emit.call_args[0][2], ValueError) + + @patch("sagemaker.train.common._emit_failure_telemetry") + def test_valid_set_does_not_emit(self, mock_emit): + options = FineTuningOptions(self._SPECS) + options.learning_rate = 1e-3 # within range -> no telemetry + assert options.learning_rate == 1e-3 + mock_emit.assert_not_called() + + @patch("sagemaker.train.common._emit_failure_telemetry") + def test_construction_does_not_emit(self, mock_emit): + # __init__ sets defaults via super().__setattr__ and internal _-prefixed attrs, + # none of which should emit telemetry. + FineTuningOptions(self._SPECS) + mock_emit.assert_not_called() + + @patch("sagemaker.train.common._emit_failure_telemetry") + def test_emits_model_customization_feature(self, mock_emit): + from sagemaker.core.telemetry.constants import Feature + + options = FineTuningOptions(self._SPECS) + with pytest.raises(AttributeError): + options.bogus = 1 + assert mock_emit.call_args[0][0] == Feature.MODEL_CUSTOMIZATION