Skip to content

Commit b4fbc56

Browse files
committed
Merge origin/master (aws#6286) into fix/codestyle-doc-tests-ci
Resolve common.py conflict in favour of master's telemetry emission, black-format the new telemetry test, and drop a duplicate pytest import in test_common.py that aws#6286 introduced. --- X-AI-Prompt: fix failing codestyle-doc-tests reruns on PR aws#6294 X-AI-Tool: Kiro
2 parents 4f1aad2 + 2c9fd13 commit b4fbc56

4 files changed

Lines changed: 227 additions & 5 deletions

File tree

‎sagemaker-core/src/sagemaker/core/telemetry/telemetry_logging.py‎

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -469,6 +469,65 @@ def _run():
469469
return thread
470470

471471

472+
def _emit_failure_telemetry(
473+
feature: str,
474+
func_name: str,
475+
exc: Exception,
476+
sagemaker_session: Session = None,
477+
) -> None:
478+
"""Emit a single FAILURE telemetry event for a client-side failure.
479+
480+
Unlike the ``@_telemetry_emitter`` decorator -- which wraps a call and emits on
481+
both success and failure for every invocation -- this helper emits only when a
482+
caller has explicitly hit a failure it wants recorded. Use it to capture a class
483+
of client-side failure (e.g. invalid user input) without adding any happy-path
484+
telemetry or per-call overhead to the surrounding code.
485+
486+
Best-effort: it resolves a session (falling back to the default), honors the
487+
telemetry opt-out configuration, and swallows any error while emitting, so it can
488+
never mask or replace the caller's own exception.
489+
490+
Args:
491+
feature: The Feature enum value to attribute this event to.
492+
func_name: Human-readable name of the failing operation, for tracking.
493+
exc: The exception representing the failure (used for reason/type/category).
494+
sagemaker_session: Optional session; the default session is used if omitted.
495+
"""
496+
try:
497+
session = sagemaker_session or _get_default_sagemaker_session()
498+
if not session:
499+
return
500+
# Honor the same telemetry opt-out contract as @_telemetry_emitter: a user
501+
# who has opted out must not have these events emitted.
502+
if resolve_value_from_config(
503+
direct_input=None,
504+
config_path=TELEMETRY_OPT_OUT_PATH,
505+
default_value=False,
506+
sagemaker_session=session,
507+
):
508+
return
509+
# Mirror the decorator's platform/env dimensions so these events can be
510+
# sliced consistently alongside decorator-emitted ones.
511+
extra = (
512+
f"{func_name}"
513+
f"&x-sdkVersion={SDK_VERSION}"
514+
f"&x-env={PYTHON_VERSION}"
515+
f"&x-sys={OS_NAME_VERSION}"
516+
f"&x-platform={process_studio_metadata_file()}"
517+
f"&x-errorCategory={_classify_error(exc)}"
518+
)
519+
_send_telemetry_request(
520+
STATUS_TO_CODE[str(Status.FAILURE)],
521+
[FEATURE_TO_CODE[str(feature)]],
522+
session,
523+
str(exc),
524+
exc.__class__.__name__,
525+
extra,
526+
)
527+
except Exception: # pragma: no cover - telemetry must never break the caller
528+
pass
529+
530+
472531
def _send_telemetry_request_sync(
473532
status: int,
474533
feature_list: List[int],

‎sagemaker-core/tests/unit/telemetry/test_telemetry_logging.py‎

Lines changed: 81 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,19 @@
2424
from sagemaker.core.telemetry.telemetry_logging import (
2525
_send_telemetry_request,
2626
_send_telemetry_request_sync,
27+
_emit_failure_telemetry,
2728
_telemetry_emitter,
2829
_construct_url,
2930
_get_accountId,
3031
_requests_helper,
3132
_get_region_or_default,
3233
_get_default_sagemaker_session,
34+
STATUS_TO_CODE,
3335
OS_NAME_VERSION,
3436
PYTHON_VERSION,
3537
TELEMETRY_REQUEST_TIMEOUT,
3638
)
39+
from sagemaker.core.telemetry.constants import Status
3740
from sagemaker.core.user_agent import SDK_VERSION, process_studio_metadata_file
3841

3942
# Try to import sagemaker-serve exceptions, skip tests if not available
@@ -889,3 +892,81 @@ def test_falls_back_to_default_region_when_none_resolved(self, mock_boto_session
889892
mock_boto_session.call_args_list[-1],
890893
unittest.mock.call(region_name=DEFAULT_AWS_REGION),
891894
)
895+
896+
897+
class TestEmitFailureTelemetry(unittest.TestCase):
898+
"""Tests for the failure-only _emit_failure_telemetry helper."""
899+
900+
@patch(
901+
"sagemaker.core.telemetry.telemetry_logging.resolve_value_from_config", return_value=False
902+
)
903+
@patch("sagemaker.core.telemetry.telemetry_logging._get_default_sagemaker_session")
904+
@patch("sagemaker.core.telemetry.telemetry_logging._send_telemetry_request")
905+
def test_emits_failure_event(self, mock_send, mock_default_session, mock_optout):
906+
mock_default_session.return_value = Mock()
907+
exc = ValueError("bad value")
908+
909+
_emit_failure_telemetry(Feature.MODEL_CUSTOMIZATION, "MyClass.method", exc)
910+
911+
mock_send.assert_called_once()
912+
args = mock_send.call_args.args
913+
assert args[0] == STATUS_TO_CODE[str(Status.FAILURE)] # status code
914+
assert args[3] == "bad value" # failure_reason
915+
assert args[4] == "ValueError" # failure_type
916+
assert "MyClass.method" in args[5] # extra_info carries func_name
917+
918+
@patch(
919+
"sagemaker.core.telemetry.telemetry_logging.resolve_value_from_config", return_value=True
920+
)
921+
@patch("sagemaker.core.telemetry.telemetry_logging._get_default_sagemaker_session")
922+
@patch("sagemaker.core.telemetry.telemetry_logging._send_telemetry_request")
923+
def test_opt_out_suppresses_emit(self, mock_send, mock_default_session, mock_optout):
924+
# A user who has opted out of telemetry must not have these events emitted.
925+
mock_default_session.return_value = Mock()
926+
927+
_emit_failure_telemetry(Feature.MODEL_CUSTOMIZATION, "MyClass.method", ValueError("x"))
928+
929+
mock_send.assert_not_called()
930+
931+
@patch(
932+
"sagemaker.core.telemetry.telemetry_logging.resolve_value_from_config", return_value=False
933+
)
934+
@patch("sagemaker.core.telemetry.telemetry_logging._get_default_sagemaker_session")
935+
@patch("sagemaker.core.telemetry.telemetry_logging._send_telemetry_request")
936+
def test_no_session_does_not_emit(self, mock_send, mock_default_session, mock_optout):
937+
mock_default_session.return_value = None
938+
939+
_emit_failure_telemetry(Feature.MODEL_CUSTOMIZATION, "MyClass.method", ValueError("x"))
940+
941+
mock_send.assert_not_called()
942+
943+
@patch(
944+
"sagemaker.core.telemetry.telemetry_logging.resolve_value_from_config", return_value=False
945+
)
946+
@patch("sagemaker.core.telemetry.telemetry_logging._get_default_sagemaker_session")
947+
@patch("sagemaker.core.telemetry.telemetry_logging._send_telemetry_request")
948+
def test_uses_provided_session_without_default_lookup(
949+
self, mock_send, mock_default_session, mock_optout
950+
):
951+
_emit_failure_telemetry(
952+
Feature.MODEL_CUSTOMIZATION,
953+
"MyClass.method",
954+
ValueError("x"),
955+
sagemaker_session=Mock(),
956+
)
957+
958+
mock_default_session.assert_not_called()
959+
mock_send.assert_called_once()
960+
961+
@patch(
962+
"sagemaker.core.telemetry.telemetry_logging.resolve_value_from_config", return_value=False
963+
)
964+
@patch("sagemaker.core.telemetry.telemetry_logging._get_default_sagemaker_session")
965+
@patch("sagemaker.core.telemetry.telemetry_logging._send_telemetry_request")
966+
def test_backend_error_is_swallowed(self, mock_send, mock_default_session, mock_optout):
967+
# Telemetry is best-effort: an error while emitting must never propagate.
968+
mock_default_session.return_value = Mock()
969+
mock_send.side_effect = RuntimeError("telemetry backend down")
970+
971+
# Should not raise.
972+
_emit_failure_telemetry(Feature.MODEL_CUSTOMIZATION, "MyClass.method", ValueError("x"))

‎sagemaker-train/src/sagemaker/train/common.py‎

Lines changed: 18 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,10 @@
22

33
from typing import Dict, Any
44
from enum import Enum
5-
from sagemaker.core.telemetry.telemetry_logging import _telemetry_emitter
5+
from sagemaker.core.telemetry.telemetry_logging import (
6+
_telemetry_emitter,
7+
_emit_failure_telemetry,
8+
)
69
from sagemaker.core.telemetry.constants import Feature
710

811
JOB_TYPE = "FineTuning"
@@ -101,13 +104,24 @@ def __setattr__(self, name: str, value: Any):
101104
if getattr(self, "_initialized", False):
102105
spec = self._specs[name]
103106
if isinstance(spec, dict):
104-
self._validate_value(name, value, spec)
107+
try:
108+
self._validate_value(name, value, spec)
109+
except Exception as exc:
110+
_emit_failure_telemetry(
111+
Feature.MODEL_CUSTOMIZATION, "FineTuningOptions.__setattr__", exc
112+
)
113+
raise
105114
self._user_set.add(name)
106115
super().__setattr__(name, value)
107116
elif hasattr(self, "_specs"):
108-
raise AttributeError(
109-
f"'{name}' is not a valid fine-tuning option. Valid options: {list(self._specs.keys())}"
117+
exc = AttributeError(
118+
f"'{name}' is not a valid fine-tuning option. "
119+
f"Valid options: {list(self._specs.keys())}"
110120
)
121+
_emit_failure_telemetry(
122+
Feature.MODEL_CUSTOMIZATION, "FineTuningOptions.__setattr__", exc
123+
)
124+
raise exc
111125
else:
112126
super().__setattr__(name, value)
113127

‎sagemaker-train/tests/unit/train/test_common.py‎

Lines changed: 69 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,7 +1,9 @@
1-
from sagemaker.train.common import FineTuningOptions
1+
from unittest.mock import patch
22

33
import pytest
44

5+
from sagemaker.train.common import FineTuningOptions
6+
57

68
class TestFineTuningOptionsToDict:
79
"""Tests for FineTuningOptions.to_dict() None value handling."""
@@ -138,3 +140,69 @@ def test_framework_agnostic_gated_on_sequence_length_metadata_only(self):
138140
)
139141
object.__setattr__(opts, "dataset_max_len", 999999)
140142
opts.validate_length_constraints() # unknown ceiling -> no raise
143+
144+
145+
class TestFineTuningOptionsValidationTelemetry:
146+
"""Failure-only telemetry emitted from FineTuningOptions.__setattr__.
147+
148+
A single FAILURE event (via the core _emit_failure_telemetry helper) is emitted
149+
when a caller sets an invalid option name or an out-of-spec value, and NOTHING is
150+
emitted on the happy path (valid sets, internal sets, or construction).
151+
"""
152+
153+
_SPECS = {
154+
"learning_rate": {"default": 1e-4, "type": "float", "min": 1e-7, "max": 1.0},
155+
"num_epochs": {"default": 3, "type": "integer", "min": 1, "max": 100},
156+
}
157+
158+
@patch("sagemaker.train.common._emit_failure_telemetry")
159+
def test_invalid_option_name_emits_failure(self, mock_emit):
160+
options = FineTuningOptions(self._SPECS)
161+
with pytest.raises(AttributeError):
162+
options.not_a_real_param = 5
163+
164+
mock_emit.assert_called_once()
165+
args = mock_emit.call_args[0]
166+
assert args[1] == "FineTuningOptions.__setattr__" # func_name
167+
assert isinstance(args[2], AttributeError) # exc
168+
169+
@patch("sagemaker.train.common._emit_failure_telemetry")
170+
def test_out_of_range_value_emits_failure(self, mock_emit):
171+
options = FineTuningOptions(self._SPECS)
172+
with pytest.raises(ValueError):
173+
options.learning_rate = 999.0 # exceeds max=1.0
174+
175+
mock_emit.assert_called_once()
176+
assert isinstance(mock_emit.call_args[0][2], ValueError)
177+
178+
@patch("sagemaker.train.common._emit_failure_telemetry")
179+
def test_wrong_type_value_emits_failure(self, mock_emit):
180+
options = FineTuningOptions(self._SPECS)
181+
with pytest.raises(ValueError):
182+
options.num_epochs = "three" # not an integer
183+
184+
mock_emit.assert_called_once()
185+
assert isinstance(mock_emit.call_args[0][2], ValueError)
186+
187+
@patch("sagemaker.train.common._emit_failure_telemetry")
188+
def test_valid_set_does_not_emit(self, mock_emit):
189+
options = FineTuningOptions(self._SPECS)
190+
options.learning_rate = 1e-3 # within range -> no telemetry
191+
assert options.learning_rate == 1e-3
192+
mock_emit.assert_not_called()
193+
194+
@patch("sagemaker.train.common._emit_failure_telemetry")
195+
def test_construction_does_not_emit(self, mock_emit):
196+
# __init__ sets defaults via super().__setattr__ and internal _-prefixed attrs,
197+
# none of which should emit telemetry.
198+
FineTuningOptions(self._SPECS)
199+
mock_emit.assert_not_called()
200+
201+
@patch("sagemaker.train.common._emit_failure_telemetry")
202+
def test_emits_model_customization_feature(self, mock_emit):
203+
from sagemaker.core.telemetry.constants import Feature
204+
205+
options = FineTuningOptions(self._SPECS)
206+
with pytest.raises(AttributeError):
207+
options.bogus = 1
208+
assert mock_emit.call_args[0][0] == Feature.MODEL_CUSTOMIZATION

0 commit comments

Comments
 (0)