Add telemetry metrics on usage of default images for ModelBuilder#4430

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
akrishna1995 merged 6 commits into from
Feb 21, 2024
Merged

Add telemetry metrics on usage of default images for ModelBuilder #4430

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ce87772
Initial commit
jiapinw Feb 15, 2024
334ce76
Fix formatting and add uts
jiapinw Feb 16, 2024
f93594e
try to fix ut failure
jiapinw Feb 16, 2024
ce74dd1
fix missing attribute in telemetry ut
jiapinw Feb 16, 2024
aa21238
fix docstyle
jiapinw Feb 16, 2024
f0c3c64
fix pylint
jiapinw Feb 16, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions src/sagemaker/serve/builder/djl_builder.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -83,6 +83,7 @@ def __init__(self):
self.mode = None
self.model_server = None
self.image_uri = None
self._is_custom_image_uri = False
self.image_config = None
self.vpc_config = None
self._original_deploy = None
Expand Down
4 changes: 3 additions & 1 deletion src/sagemaker/serve/builder/jumpstart_builder.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -57,6 +57,8 @@ def __init__(self):
self.mode = None
self.model_server = None
self.image_uri = None
self._is_custom_image_uri = False
self.vpc_config = None

@grenmestergrenmesterFeb 16, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the added VPC config for?

@jiapinwjiapinwFeb 16, 2024

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's for feature parity. This config has already exposed to djl/tgi/torchserve/tri

self._original_deploy = None
self.secret_key = None
self.js_model_config = None
Expand DownExpand Up@@ -94,7 +96,7 @@ def _is_jumpstart_model_id(self) -> bool:

def _create_pre_trained_js_model(self) -> Type[Model]:
"""Placeholder docstring"""
pysdk_model = JumpStartModel(self.model)
pysdk_model = JumpStartModel(self.model, vpc_config=self.vpc_config)
pysdk_model.sagemaker_session = self.sagemaker_session

self._original_deploy = pysdk_model.deploy
Expand Down
2 changes: 2 additions & 0 deletions src/sagemaker/serve/builder/model_builder.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -605,6 +605,8 @@ def build(

self.serve_settings = self._get_serve_setting()

self._is_custom_image_uri = self.image_uri is None

if isinstance(self.model, str):
if self._is_jumpstart_model_id():
return self._build_for_jumpstart()
Expand Down
1 change: 1 addition & 0 deletions src/sagemaker/serve/builder/tgi_builder.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -76,6 +76,7 @@ def __init__(self):
self.mode = None
self.model_server = None
self.image_uri = None
self._is_custom_image_uri = False
self.image_config = None
self.vpc_config = None
self._original_deploy = None
Expand Down
4 changes: 4 additions & 0 deletions src/sagemaker/serve/builder/transformers_builder.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -56,6 +56,8 @@ def __init__(self):
self.mode = None
self.model_server = None
self.image_uri = None
self._is_custom_image_uri = False
self.vpc_config = None
self._original_deploy = None
self.hf_model_config = None
self._default_data_type = None
Expand DownExpand Up@@ -111,6 +113,7 @@ def _create_transformers_model(self) -> Type[Model]:
py_version=self.py_version,
transformers_version=base_hf_version,
pytorch_version=self.pytorch_version,
vpc_config=self.vpc_config,
)
elif "keras" in hf_model_md.get("tags") or "tensorflow" in hf_model_md.get("tags"):
self.tensorflow_version = self._get_supported_version(
Expand All@@ -126,6 +129,7 @@ def _create_transformers_model(self) -> Type[Model]:
py_version=self.py_version,
transformers_version=base_hf_version,
tensorflow_version=self.tensorflow_version,
vpc_config=self.vpc_config,
)

if self.mode == Mode.LOCAL_CONTAINER:
Expand Down
24 changes: 23 additions & 1 deletion src/sagemaker/serve/utils/telemetry_logger.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -20,7 +20,8 @@
from sagemaker import Session, exceptions
from sagemaker.serve.mode.function_pointers import Mode
from sagemaker.serve.utils.exceptions import ModelBuilderException
from sagemaker.serve.utils.types import ModelServer
from sagemaker.serve.utils.types import ModelServer, ImageUriOption
from sagemaker.serve.validations.check_image_uri import is_1p_image_uri
from sagemaker.user_agent import SDK_VERSION

logger = logging.getLogger(__name__)
Expand DownExpand Up@@ -62,11 +63,13 @@ def wrapper(self, *args, **kwargs):
caught_ex = None

image_uri_tail = self.image_uri.split("/")[1]
image_uri_option = _get_image_uri_option(self.image_uri, self._is_custom_image_uri)
extra = (
f"{func_name}"
f"&x-modelServer={MODEL_SERVER_TO_CODE[str(self.model_server)]}"
f"&x-imageTag={image_uri_tail}"
f"&x-sdkVersion={SDK_VERSION}"
f"&x-defaultImageUsage={image_uri_option}"

@gwang111gwang111Feb 19, 2024

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious, is "default" important to the naming here or can we just go with "imageUsage"

@gwang111gwang111Feb 19, 2024

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or "imageType"

@jiapinwjiapinwFeb 19, 2024

Copy link
Copy Markdown
ContributorAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, this metric is to capture whether customers provide their own image or not, the further differentiation on custom image or custom 1p image is not that important.

@gwang111gwang111Feb 19, 2024•
edited
Loading

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack sound good. posed the question from a string len perspective. 1024 is max i believe. So if there are ways to cut down on length without losing the meaningfulness we should try and do so

)

if self.model_server == ModelServer.DJL_SERVING or self.model_server == ModelServer.TGI:
Expand DownExpand Up@@ -201,3 +204,22 @@ def _get_region_or_default(session):
return session.boto_session.region_name
except Exception: # pylint: disable=W0703
return "us-west-2"


def _get_image_uri_option(image_uri: str, is_custom_image: bool) -> int:
"""Detect whether default values are used for ModelBuilder

Args:
image_uri (str): Image uri used by ModelBuilder.
is_custom_image: (bool): Boolean indicating whether customer provides with custom image.
Returns:
bool: Integer code of image option types.
"""

if not is_custom_image:
return ImageUriOption.DEFAULT_IMAGE.value

if is_1p_image_uri(image_uri):
return ImageUriOption.CUSTOM_1P_IMAGE.value

return ImageUriOption.CUSTOM_IMAGE.value
12 changes: 12 additions & 0 deletions src/sagemaker/serve/utils/types.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -43,3 +43,15 @@ def __str__(self) -> str:
INFERENTIA_1 = 3
INFERENTIA_2 = 4
GRAVITON = 5


class ImageUriOption(Enum):
"""Enum type for image uri options"""

def __str__(self) -> str:
"""Convert enum to string"""
return str(self.name)

CUSTOM_IMAGE = 1
CUSTOM_1P_IMAGE = 2
DEFAULT_IMAGE = 3
3 changes: 3 additions & 0 deletions tests/unit/sagemaker/serve/builder/test_transformers_builder.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -16,6 +16,7 @@
import unittest
from sagemaker.serve.builder.model_builder import ModelBuilder
from sagemaker.serve.mode.function_pointers import Mode
from tests.unit.sagemaker.serve.constants import MOCK_VPC_CONFIG

from sagemaker.serve.utils.predictors import TransformersLocalModePredictor

Expand DownExpand Up@@ -74,6 +75,7 @@ def test_build_deploy_for_transformers_local_container_and_remote_container(
model=mock_model_id,
schema_builder=mock_schema_builder,
mode=Mode.LOCAL_CONTAINER,
vpc_config=MOCK_VPC_CONFIG,
)

builder._prepare_for_mode = MagicMock()
Expand All@@ -85,6 +87,7 @@ def test_build_deploy_for_transformers_local_container_and_remote_container(
builder.modes[str(Mode.LOCAL_CONTAINER)] = MagicMock()
predictor = model.deploy(model_data_download_timeout=1800)

assert model.vpc_config == MOCK_VPC_CONFIG
assert builder.env_vars["MODEL_LOADING_TIMEOUT"] == "1800"
assert isinstance(predictor, TransformersLocalModePredictor)

Expand Down
38 changes: 38 additions & 0 deletions tests/unit/sagemaker/serve/utils/test_telemetry_logger.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -20,6 +20,7 @@
_construct_url,
)
from sagemaker.serve.utils.exceptions import ModelBuilderException, LocalModelOutOfMemoryException
from sagemaker.serve.utils.types import ImageUriOption
from sagemaker.user_agent import SDK_VERSION

MOCK_SESSION = Mock()
Expand DownExpand Up@@ -71,6 +72,7 @@ def test_capture_telemetry_decorator_djl_success(self, mock_send_telemetry):
mock_model_builder = ModelBuilderMock()
mock_model_builder.serve_settings.telemetry_opt_out = False
mock_model_builder.image_uri = MOCK_DJL_CONTAINER
mock_model_builder._is_custom_image_uri = False
mock_model_builder.model = MOCK_HUGGINGFACE_ID
mock_model_builder.mode = Mode.LOCAL_CONTAINER
mock_model_builder.model_server = ModelServer.DJL_SERVING
Expand All@@ -85,6 +87,37 @@ def test_capture_telemetry_decorator_djl_success(self, mock_send_telemetry):
"&x-modelServer=4"
"&x-imageTag=djl-inference:0.25.0-deepspeed0.11.0-cu118"
f"&x-sdkVersion={SDK_VERSION}"
f"&x-defaultImageUsage={ImageUriOption.DEFAULT_IMAGE.value}"
f"&x-modelName={MOCK_HUGGINGFACE_ID}"
f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
f"&x-latency={latency}"
)

mock_send_telemetry.assert_called_once_with(
"1", 2, MOCK_SESSION, None, None, expected_extra_str
)

@patch("sagemaker.serve.utils.telemetry_logger._send_telemetry")
def test_capture_telemetry_decorator_djl_success_with_custom_image(self, mock_send_telemetry):
mock_model_builder = ModelBuilderMock()
mock_model_builder.serve_settings.telemetry_opt_out = False
mock_model_builder.image_uri = MOCK_DJL_CONTAINER
mock_model_builder._is_custom_image_uri = True
mock_model_builder.model = MOCK_HUGGINGFACE_ID
mock_model_builder.mode = Mode.LOCAL_CONTAINER
mock_model_builder.model_server = ModelServer.DJL_SERVING
mock_model_builder.sagemaker_session.endpoint_arn = MOCK_ENDPOINT_ARN

mock_model_builder.mock_deploy()

args = mock_send_telemetry.call_args.args
latency = str(args[5]).split("latency=")[1]
expected_extra_str = (
f"{MOCK_FUNC_NAME}"
"&x-modelServer=4"
"&x-imageTag=djl-inference:0.25.0-deepspeed0.11.0-cu118"
f"&x-sdkVersion={SDK_VERSION}"
f"&x-defaultImageUsage={ImageUriOption.CUSTOM_1P_IMAGE.value}"
f"&x-modelName={MOCK_HUGGINGFACE_ID}"
f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
f"&x-latency={latency}"
Expand All@@ -99,6 +132,7 @@ def test_capture_telemetry_decorator_tgi_success(self, mock_send_telemetry):
mock_model_builder = ModelBuilderMock()
mock_model_builder.serve_settings.telemetry_opt_out = False
mock_model_builder.image_uri = MOCK_TGI_CONTAINER
mock_model_builder._is_custom_image_uri = False
mock_model_builder.model = MOCK_HUGGINGFACE_ID
mock_model_builder.mode = Mode.LOCAL_CONTAINER
mock_model_builder.model_server = ModelServer.TGI
Expand All@@ -113,6 +147,7 @@ def test_capture_telemetry_decorator_tgi_success(self, mock_send_telemetry):
"&x-modelServer=6"
"&x-imageTag=huggingface-pytorch-inference:2.0.0-transformers4.28.1-cpu-py310-ubuntu20.04"
f"&x-sdkVersion={SDK_VERSION}"
f"&x-defaultImageUsage={ImageUriOption.DEFAULT_IMAGE.value}"
f"&x-modelName={MOCK_HUGGINGFACE_ID}"
f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
f"&x-latency={latency}"
Expand All@@ -127,6 +162,7 @@ def test_capture_telemetry_decorator_no_call_when_disabled(self, mock_send_telem
mock_model_builder = ModelBuilderMock()
mock_model_builder.serve_settings.telemetry_opt_out = True
mock_model_builder.image_uri = MOCK_DJL_CONTAINER
mock_model_builder._is_custom_image_uri = False
mock_model_builder.model = MOCK_HUGGINGFACE_ID
mock_model_builder.model_server = ModelServer.DJL_SERVING

Expand All@@ -139,6 +175,7 @@ def test_capture_telemetry_decorator_handle_exception_success(self, mock_send_te
mock_model_builder = ModelBuilderMock()
mock_model_builder.serve_settings.telemetry_opt_out = False
mock_model_builder.image_uri = MOCK_DJL_CONTAINER
mock_model_builder._is_custom_image_uri = False
mock_model_builder.model = MOCK_HUGGINGFACE_ID
mock_model_builder.mode = Mode.LOCAL_CONTAINER
mock_model_builder.model_server = ModelServer.DJL_SERVING
Expand All@@ -158,6 +195,7 @@ def test_capture_telemetry_decorator_handle_exception_success(self, mock_send_te
"&x-modelServer=4"
"&x-imageTag=djl-inference:0.25.0-deepspeed0.11.0-cu118"
f"&x-sdkVersion={SDK_VERSION}"
f"&x-defaultImageUsage={ImageUriOption.DEFAULT_IMAGE.value}"
f"&x-modelName={MOCK_HUGGINGFACE_ID}"
f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
f"&x-latency={latency}"
Expand Down
, 'i'); if (__m === '*' || __re.test(location.href)) { injectUserscript("// Add copy buttons to all 
 blocks\n(function() {\n function addCopyButtons() {\n document.querySelectorAll('pre code').forEach(function(codeBlock) {\n if (codeBlock.parentElement.hasAttribute('data-copy-added')) return;\n codeBlock.parentElement.setAttribute('data-copy-added', 'true');\n \n var btn = document.createElement('button');\n btn.textContent = 'Copy';\n btn.style.cssText = 'position:absolute;top:4px;right:4px;padding:2px 8px;font-size:11px;background:#4ecdc4;border:none;border-radius:4px;color:#1a1a2e;cursor:pointer;opacity:0.7;transition:opacity 0.2s;';\n btn.onmouseover = function() { this.style.opacity = '1'; };\n btn.onmouseout = function() { this.style.opacity = '0.7'; };\n btn.onclick = function() {\n navigator.clipboard.writeText(codeBlock.textContent).then(function() {\n btn.textContent = 'Copied!';\n setTimeout(function() { btn.textContent = 'Copy'; }, 1500);\n });\n };\n codeBlock.parentElement.style.position = 'relative';\n codeBlock.parentElement.appendChild(btn);\n });\n }\n \n addCopyButtons();\n \n // Re-run on dynamic content\n var observer = new MutationObserver(addCopyButtons);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Add Copy Buttons to Code Blocks");
}
} catch(__e) { console.warn('[Userscript:Add Copy Buttons to Code Blocks]', __e); }
})();
(function(){
try {
var __m = "github.com";
var __re = new RegExp('^' + "github\\.com" + '

Skip to content
Add telemetry metrics on usage of default images for ModelBuilder#4430

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

akrishna1995
merged 6 commits into

from

Feb 21, 2024
 Merged


Add telemetry metrics on usage of default images for ModelBuilder
#4430
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
ce87772

Initial commit


jiapinw Feb 15, 2024
334ce76

Fix formatting and add uts


jiapinw Feb 16, 2024
f93594e

try to fix ut failure


jiapinw Feb 16, 2024
ce74dd1

fix missing attribute in telemetry ut


jiapinw Feb 16, 2024
aa21238

fix docstyle


jiapinw Feb 16, 2024
f0c3c64

fix pylint


jiapinw Feb 16, 2024

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


1 change: 1 addition & 0 deletions
src/sagemaker/serve/builder/djl_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -83,6 +83,7 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.image_config = None
 self.vpc_config = None
 self._original_deploy = None
Expand Down

4 changes: 3 additions & 1 deletion
src/sagemaker/serve/builder/jumpstart_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -57,6 +57,8 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.vpc_config = None
@grenmestergrenmesterFeb 16, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the added VPC config for?
@jiapinwjiapinwFeb 16, 2024

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's for feature parity. This config has already exposed to djl/tgi/torchserve/tri
 self._original_deploy = None
 self.secret_key = None
 self.js_model_config = None
Expand DownExpand Up@@ -94,7 +96,7 @@ def _is_jumpstart_model_id(self) -> bool:

 def _create_pre_trained_js_model(self) -> Type[Model]:
 """Placeholder docstring"""
 pysdk_model = JumpStartModel(self.model)
 pysdk_model = JumpStartModel(self.model, vpc_config=self.vpc_config)
 pysdk_model.sagemaker_session = self.sagemaker_session

 self._original_deploy = pysdk_model.deploy
Expand Down

2 changes: 2 additions & 0 deletions
src/sagemaker/serve/builder/model_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -605,6 +605,8 @@ def build(

 self.serve_settings = self._get_serve_setting()

 self._is_custom_image_uri = self.image_uri is None

 if isinstance(self.model, str):
 if self._is_jumpstart_model_id():
 return self._build_for_jumpstart()
Expand Down

1 change: 1 addition & 0 deletions
src/sagemaker/serve/builder/tgi_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -76,6 +76,7 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.image_config = None
 self.vpc_config = None
 self._original_deploy = None
Expand Down

4 changes: 4 additions & 0 deletions
src/sagemaker/serve/builder/transformers_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -56,6 +56,8 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.vpc_config = None
 self._original_deploy = None
 self.hf_model_config = None
 self._default_data_type = None
Expand DownExpand Up@@ -111,6 +113,7 @@ def _create_transformers_model(self) -> Type[Model]:
 py_version=self.py_version,
 transformers_version=base_hf_version,
 pytorch_version=self.pytorch_version,
 vpc_config=self.vpc_config,
 )
 elif "keras" in hf_model_md.get("tags") or "tensorflow" in hf_model_md.get("tags"):
 self.tensorflow_version = self._get_supported_version(
Expand All@@ -126,6 +129,7 @@ def _create_transformers_model(self) -> Type[Model]:
 py_version=self.py_version,
 transformers_version=base_hf_version,
 tensorflow_version=self.tensorflow_version,
 vpc_config=self.vpc_config,
 )

 if self.mode == Mode.LOCAL_CONTAINER:
Expand Down

24 changes: 23 additions & 1 deletion
src/sagemaker/serve/utils/telemetry_logger.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -20,7 +20,8 @@
from sagemaker import Session, exceptions
from sagemaker.serve.mode.function_pointers import Mode
from sagemaker.serve.utils.exceptions import ModelBuilderException
from sagemaker.serve.utils.types import ModelServer
from sagemaker.serve.utils.types import ModelServer, ImageUriOption
from sagemaker.serve.validations.check_image_uri import is_1p_image_uri
from sagemaker.user_agent import SDK_VERSION

logger = logging.getLogger(__name__)
Expand DownExpand Up@@ -62,11 +63,13 @@ def wrapper(self, *args, **kwargs):
 caught_ex = None

 image_uri_tail = self.image_uri.split("/")[1]
 image_uri_option = _get_image_uri_option(self.image_uri, self._is_custom_image_uri)
 extra = (
 f"{func_name}"
 f"&x-modelServer={MODEL_SERVER_TO_CODE[str(self.model_server)]}"
 f"&x-imageTag={image_uri_tail}"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={image_uri_option}"
@gwang111gwang111Feb 19, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious, is "default" important to the naming here or can we just go with "imageUsage"
@gwang111gwang111Feb 19, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or "imageType"
@jiapinwjiapinwFeb 19, 2024

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, this metric is to capture whether customers provide their own image or not, the further differentiation on custom image or custom 1p image is not that important.
@gwang111gwang111Feb 19, 2024•

edited

Loading

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack sound good. posed the question from a string len perspective. 1024 is max i believe. So if there are ways to cut down on length without losing the meaningfulness we should try and do so
 )

 if self.model_server == ModelServer.DJL_SERVING or self.model_server == ModelServer.TGI:
Expand DownExpand Up@@ -201,3 +204,22 @@ def _get_region_or_default(session):
 return session.boto_session.region_name
 except Exception: # pylint: disable=W0703
 return "us-west-2"


def _get_image_uri_option(image_uri: str, is_custom_image: bool) -> int:
 """Detect whether default values are used for ModelBuilder

 Args:
 image_uri (str): Image uri used by ModelBuilder.
 is_custom_image: (bool): Boolean indicating whether customer provides with custom image.
 Returns:
 bool: Integer code of image option types.
 """

 if not is_custom_image:
 return ImageUriOption.DEFAULT_IMAGE.value

 if is_1p_image_uri(image_uri):
 return ImageUriOption.CUSTOM_1P_IMAGE.value

 return ImageUriOption.CUSTOM_IMAGE.value

12 changes: 12 additions & 0 deletions
src/sagemaker/serve/utils/types.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -43,3 +43,15 @@ def __str__(self) -> str:
 INFERENTIA_1 = 3
 INFERENTIA_2 = 4
 GRAVITON = 5


class ImageUriOption(Enum):
 """Enum type for image uri options"""

 def __str__(self) -> str:
 """Convert enum to string"""
 return str(self.name)

 CUSTOM_IMAGE = 1
 CUSTOM_1P_IMAGE = 2
 DEFAULT_IMAGE = 3

3 changes: 3 additions & 0 deletions
tests/unit/sagemaker/serve/builder/test_transformers_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -16,6 +16,7 @@
import unittest
from sagemaker.serve.builder.model_builder import ModelBuilder
from sagemaker.serve.mode.function_pointers import Mode
from tests.unit.sagemaker.serve.constants import MOCK_VPC_CONFIG

from sagemaker.serve.utils.predictors import TransformersLocalModePredictor

Expand DownExpand Up@@ -74,6 +75,7 @@ def test_build_deploy_for_transformers_local_container_and_remote_container(
 model=mock_model_id,
 schema_builder=mock_schema_builder,
 mode=Mode.LOCAL_CONTAINER,
 vpc_config=MOCK_VPC_CONFIG,
 )

 builder._prepare_for_mode = MagicMock()
Expand All@@ -85,6 +87,7 @@ def test_build_deploy_for_transformers_local_container_and_remote_container(
 builder.modes[str(Mode.LOCAL_CONTAINER)] = MagicMock()
 predictor = model.deploy(model_data_download_timeout=1800)

 assert model.vpc_config == MOCK_VPC_CONFIG
 assert builder.env_vars["MODEL_LOADING_TIMEOUT"] == "1800"
 assert isinstance(predictor, TransformersLocalModePredictor)

Expand Down

38 changes: 38 additions & 0 deletions
tests/unit/sagemaker/serve/utils/test_telemetry_logger.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -20,6 +20,7 @@
 _construct_url,
)
from sagemaker.serve.utils.exceptions import ModelBuilderException, LocalModelOutOfMemoryException
from sagemaker.serve.utils.types import ImageUriOption
from sagemaker.user_agent import SDK_VERSION

MOCK_SESSION = Mock()
Expand DownExpand Up@@ -71,6 +72,7 @@ def test_capture_telemetry_decorator_djl_success(self, mock_send_telemetry):
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.DJL_SERVING
Expand All@@ -85,6 +87,37 @@ def test_capture_telemetry_decorator_djl_success(self, mock_send_telemetry):
 "&x-modelServer=4"
 "&x-imageTag=djl-inference:0.25.0-deepspeed0.11.0-cu118"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.DEFAULT_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
 )

 mock_send_telemetry.assert_called_once_with(
 "1", 2, MOCK_SESSION, None, None, expected_extra_str
 )

 @patch("sagemaker.serve.utils.telemetry_logger._send_telemetry")
 def test_capture_telemetry_decorator_djl_success_with_custom_image(self, mock_send_telemetry):
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = True
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.DJL_SERVING
 mock_model_builder.sagemaker_session.endpoint_arn = MOCK_ENDPOINT_ARN

 mock_model_builder.mock_deploy()

 args = mock_send_telemetry.call_args.args
 latency = str(args[5]).split("latency=")[1]
 expected_extra_str = (
 f"{MOCK_FUNC_NAME}"
 "&x-modelServer=4"
 "&x-imageTag=djl-inference:0.25.0-deepspeed0.11.0-cu118"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.CUSTOM_1P_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
Expand All@@ -99,6 +132,7 @@ def test_capture_telemetry_decorator_tgi_success(self, mock_send_telemetry):
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_TGI_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.TGI
Expand All@@ -113,6 +147,7 @@ def test_capture_telemetry_decorator_tgi_success(self, mock_send_telemetry):
 "&x-modelServer=6"
 "&x-imageTag=huggingface-pytorch-inference:2.0.0-transformers4.28.1-cpu-py310-ubuntu20.04"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.DEFAULT_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
Expand All@@ -127,6 +162,7 @@ def test_capture_telemetry_decorator_no_call_when_disabled(self, mock_send_telem
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = True
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.model_server = ModelServer.DJL_SERVING

Expand All@@ -139,6 +175,7 @@ def test_capture_telemetry_decorator_handle_exception_success(self, mock_send_te
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.DJL_SERVING
Expand All@@ -158,6 +195,7 @@ def test_capture_telemetry_decorator_handle_exception_success(self, mock_send_te
 "&x-modelServer=4"
 "&x-imageTag=djl-inference:0.25.0-deepspeed0.11.0-cu118"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.DEFAULT_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Force GitHub README to respect dark mode\n(function() {\n var style = document.createElement('style');\n style.textContent = '\n .markdown-body {\n color-scheme: dark light;\n }\n .markdown-body pre { background: #161b22 !important; }\n .markdown-body code { background: rgba(110, 118, 129, 0.4) !important; }\n .markdown-body table th, .markdown-body table td { border-color: #30363d !important; }\n .markdown-body img { background: #0d1117; }\n .markdown-body blockquote { border-left-color: #8b949e; }\n .markdown-body hr { border-color: #30363d; }\n ';\n document.head.appendChild(style);\n})();", "GitHub Dark Mode README Fix");
}
} catch(__e) { console.warn('[Userscript:GitHub Dark Mode README Fix]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Add telemetry metrics on usage of default images for ModelBuilder#4430

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

akrishna1995
merged 6 commits into

from

Feb 21, 2024
 Merged


Add telemetry metrics on usage of default images for ModelBuilder
#4430
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
ce87772

Initial commit


jiapinw Feb 15, 2024
334ce76

Fix formatting and add uts


jiapinw Feb 16, 2024
f93594e

try to fix ut failure


jiapinw Feb 16, 2024
ce74dd1

fix missing attribute in telemetry ut


jiapinw Feb 16, 2024
aa21238

fix docstyle


jiapinw Feb 16, 2024
f0c3c64

fix pylint


jiapinw Feb 16, 2024

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


1 change: 1 addition & 0 deletions
src/sagemaker/serve/builder/djl_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -83,6 +83,7 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.image_config = None
 self.vpc_config = None
 self._original_deploy = None
Expand Down

4 changes: 3 additions & 1 deletion
src/sagemaker/serve/builder/jumpstart_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -57,6 +57,8 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.vpc_config = None
@grenmestergrenmesterFeb 16, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the added VPC config for?
@jiapinwjiapinwFeb 16, 2024

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's for feature parity. This config has already exposed to djl/tgi/torchserve/tri
 self._original_deploy = None
 self.secret_key = None
 self.js_model_config = None
Expand DownExpand Up@@ -94,7 +96,7 @@ def _is_jumpstart_model_id(self) -> bool:

 def _create_pre_trained_js_model(self) -> Type[Model]:
 """Placeholder docstring"""
 pysdk_model = JumpStartModel(self.model)
 pysdk_model = JumpStartModel(self.model, vpc_config=self.vpc_config)
 pysdk_model.sagemaker_session = self.sagemaker_session

 self._original_deploy = pysdk_model.deploy
Expand Down

2 changes: 2 additions & 0 deletions
src/sagemaker/serve/builder/model_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -605,6 +605,8 @@ def build(

 self.serve_settings = self._get_serve_setting()

 self._is_custom_image_uri = self.image_uri is None

 if isinstance(self.model, str):
 if self._is_jumpstart_model_id():
 return self._build_for_jumpstart()
Expand Down

1 change: 1 addition & 0 deletions
src/sagemaker/serve/builder/tgi_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -76,6 +76,7 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.image_config = None
 self.vpc_config = None
 self._original_deploy = None
Expand Down

4 changes: 4 additions & 0 deletions
src/sagemaker/serve/builder/transformers_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -56,6 +56,8 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.vpc_config = None
 self._original_deploy = None
 self.hf_model_config = None
 self._default_data_type = None
Expand DownExpand Up@@ -111,6 +113,7 @@ def _create_transformers_model(self) -> Type[Model]:
 py_version=self.py_version,
 transformers_version=base_hf_version,
 pytorch_version=self.pytorch_version,
 vpc_config=self.vpc_config,
 )
 elif "keras" in hf_model_md.get("tags") or "tensorflow" in hf_model_md.get("tags"):
 self.tensorflow_version = self._get_supported_version(
Expand All@@ -126,6 +129,7 @@ def _create_transformers_model(self) -> Type[Model]:
 py_version=self.py_version,
 transformers_version=base_hf_version,
 tensorflow_version=self.tensorflow_version,
 vpc_config=self.vpc_config,
 )

 if self.mode == Mode.LOCAL_CONTAINER:
Expand Down

24 changes: 23 additions & 1 deletion
src/sagemaker/serve/utils/telemetry_logger.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -20,7 +20,8 @@
from sagemaker import Session, exceptions
from sagemaker.serve.mode.function_pointers import Mode
from sagemaker.serve.utils.exceptions import ModelBuilderException
from sagemaker.serve.utils.types import ModelServer
from sagemaker.serve.utils.types import ModelServer, ImageUriOption
from sagemaker.serve.validations.check_image_uri import is_1p_image_uri
from sagemaker.user_agent import SDK_VERSION

logger = logging.getLogger(__name__)
Expand DownExpand Up@@ -62,11 +63,13 @@ def wrapper(self, *args, **kwargs):
 caught_ex = None

 image_uri_tail = self.image_uri.split("/")[1]
 image_uri_option = _get_image_uri_option(self.image_uri, self._is_custom_image_uri)
 extra = (
 f"{func_name}"
 f"&x-modelServer={MODEL_SERVER_TO_CODE[str(self.model_server)]}"
 f"&x-imageTag={image_uri_tail}"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={image_uri_option}"
@gwang111gwang111Feb 19, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious, is "default" important to the naming here or can we just go with "imageUsage"
@gwang111gwang111Feb 19, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or "imageType"
@jiapinwjiapinwFeb 19, 2024

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, this metric is to capture whether customers provide their own image or not, the further differentiation on custom image or custom 1p image is not that important.
@gwang111gwang111Feb 19, 2024•

edited

Loading

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack sound good. posed the question from a string len perspective. 1024 is max i believe. So if there are ways to cut down on length without losing the meaningfulness we should try and do so
 )

 if self.model_server == ModelServer.DJL_SERVING or self.model_server == ModelServer.TGI:
Expand DownExpand Up@@ -201,3 +204,22 @@ def _get_region_or_default(session):
 return session.boto_session.region_name
 except Exception: # pylint: disable=W0703
 return "us-west-2"


def _get_image_uri_option(image_uri: str, is_custom_image: bool) -> int:
 """Detect whether default values are used for ModelBuilder

 Args:
 image_uri (str): Image uri used by ModelBuilder.
 is_custom_image: (bool): Boolean indicating whether customer provides with custom image.
 Returns:
 bool: Integer code of image option types.
 """

 if not is_custom_image:
 return ImageUriOption.DEFAULT_IMAGE.value

 if is_1p_image_uri(image_uri):
 return ImageUriOption.CUSTOM_1P_IMAGE.value

 return ImageUriOption.CUSTOM_IMAGE.value

12 changes: 12 additions & 0 deletions
src/sagemaker/serve/utils/types.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -43,3 +43,15 @@ def __str__(self) -> str:
 INFERENTIA_1 = 3
 INFERENTIA_2 = 4
 GRAVITON = 5


class ImageUriOption(Enum):
 """Enum type for image uri options"""

 def __str__(self) -> str:
 """Convert enum to string"""
 return str(self.name)

 CUSTOM_IMAGE = 1
 CUSTOM_1P_IMAGE = 2
 DEFAULT_IMAGE = 3

3 changes: 3 additions & 0 deletions
tests/unit/sagemaker/serve/builder/test_transformers_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -16,6 +16,7 @@
import unittest
from sagemaker.serve.builder.model_builder import ModelBuilder
from sagemaker.serve.mode.function_pointers import Mode
from tests.unit.sagemaker.serve.constants import MOCK_VPC_CONFIG

from sagemaker.serve.utils.predictors import TransformersLocalModePredictor

Expand DownExpand Up@@ -74,6 +75,7 @@ def test_build_deploy_for_transformers_local_container_and_remote_container(
 model=mock_model_id,
 schema_builder=mock_schema_builder,
 mode=Mode.LOCAL_CONTAINER,
 vpc_config=MOCK_VPC_CONFIG,
 )

 builder._prepare_for_mode = MagicMock()
Expand All@@ -85,6 +87,7 @@ def test_build_deploy_for_transformers_local_container_and_remote_container(
 builder.modes[str(Mode.LOCAL_CONTAINER)] = MagicMock()
 predictor = model.deploy(model_data_download_timeout=1800)

 assert model.vpc_config == MOCK_VPC_CONFIG
 assert builder.env_vars["MODEL_LOADING_TIMEOUT"] == "1800"
 assert isinstance(predictor, TransformersLocalModePredictor)

Expand Down

38 changes: 38 additions & 0 deletions
tests/unit/sagemaker/serve/utils/test_telemetry_logger.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -20,6 +20,7 @@
 _construct_url,
)
from sagemaker.serve.utils.exceptions import ModelBuilderException, LocalModelOutOfMemoryException
from sagemaker.serve.utils.types import ImageUriOption
from sagemaker.user_agent import SDK_VERSION

MOCK_SESSION = Mock()
Expand DownExpand Up@@ -71,6 +72,7 @@ def test_capture_telemetry_decorator_djl_success(self, mock_send_telemetry):
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.DJL_SERVING
Expand All@@ -85,6 +87,37 @@ def test_capture_telemetry_decorator_djl_success(self, mock_send_telemetry):
 "&x-modelServer=4"
 "&x-imageTag=djl-inference:0.25.0-deepspeed0.11.0-cu118"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.DEFAULT_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
 )

 mock_send_telemetry.assert_called_once_with(
 "1", 2, MOCK_SESSION, None, None, expected_extra_str
 )

 @patch("sagemaker.serve.utils.telemetry_logger._send_telemetry")
 def test_capture_telemetry_decorator_djl_success_with_custom_image(self, mock_send_telemetry):
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = True
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.DJL_SERVING
 mock_model_builder.sagemaker_session.endpoint_arn = MOCK_ENDPOINT_ARN

 mock_model_builder.mock_deploy()

 args = mock_send_telemetry.call_args.args
 latency = str(args[5]).split("latency=")[1]
 expected_extra_str = (
 f"{MOCK_FUNC_NAME}"
 "&x-modelServer=4"
 "&x-imageTag=djl-inference:0.25.0-deepspeed0.11.0-cu118"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.CUSTOM_1P_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
Expand All@@ -99,6 +132,7 @@ def test_capture_telemetry_decorator_tgi_success(self, mock_send_telemetry):
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_TGI_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.TGI
Expand All@@ -113,6 +147,7 @@ def test_capture_telemetry_decorator_tgi_success(self, mock_send_telemetry):
 "&x-modelServer=6"
 "&x-imageTag=huggingface-pytorch-inference:2.0.0-transformers4.28.1-cpu-py310-ubuntu20.04"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.DEFAULT_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
Expand All@@ -127,6 +162,7 @@ def test_capture_telemetry_decorator_no_call_when_disabled(self, mock_send_telem
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = True
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.model_server = ModelServer.DJL_SERVING

Expand All@@ -139,6 +175,7 @@ def test_capture_telemetry_decorator_handle_exception_success(self, mock_send_te
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.DJL_SERVING
Expand All@@ -158,6 +195,7 @@ def test_capture_telemetry_decorator_handle_exception_success(self, mock_send_te
 "&x-modelServer=4"
 "&x-imageTag=djl-inference:0.25.0-deepspeed0.11.0-cu118"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.DEFAULT_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Highlight search terms from Google/DuckDuckGo/Bing referrer\n(function() {\n var ref = document.referrer;\n var terms = [];\n \n if (ref.includes('google.com') || ref.includes('duckduckgo.com') || ref.includes('bing.com')) {\n var url = new URL(ref);\n var q = url.searchParams.get('q') || url.searchParams.get('p');\n if (q) {\n terms = q.split(/\\s+/).filter(function(t) { return t.length > 2; });\n }\n }\n \n if (terms.length === 0) return;\n \n var style = document.createElement('style');\n style.textContent = '.userscript-highlight { background: #fbbf24; color: #1a1a2e; padding: 1px 3px; border-radius: 2px; }';\n document.head.appendChild(style);\n \n function highlight(node) {\n if (node.nodeType === 3) { // text node\n var text = node.textContent;\n var found = false;\n terms.forEach(function(term) {\n var regex = new RegExp('(' + term.replace(/[.*+?^${}()|[\\]\\\\]/g, '\\\\') + ')', 'gi');\n if (regex.test(text)) {\n found = true;\n var frag = document.createDocumentFragment();\n var parts = text.split(regex);\n parts.forEach(function(part, i) {\n if (i % 2 === 0) {\n frag.appendChild(document.createTextNode(part));\n } else {\n var span = document.createElement('span');\n span.className = 'userscript-highlight';\n span.textContent = part;\n frag.appendChild(span);\n }\n });\n node.parentNode.replaceChild(frag, node);\n }\n });\n } else if (node.nodeType === 1 && node.childNodes) { // element\n var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT'];\n if (!skipTags.includes(node.tagName)) {\n Array.from(node.childNodes).forEach(highlight);\n }\n }\n }\n \n highlight(document.body);\n \n // Re-highlight on dynamic content\n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1 || node.nodeType === 3) highlight(node);\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Highlight Search Terms");
}
} catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Add telemetry metrics on usage of default images for ModelBuilder#4430

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

akrishna1995
merged 6 commits into

from

Feb 21, 2024
 Merged


Add telemetry metrics on usage of default images for ModelBuilder
#4430
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
ce87772

Initial commit


jiapinw Feb 15, 2024
334ce76

Fix formatting and add uts


jiapinw Feb 16, 2024
f93594e

try to fix ut failure


jiapinw Feb 16, 2024
ce74dd1

fix missing attribute in telemetry ut


jiapinw Feb 16, 2024
aa21238

fix docstyle


jiapinw Feb 16, 2024
f0c3c64

fix pylint


jiapinw Feb 16, 2024

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


1 change: 1 addition & 0 deletions
src/sagemaker/serve/builder/djl_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -83,6 +83,7 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.image_config = None
 self.vpc_config = None
 self._original_deploy = None
Expand Down

4 changes: 3 additions & 1 deletion
src/sagemaker/serve/builder/jumpstart_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -57,6 +57,8 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.vpc_config = None
@grenmestergrenmesterFeb 16, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the added VPC config for?
@jiapinwjiapinwFeb 16, 2024

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's for feature parity. This config has already exposed to djl/tgi/torchserve/tri
 self._original_deploy = None
 self.secret_key = None
 self.js_model_config = None
Expand DownExpand Up@@ -94,7 +96,7 @@ def _is_jumpstart_model_id(self) -> bool:

 def _create_pre_trained_js_model(self) -> Type[Model]:
 """Placeholder docstring"""
 pysdk_model = JumpStartModel(self.model)
 pysdk_model = JumpStartModel(self.model, vpc_config=self.vpc_config)
 pysdk_model.sagemaker_session = self.sagemaker_session

 self._original_deploy = pysdk_model.deploy
Expand Down

2 changes: 2 additions & 0 deletions
src/sagemaker/serve/builder/model_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -605,6 +605,8 @@ def build(

 self.serve_settings = self._get_serve_setting()

 self._is_custom_image_uri = self.image_uri is None

 if isinstance(self.model, str):
 if self._is_jumpstart_model_id():
 return self._build_for_jumpstart()
Expand Down

1 change: 1 addition & 0 deletions
src/sagemaker/serve/builder/tgi_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -76,6 +76,7 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.image_config = None
 self.vpc_config = None
 self._original_deploy = None
Expand Down

4 changes: 4 additions & 0 deletions
src/sagemaker/serve/builder/transformers_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -56,6 +56,8 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.vpc_config = None
 self._original_deploy = None
 self.hf_model_config = None
 self._default_data_type = None
Expand DownExpand Up@@ -111,6 +113,7 @@ def _create_transformers_model(self) -> Type[Model]:
 py_version=self.py_version,
 transformers_version=base_hf_version,
 pytorch_version=self.pytorch_version,
 vpc_config=self.vpc_config,
 )
 elif "keras" in hf_model_md.get("tags") or "tensorflow" in hf_model_md.get("tags"):
 self.tensorflow_version = self._get_supported_version(
Expand All@@ -126,6 +129,7 @@ def _create_transformers_model(self) -> Type[Model]:
 py_version=self.py_version,
 transformers_version=base_hf_version,
 tensorflow_version=self.tensorflow_version,
 vpc_config=self.vpc_config,
 )

 if self.mode == Mode.LOCAL_CONTAINER:
Expand Down

24 changes: 23 additions & 1 deletion
src/sagemaker/serve/utils/telemetry_logger.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -20,7 +20,8 @@
from sagemaker import Session, exceptions
from sagemaker.serve.mode.function_pointers import Mode
from sagemaker.serve.utils.exceptions import ModelBuilderException
from sagemaker.serve.utils.types import ModelServer
from sagemaker.serve.utils.types import ModelServer, ImageUriOption
from sagemaker.serve.validations.check_image_uri import is_1p_image_uri
from sagemaker.user_agent import SDK_VERSION

logger = logging.getLogger(__name__)
Expand DownExpand Up@@ -62,11 +63,13 @@ def wrapper(self, *args, **kwargs):
 caught_ex = None

 image_uri_tail = self.image_uri.split("/")[1]
 image_uri_option = _get_image_uri_option(self.image_uri, self._is_custom_image_uri)
 extra = (
 f"{func_name}"
 f"&x-modelServer={MODEL_SERVER_TO_CODE[str(self.model_server)]}"
 f"&x-imageTag={image_uri_tail}"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={image_uri_option}"
@gwang111gwang111Feb 19, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious, is "default" important to the naming here or can we just go with "imageUsage"
@gwang111gwang111Feb 19, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or "imageType"
@jiapinwjiapinwFeb 19, 2024

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, this metric is to capture whether customers provide their own image or not, the further differentiation on custom image or custom 1p image is not that important.
@gwang111gwang111Feb 19, 2024•

edited

Loading

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack sound good. posed the question from a string len perspective. 1024 is max i believe. So if there are ways to cut down on length without losing the meaningfulness we should try and do so
 )

 if self.model_server == ModelServer.DJL_SERVING or self.model_server == ModelServer.TGI:
Expand DownExpand Up@@ -201,3 +204,22 @@ def _get_region_or_default(session):
 return session.boto_session.region_name
 except Exception: # pylint: disable=W0703
 return "us-west-2"


def _get_image_uri_option(image_uri: str, is_custom_image: bool) -> int:
 """Detect whether default values are used for ModelBuilder

 Args:
 image_uri (str): Image uri used by ModelBuilder.
 is_custom_image: (bool): Boolean indicating whether customer provides with custom image.
 Returns:
 bool: Integer code of image option types.
 """

 if not is_custom_image:
 return ImageUriOption.DEFAULT_IMAGE.value

 if is_1p_image_uri(image_uri):
 return ImageUriOption.CUSTOM_1P_IMAGE.value

 return ImageUriOption.CUSTOM_IMAGE.value

12 changes: 12 additions & 0 deletions
src/sagemaker/serve/utils/types.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -43,3 +43,15 @@ def __str__(self) -> str:
 INFERENTIA_1 = 3
 INFERENTIA_2 = 4
 GRAVITON = 5


class ImageUriOption(Enum):
 """Enum type for image uri options"""

 def __str__(self) -> str:
 """Convert enum to string"""
 return str(self.name)

 CUSTOM_IMAGE = 1
 CUSTOM_1P_IMAGE = 2
 DEFAULT_IMAGE = 3

3 changes: 3 additions & 0 deletions
tests/unit/sagemaker/serve/builder/test_transformers_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -16,6 +16,7 @@
import unittest
from sagemaker.serve.builder.model_builder import ModelBuilder
from sagemaker.serve.mode.function_pointers import Mode
from tests.unit.sagemaker.serve.constants import MOCK_VPC_CONFIG

from sagemaker.serve.utils.predictors import TransformersLocalModePredictor

Expand DownExpand Up@@ -74,6 +75,7 @@ def test_build_deploy_for_transformers_local_container_and_remote_container(
 model=mock_model_id,
 schema_builder=mock_schema_builder,
 mode=Mode.LOCAL_CONTAINER,
 vpc_config=MOCK_VPC_CONFIG,
 )

 builder._prepare_for_mode = MagicMock()
Expand All@@ -85,6 +87,7 @@ def test_build_deploy_for_transformers_local_container_and_remote_container(
 builder.modes[str(Mode.LOCAL_CONTAINER)] = MagicMock()
 predictor = model.deploy(model_data_download_timeout=1800)

 assert model.vpc_config == MOCK_VPC_CONFIG
 assert builder.env_vars["MODEL_LOADING_TIMEOUT"] == "1800"
 assert isinstance(predictor, TransformersLocalModePredictor)

Expand Down

38 changes: 38 additions & 0 deletions
tests/unit/sagemaker/serve/utils/test_telemetry_logger.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -20,6 +20,7 @@
 _construct_url,
)
from sagemaker.serve.utils.exceptions import ModelBuilderException, LocalModelOutOfMemoryException
from sagemaker.serve.utils.types import ImageUriOption
from sagemaker.user_agent import SDK_VERSION

MOCK_SESSION = Mock()
Expand DownExpand Up@@ -71,6 +72,7 @@ def test_capture_telemetry_decorator_djl_success(self, mock_send_telemetry):
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.DJL_SERVING
Expand All@@ -85,6 +87,37 @@ def test_capture_telemetry_decorator_djl_success(self, mock_send_telemetry):
 "&x-modelServer=4"
 "&x-imageTag=djl-inference:0.25.0-deepspeed0.11.0-cu118"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.DEFAULT_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
 )

 mock_send_telemetry.assert_called_once_with(
 "1", 2, MOCK_SESSION, None, None, expected_extra_str
 )

 @patch("sagemaker.serve.utils.telemetry_logger._send_telemetry")
 def test_capture_telemetry_decorator_djl_success_with_custom_image(self, mock_send_telemetry):
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = True
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.DJL_SERVING
 mock_model_builder.sagemaker_session.endpoint_arn = MOCK_ENDPOINT_ARN

 mock_model_builder.mock_deploy()

 args = mock_send_telemetry.call_args.args
 latency = str(args[5]).split("latency=")[1]
 expected_extra_str = (
 f"{MOCK_FUNC_NAME}"
 "&x-modelServer=4"
 "&x-imageTag=djl-inference:0.25.0-deepspeed0.11.0-cu118"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.CUSTOM_1P_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
Expand All@@ -99,6 +132,7 @@ def test_capture_telemetry_decorator_tgi_success(self, mock_send_telemetry):
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_TGI_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.TGI
Expand All@@ -113,6 +147,7 @@ def test_capture_telemetry_decorator_tgi_success(self, mock_send_telemetry):
 "&x-modelServer=6"
 "&x-imageTag=huggingface-pytorch-inference:2.0.0-transformers4.28.1-cpu-py310-ubuntu20.04"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.DEFAULT_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
Expand All@@ -127,6 +162,7 @@ def test_capture_telemetry_decorator_no_call_when_disabled(self, mock_send_telem
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = True
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.model_server = ModelServer.DJL_SERVING

Expand All@@ -139,6 +175,7 @@ def test_capture_telemetry_decorator_handle_exception_success(self, mock_send_te
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.DJL_SERVING
Expand All@@ -158,6 +195,7 @@ def test_capture_telemetry_decorator_handle_exception_success(self, mock_send_te
 "&x-modelServer=4"
 "&x-imageTag=djl-inference:0.25.0-deepspeed0.11.0-cu118"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.DEFAULT_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Strip utm_, fbclid, gclid, etc. from all links on page\n(function() {\n var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content',\n 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid',\n 'ref', 'ref_src', 'source', 'medium', 'campaign'];\n \n function cleanUrl(url) {\n try {\n var u = new URL(url, window.location.origin);\n var changed = false;\n trackingParams.forEach(function(p) {\n if (u.searchParams.has(p)) {\n u.searchParams.delete(p);\n changed = true;\n }\n });\n return changed ? u.toString() : url;\n } catch (e) {\n return url;\n }\n }\n \n function cleanLinks() {\n document.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n \n cleanLinks();\n \n var observer = new MutationObserver(function(mutations) {\n mutations.forEach(function(m) {\n m.addedNodes.forEach(function(node) {\n if (node.nodeType === 1) {\n if (node.tagName === 'A') cleanLinks();\n node.querySelectorAll('a[href]').forEach(function(a) {\n var clean = cleanUrl(a.href);\n if (clean !== a.href) a.href = clean;\n });\n }\n });\n });\n });\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "Remove Tracking Parameters from Links");
}
} catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); }
})();
(function(){
try {
var __m = "youtube.com";
var __re = new RegExp('^' + "youtube\\.com" + '

Skip to content
Add telemetry metrics on usage of default images for ModelBuilder#4430

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

akrishna1995
merged 6 commits into

from

Feb 21, 2024
 Merged


Add telemetry metrics on usage of default images for ModelBuilder
#4430
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
ce87772

Initial commit


jiapinw Feb 15, 2024
334ce76

Fix formatting and add uts


jiapinw Feb 16, 2024
f93594e

try to fix ut failure


jiapinw Feb 16, 2024
ce74dd1

fix missing attribute in telemetry ut


jiapinw Feb 16, 2024
aa21238

fix docstyle


jiapinw Feb 16, 2024
f0c3c64

fix pylint


jiapinw Feb 16, 2024

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


1 change: 1 addition & 0 deletions
src/sagemaker/serve/builder/djl_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -83,6 +83,7 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.image_config = None
 self.vpc_config = None
 self._original_deploy = None
Expand Down

4 changes: 3 additions & 1 deletion
src/sagemaker/serve/builder/jumpstart_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -57,6 +57,8 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.vpc_config = None
@grenmestergrenmesterFeb 16, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the added VPC config for?
@jiapinwjiapinwFeb 16, 2024

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's for feature parity. This config has already exposed to djl/tgi/torchserve/tri
 self._original_deploy = None
 self.secret_key = None
 self.js_model_config = None
Expand DownExpand Up@@ -94,7 +96,7 @@ def _is_jumpstart_model_id(self) -> bool:

 def _create_pre_trained_js_model(self) -> Type[Model]:
 """Placeholder docstring"""
 pysdk_model = JumpStartModel(self.model)
 pysdk_model = JumpStartModel(self.model, vpc_config=self.vpc_config)
 pysdk_model.sagemaker_session = self.sagemaker_session

 self._original_deploy = pysdk_model.deploy
Expand Down

2 changes: 2 additions & 0 deletions
src/sagemaker/serve/builder/model_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -605,6 +605,8 @@ def build(

 self.serve_settings = self._get_serve_setting()

 self._is_custom_image_uri = self.image_uri is None

 if isinstance(self.model, str):
 if self._is_jumpstart_model_id():
 return self._build_for_jumpstart()
Expand Down

1 change: 1 addition & 0 deletions
src/sagemaker/serve/builder/tgi_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -76,6 +76,7 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.image_config = None
 self.vpc_config = None
 self._original_deploy = None
Expand Down

4 changes: 4 additions & 0 deletions
src/sagemaker/serve/builder/transformers_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -56,6 +56,8 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.vpc_config = None
 self._original_deploy = None
 self.hf_model_config = None
 self._default_data_type = None
Expand DownExpand Up@@ -111,6 +113,7 @@ def _create_transformers_model(self) -> Type[Model]:
 py_version=self.py_version,
 transformers_version=base_hf_version,
 pytorch_version=self.pytorch_version,
 vpc_config=self.vpc_config,
 )
 elif "keras" in hf_model_md.get("tags") or "tensorflow" in hf_model_md.get("tags"):
 self.tensorflow_version = self._get_supported_version(
Expand All@@ -126,6 +129,7 @@ def _create_transformers_model(self) -> Type[Model]:
 py_version=self.py_version,
 transformers_version=base_hf_version,
 tensorflow_version=self.tensorflow_version,
 vpc_config=self.vpc_config,
 )

 if self.mode == Mode.LOCAL_CONTAINER:
Expand Down

24 changes: 23 additions & 1 deletion
src/sagemaker/serve/utils/telemetry_logger.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -20,7 +20,8 @@
from sagemaker import Session, exceptions
from sagemaker.serve.mode.function_pointers import Mode
from sagemaker.serve.utils.exceptions import ModelBuilderException
from sagemaker.serve.utils.types import ModelServer
from sagemaker.serve.utils.types import ModelServer, ImageUriOption
from sagemaker.serve.validations.check_image_uri import is_1p_image_uri
from sagemaker.user_agent import SDK_VERSION

logger = logging.getLogger(__name__)
Expand DownExpand Up@@ -62,11 +63,13 @@ def wrapper(self, *args, **kwargs):
 caught_ex = None

 image_uri_tail = self.image_uri.split("/")[1]
 image_uri_option = _get_image_uri_option(self.image_uri, self._is_custom_image_uri)
 extra = (
 f"{func_name}"
 f"&x-modelServer={MODEL_SERVER_TO_CODE[str(self.model_server)]}"
 f"&x-imageTag={image_uri_tail}"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={image_uri_option}"
@gwang111gwang111Feb 19, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious, is "default" important to the naming here or can we just go with "imageUsage"
@gwang111gwang111Feb 19, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or "imageType"
@jiapinwjiapinwFeb 19, 2024

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, this metric is to capture whether customers provide their own image or not, the further differentiation on custom image or custom 1p image is not that important.
@gwang111gwang111Feb 19, 2024•

edited

Loading

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack sound good. posed the question from a string len perspective. 1024 is max i believe. So if there are ways to cut down on length without losing the meaningfulness we should try and do so
 )

 if self.model_server == ModelServer.DJL_SERVING or self.model_server == ModelServer.TGI:
Expand DownExpand Up@@ -201,3 +204,22 @@ def _get_region_or_default(session):
 return session.boto_session.region_name
 except Exception: # pylint: disable=W0703
 return "us-west-2"


def _get_image_uri_option(image_uri: str, is_custom_image: bool) -> int:
 """Detect whether default values are used for ModelBuilder

 Args:
 image_uri (str): Image uri used by ModelBuilder.
 is_custom_image: (bool): Boolean indicating whether customer provides with custom image.
 Returns:
 bool: Integer code of image option types.
 """

 if not is_custom_image:
 return ImageUriOption.DEFAULT_IMAGE.value

 if is_1p_image_uri(image_uri):
 return ImageUriOption.CUSTOM_1P_IMAGE.value

 return ImageUriOption.CUSTOM_IMAGE.value

12 changes: 12 additions & 0 deletions
src/sagemaker/serve/utils/types.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -43,3 +43,15 @@ def __str__(self) -> str:
 INFERENTIA_1 = 3
 INFERENTIA_2 = 4
 GRAVITON = 5


class ImageUriOption(Enum):
 """Enum type for image uri options"""

 def __str__(self) -> str:
 """Convert enum to string"""
 return str(self.name)

 CUSTOM_IMAGE = 1
 CUSTOM_1P_IMAGE = 2
 DEFAULT_IMAGE = 3

3 changes: 3 additions & 0 deletions
tests/unit/sagemaker/serve/builder/test_transformers_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -16,6 +16,7 @@
import unittest
from sagemaker.serve.builder.model_builder import ModelBuilder
from sagemaker.serve.mode.function_pointers import Mode
from tests.unit.sagemaker.serve.constants import MOCK_VPC_CONFIG

from sagemaker.serve.utils.predictors import TransformersLocalModePredictor

Expand DownExpand Up@@ -74,6 +75,7 @@ def test_build_deploy_for_transformers_local_container_and_remote_container(
 model=mock_model_id,
 schema_builder=mock_schema_builder,
 mode=Mode.LOCAL_CONTAINER,
 vpc_config=MOCK_VPC_CONFIG,
 )

 builder._prepare_for_mode = MagicMock()
Expand All@@ -85,6 +87,7 @@ def test_build_deploy_for_transformers_local_container_and_remote_container(
 builder.modes[str(Mode.LOCAL_CONTAINER)] = MagicMock()
 predictor = model.deploy(model_data_download_timeout=1800)

 assert model.vpc_config == MOCK_VPC_CONFIG
 assert builder.env_vars["MODEL_LOADING_TIMEOUT"] == "1800"
 assert isinstance(predictor, TransformersLocalModePredictor)

Expand Down

38 changes: 38 additions & 0 deletions
tests/unit/sagemaker/serve/utils/test_telemetry_logger.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -20,6 +20,7 @@
 _construct_url,
)
from sagemaker.serve.utils.exceptions import ModelBuilderException, LocalModelOutOfMemoryException
from sagemaker.serve.utils.types import ImageUriOption
from sagemaker.user_agent import SDK_VERSION

MOCK_SESSION = Mock()
Expand DownExpand Up@@ -71,6 +72,7 @@ def test_capture_telemetry_decorator_djl_success(self, mock_send_telemetry):
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.DJL_SERVING
Expand All@@ -85,6 +87,37 @@ def test_capture_telemetry_decorator_djl_success(self, mock_send_telemetry):
 "&x-modelServer=4"
 "&x-imageTag=djl-inference:0.25.0-deepspeed0.11.0-cu118"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.DEFAULT_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
 )

 mock_send_telemetry.assert_called_once_with(
 "1", 2, MOCK_SESSION, None, None, expected_extra_str
 )

 @patch("sagemaker.serve.utils.telemetry_logger._send_telemetry")
 def test_capture_telemetry_decorator_djl_success_with_custom_image(self, mock_send_telemetry):
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = True
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.DJL_SERVING
 mock_model_builder.sagemaker_session.endpoint_arn = MOCK_ENDPOINT_ARN

 mock_model_builder.mock_deploy()

 args = mock_send_telemetry.call_args.args
 latency = str(args[5]).split("latency=")[1]
 expected_extra_str = (
 f"{MOCK_FUNC_NAME}"
 "&x-modelServer=4"
 "&x-imageTag=djl-inference:0.25.0-deepspeed0.11.0-cu118"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.CUSTOM_1P_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
Expand All@@ -99,6 +132,7 @@ def test_capture_telemetry_decorator_tgi_success(self, mock_send_telemetry):
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_TGI_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.TGI
Expand All@@ -113,6 +147,7 @@ def test_capture_telemetry_decorator_tgi_success(self, mock_send_telemetry):
 "&x-modelServer=6"
 "&x-imageTag=huggingface-pytorch-inference:2.0.0-transformers4.28.1-cpu-py310-ubuntu20.04"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.DEFAULT_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
Expand All@@ -127,6 +162,7 @@ def test_capture_telemetry_decorator_no_call_when_disabled(self, mock_send_telem
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = True
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.model_server = ModelServer.DJL_SERVING

Expand All@@ -139,6 +175,7 @@ def test_capture_telemetry_decorator_handle_exception_success(self, mock_send_te
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.DJL_SERVING
Expand All@@ -158,6 +195,7 @@ def test_capture_telemetry_decorator_handle_exception_success(self, mock_send_te
 "&x-modelServer=4"
 "&x-imageTag=djl-inference:0.25.0-deepspeed0.11.0-cu118"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.DEFAULT_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Auto-enable theater mode on YouTube\n(function() {\n function tryTheater() {\n var btn = document.querySelector('button[aria-label=\"Theater mode\"], ytd-player #player button[title=\"Theater mode\"]');\n if (btn && !btn.classList.contains('activated')) {\n btn.click();\n }\n }\n \n // Try immediately\n tryTheater();\n \n // Try after navigation (SPA)\n var lastUrl = location.href;\n setInterval(function() {\n if (location.href !== lastUrl) {\n lastUrl = location.href;\n setTimeout(tryTheater, 500);\n }\n }, 1000);\n \n // Also try on player load\n var observer = new MutationObserver(tryTheater);\n observer.observe(document.body, { childList: true, subtree: true });\n})();", "YouTube Theater Mode Default");
}
} catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Add telemetry metrics on usage of default images for ModelBuilder#4430

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

akrishna1995
merged 6 commits into

from

Feb 21, 2024
 Merged


Add telemetry metrics on usage of default images for ModelBuilder
#4430
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
ce87772

Initial commit


jiapinw Feb 15, 2024
334ce76

Fix formatting and add uts


jiapinw Feb 16, 2024
f93594e

try to fix ut failure


jiapinw Feb 16, 2024
ce74dd1

fix missing attribute in telemetry ut


jiapinw Feb 16, 2024
aa21238

fix docstyle


jiapinw Feb 16, 2024
f0c3c64

fix pylint


jiapinw Feb 16, 2024

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


1 change: 1 addition & 0 deletions
src/sagemaker/serve/builder/djl_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -83,6 +83,7 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.image_config = None
 self.vpc_config = None
 self._original_deploy = None
Expand Down

4 changes: 3 additions & 1 deletion
src/sagemaker/serve/builder/jumpstart_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -57,6 +57,8 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.vpc_config = None
@grenmestergrenmesterFeb 16, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the added VPC config for?
@jiapinwjiapinwFeb 16, 2024

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's for feature parity. This config has already exposed to djl/tgi/torchserve/tri
 self._original_deploy = None
 self.secret_key = None
 self.js_model_config = None
Expand DownExpand Up@@ -94,7 +96,7 @@ def _is_jumpstart_model_id(self) -> bool:

 def _create_pre_trained_js_model(self) -> Type[Model]:
 """Placeholder docstring"""
 pysdk_model = JumpStartModel(self.model)
 pysdk_model = JumpStartModel(self.model, vpc_config=self.vpc_config)
 pysdk_model.sagemaker_session = self.sagemaker_session

 self._original_deploy = pysdk_model.deploy
Expand Down

2 changes: 2 additions & 0 deletions
src/sagemaker/serve/builder/model_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -605,6 +605,8 @@ def build(

 self.serve_settings = self._get_serve_setting()

 self._is_custom_image_uri = self.image_uri is None

 if isinstance(self.model, str):
 if self._is_jumpstart_model_id():
 return self._build_for_jumpstart()
Expand Down

1 change: 1 addition & 0 deletions
src/sagemaker/serve/builder/tgi_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -76,6 +76,7 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.image_config = None
 self.vpc_config = None
 self._original_deploy = None
Expand Down

4 changes: 4 additions & 0 deletions
src/sagemaker/serve/builder/transformers_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -56,6 +56,8 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.vpc_config = None
 self._original_deploy = None
 self.hf_model_config = None
 self._default_data_type = None
Expand DownExpand Up@@ -111,6 +113,7 @@ def _create_transformers_model(self) -> Type[Model]:
 py_version=self.py_version,
 transformers_version=base_hf_version,
 pytorch_version=self.pytorch_version,
 vpc_config=self.vpc_config,
 )
 elif "keras" in hf_model_md.get("tags") or "tensorflow" in hf_model_md.get("tags"):
 self.tensorflow_version = self._get_supported_version(
Expand All@@ -126,6 +129,7 @@ def _create_transformers_model(self) -> Type[Model]:
 py_version=self.py_version,
 transformers_version=base_hf_version,
 tensorflow_version=self.tensorflow_version,
 vpc_config=self.vpc_config,
 )

 if self.mode == Mode.LOCAL_CONTAINER:
Expand Down

24 changes: 23 additions & 1 deletion
src/sagemaker/serve/utils/telemetry_logger.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -20,7 +20,8 @@
from sagemaker import Session, exceptions
from sagemaker.serve.mode.function_pointers import Mode
from sagemaker.serve.utils.exceptions import ModelBuilderException
from sagemaker.serve.utils.types import ModelServer
from sagemaker.serve.utils.types import ModelServer, ImageUriOption
from sagemaker.serve.validations.check_image_uri import is_1p_image_uri
from sagemaker.user_agent import SDK_VERSION

logger = logging.getLogger(__name__)
Expand DownExpand Up@@ -62,11 +63,13 @@ def wrapper(self, *args, **kwargs):
 caught_ex = None

 image_uri_tail = self.image_uri.split("/")[1]
 image_uri_option = _get_image_uri_option(self.image_uri, self._is_custom_image_uri)
 extra = (
 f"{func_name}"
 f"&x-modelServer={MODEL_SERVER_TO_CODE[str(self.model_server)]}"
 f"&x-imageTag={image_uri_tail}"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={image_uri_option}"
@gwang111gwang111Feb 19, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious, is "default" important to the naming here or can we just go with "imageUsage"
@gwang111gwang111Feb 19, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or "imageType"
@jiapinwjiapinwFeb 19, 2024

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, this metric is to capture whether customers provide their own image or not, the further differentiation on custom image or custom 1p image is not that important.
@gwang111gwang111Feb 19, 2024•

edited

Loading

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack sound good. posed the question from a string len perspective. 1024 is max i believe. So if there are ways to cut down on length without losing the meaningfulness we should try and do so
 )

 if self.model_server == ModelServer.DJL_SERVING or self.model_server == ModelServer.TGI:
Expand DownExpand Up@@ -201,3 +204,22 @@ def _get_region_or_default(session):
 return session.boto_session.region_name
 except Exception: # pylint: disable=W0703
 return "us-west-2"


def _get_image_uri_option(image_uri: str, is_custom_image: bool) -> int:
 """Detect whether default values are used for ModelBuilder

 Args:
 image_uri (str): Image uri used by ModelBuilder.
 is_custom_image: (bool): Boolean indicating whether customer provides with custom image.
 Returns:
 bool: Integer code of image option types.
 """

 if not is_custom_image:
 return ImageUriOption.DEFAULT_IMAGE.value

 if is_1p_image_uri(image_uri):
 return ImageUriOption.CUSTOM_1P_IMAGE.value

 return ImageUriOption.CUSTOM_IMAGE.value

12 changes: 12 additions & 0 deletions
src/sagemaker/serve/utils/types.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -43,3 +43,15 @@ def __str__(self) -> str:
 INFERENTIA_1 = 3
 INFERENTIA_2 = 4
 GRAVITON = 5


class ImageUriOption(Enum):
 """Enum type for image uri options"""

 def __str__(self) -> str:
 """Convert enum to string"""
 return str(self.name)

 CUSTOM_IMAGE = 1
 CUSTOM_1P_IMAGE = 2
 DEFAULT_IMAGE = 3

3 changes: 3 additions & 0 deletions
tests/unit/sagemaker/serve/builder/test_transformers_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -16,6 +16,7 @@
import unittest
from sagemaker.serve.builder.model_builder import ModelBuilder
from sagemaker.serve.mode.function_pointers import Mode
from tests.unit.sagemaker.serve.constants import MOCK_VPC_CONFIG

from sagemaker.serve.utils.predictors import TransformersLocalModePredictor

Expand DownExpand Up@@ -74,6 +75,7 @@ def test_build_deploy_for_transformers_local_container_and_remote_container(
 model=mock_model_id,
 schema_builder=mock_schema_builder,
 mode=Mode.LOCAL_CONTAINER,
 vpc_config=MOCK_VPC_CONFIG,
 )

 builder._prepare_for_mode = MagicMock()
Expand All@@ -85,6 +87,7 @@ def test_build_deploy_for_transformers_local_container_and_remote_container(
 builder.modes[str(Mode.LOCAL_CONTAINER)] = MagicMock()
 predictor = model.deploy(model_data_download_timeout=1800)

 assert model.vpc_config == MOCK_VPC_CONFIG
 assert builder.env_vars["MODEL_LOADING_TIMEOUT"] == "1800"
 assert isinstance(predictor, TransformersLocalModePredictor)

Expand Down

38 changes: 38 additions & 0 deletions
tests/unit/sagemaker/serve/utils/test_telemetry_logger.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -20,6 +20,7 @@
 _construct_url,
)
from sagemaker.serve.utils.exceptions import ModelBuilderException, LocalModelOutOfMemoryException
from sagemaker.serve.utils.types import ImageUriOption
from sagemaker.user_agent import SDK_VERSION

MOCK_SESSION = Mock()
Expand DownExpand Up@@ -71,6 +72,7 @@ def test_capture_telemetry_decorator_djl_success(self, mock_send_telemetry):
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.DJL_SERVING
Expand All@@ -85,6 +87,37 @@ def test_capture_telemetry_decorator_djl_success(self, mock_send_telemetry):
 "&x-modelServer=4"
 "&x-imageTag=djl-inference:0.25.0-deepspeed0.11.0-cu118"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.DEFAULT_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
 )

 mock_send_telemetry.assert_called_once_with(
 "1", 2, MOCK_SESSION, None, None, expected_extra_str
 )

 @patch("sagemaker.serve.utils.telemetry_logger._send_telemetry")
 def test_capture_telemetry_decorator_djl_success_with_custom_image(self, mock_send_telemetry):
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = True
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.DJL_SERVING
 mock_model_builder.sagemaker_session.endpoint_arn = MOCK_ENDPOINT_ARN

 mock_model_builder.mock_deploy()

 args = mock_send_telemetry.call_args.args
 latency = str(args[5]).split("latency=")[1]
 expected_extra_str = (
 f"{MOCK_FUNC_NAME}"
 "&x-modelServer=4"
 "&x-imageTag=djl-inference:0.25.0-deepspeed0.11.0-cu118"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.CUSTOM_1P_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
Expand All@@ -99,6 +132,7 @@ def test_capture_telemetry_decorator_tgi_success(self, mock_send_telemetry):
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_TGI_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.TGI
Expand All@@ -113,6 +147,7 @@ def test_capture_telemetry_decorator_tgi_success(self, mock_send_telemetry):
 "&x-modelServer=6"
 "&x-imageTag=huggingface-pytorch-inference:2.0.0-transformers4.28.1-cpu-py310-ubuntu20.04"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.DEFAULT_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
Expand All@@ -127,6 +162,7 @@ def test_capture_telemetry_decorator_no_call_when_disabled(self, mock_send_telem
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = True
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.model_server = ModelServer.DJL_SERVING

Expand All@@ -139,6 +175,7 @@ def test_capture_telemetry_decorator_handle_exception_success(self, mock_send_te
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.DJL_SERVING
Expand All@@ -158,6 +195,7 @@ def test_capture_telemetry_decorator_handle_exception_success(self, mock_send_te
 "&x-modelServer=4"
 "&x-imageTag=djl-inference:0.25.0-deepspeed0.11.0-cu118"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.DEFAULT_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Remove or un-stick sticky/fixed headers that block content\n(function() {\n function unstick() {\n document.querySelectorAll('header, nav, [role=\"banner\"], .header, .navbar, .sticky, .fixed-top, [style*=\"position: fixed\"], [style*=\"position:sticky\"]').forEach(function(el) {\n if (el.style.position === 'fixed' || el.style.position === 'sticky' || \n getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') {\n el.style.position = 'static';\n el.style.top = 'auto';\n el.style.zIndex = 'auto';\n }\n });\n }\n \n unstick();\n \n var observer = new MutationObserver(unstick);\n observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] });\n})();", "Kill Sticky Headers");
}
} catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); }
})();
(function(){
try {
var __m = "*";
var __re = new RegExp('^' + ".*" + '

Skip to content
Add telemetry metrics on usage of default images for ModelBuilder#4430

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

akrishna1995
merged 6 commits into

from

Feb 21, 2024
 Merged


Add telemetry metrics on usage of default images for ModelBuilder
#4430
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
ce87772

Initial commit


jiapinw Feb 15, 2024
334ce76

Fix formatting and add uts


jiapinw Feb 16, 2024
f93594e

try to fix ut failure


jiapinw Feb 16, 2024
ce74dd1

fix missing attribute in telemetry ut


jiapinw Feb 16, 2024
aa21238

fix docstyle


jiapinw Feb 16, 2024
f0c3c64

fix pylint


jiapinw Feb 16, 2024

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


1 change: 1 addition & 0 deletions
src/sagemaker/serve/builder/djl_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -83,6 +83,7 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.image_config = None
 self.vpc_config = None
 self._original_deploy = None
Expand Down

4 changes: 3 additions & 1 deletion
src/sagemaker/serve/builder/jumpstart_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -57,6 +57,8 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.vpc_config = None
@grenmestergrenmesterFeb 16, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the added VPC config for?
@jiapinwjiapinwFeb 16, 2024

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's for feature parity. This config has already exposed to djl/tgi/torchserve/tri
 self._original_deploy = None
 self.secret_key = None
 self.js_model_config = None
Expand DownExpand Up@@ -94,7 +96,7 @@ def _is_jumpstart_model_id(self) -> bool:

 def _create_pre_trained_js_model(self) -> Type[Model]:
 """Placeholder docstring"""
 pysdk_model = JumpStartModel(self.model)
 pysdk_model = JumpStartModel(self.model, vpc_config=self.vpc_config)
 pysdk_model.sagemaker_session = self.sagemaker_session

 self._original_deploy = pysdk_model.deploy
Expand Down

2 changes: 2 additions & 0 deletions
src/sagemaker/serve/builder/model_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -605,6 +605,8 @@ def build(

 self.serve_settings = self._get_serve_setting()

 self._is_custom_image_uri = self.image_uri is None

 if isinstance(self.model, str):
 if self._is_jumpstart_model_id():
 return self._build_for_jumpstart()
Expand Down

1 change: 1 addition & 0 deletions
src/sagemaker/serve/builder/tgi_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -76,6 +76,7 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.image_config = None
 self.vpc_config = None
 self._original_deploy = None
Expand Down

4 changes: 4 additions & 0 deletions
src/sagemaker/serve/builder/transformers_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -56,6 +56,8 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.vpc_config = None
 self._original_deploy = None
 self.hf_model_config = None
 self._default_data_type = None
Expand DownExpand Up@@ -111,6 +113,7 @@ def _create_transformers_model(self) -> Type[Model]:
 py_version=self.py_version,
 transformers_version=base_hf_version,
 pytorch_version=self.pytorch_version,
 vpc_config=self.vpc_config,
 )
 elif "keras" in hf_model_md.get("tags") or "tensorflow" in hf_model_md.get("tags"):
 self.tensorflow_version = self._get_supported_version(
Expand All@@ -126,6 +129,7 @@ def _create_transformers_model(self) -> Type[Model]:
 py_version=self.py_version,
 transformers_version=base_hf_version,
 tensorflow_version=self.tensorflow_version,
 vpc_config=self.vpc_config,
 )

 if self.mode == Mode.LOCAL_CONTAINER:
Expand Down

24 changes: 23 additions & 1 deletion
src/sagemaker/serve/utils/telemetry_logger.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -20,7 +20,8 @@
from sagemaker import Session, exceptions
from sagemaker.serve.mode.function_pointers import Mode
from sagemaker.serve.utils.exceptions import ModelBuilderException
from sagemaker.serve.utils.types import ModelServer
from sagemaker.serve.utils.types import ModelServer, ImageUriOption
from sagemaker.serve.validations.check_image_uri import is_1p_image_uri
from sagemaker.user_agent import SDK_VERSION

logger = logging.getLogger(__name__)
Expand DownExpand Up@@ -62,11 +63,13 @@ def wrapper(self, *args, **kwargs):
 caught_ex = None

 image_uri_tail = self.image_uri.split("/")[1]
 image_uri_option = _get_image_uri_option(self.image_uri, self._is_custom_image_uri)
 extra = (
 f"{func_name}"
 f"&x-modelServer={MODEL_SERVER_TO_CODE[str(self.model_server)]}"
 f"&x-imageTag={image_uri_tail}"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={image_uri_option}"
@gwang111gwang111Feb 19, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious, is "default" important to the naming here or can we just go with "imageUsage"
@gwang111gwang111Feb 19, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or "imageType"
@jiapinwjiapinwFeb 19, 2024

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, this metric is to capture whether customers provide their own image or not, the further differentiation on custom image or custom 1p image is not that important.
@gwang111gwang111Feb 19, 2024•

edited

Loading

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack sound good. posed the question from a string len perspective. 1024 is max i believe. So if there are ways to cut down on length without losing the meaningfulness we should try and do so
 )

 if self.model_server == ModelServer.DJL_SERVING or self.model_server == ModelServer.TGI:
Expand DownExpand Up@@ -201,3 +204,22 @@ def _get_region_or_default(session):
 return session.boto_session.region_name
 except Exception: # pylint: disable=W0703
 return "us-west-2"


def _get_image_uri_option(image_uri: str, is_custom_image: bool) -> int:
 """Detect whether default values are used for ModelBuilder

 Args:
 image_uri (str): Image uri used by ModelBuilder.
 is_custom_image: (bool): Boolean indicating whether customer provides with custom image.
 Returns:
 bool: Integer code of image option types.
 """

 if not is_custom_image:
 return ImageUriOption.DEFAULT_IMAGE.value

 if is_1p_image_uri(image_uri):
 return ImageUriOption.CUSTOM_1P_IMAGE.value

 return ImageUriOption.CUSTOM_IMAGE.value

12 changes: 12 additions & 0 deletions
src/sagemaker/serve/utils/types.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -43,3 +43,15 @@ def __str__(self) -> str:
 INFERENTIA_1 = 3
 INFERENTIA_2 = 4
 GRAVITON = 5


class ImageUriOption(Enum):
 """Enum type for image uri options"""

 def __str__(self) -> str:
 """Convert enum to string"""
 return str(self.name)

 CUSTOM_IMAGE = 1
 CUSTOM_1P_IMAGE = 2
 DEFAULT_IMAGE = 3

3 changes: 3 additions & 0 deletions
tests/unit/sagemaker/serve/builder/test_transformers_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -16,6 +16,7 @@
import unittest
from sagemaker.serve.builder.model_builder import ModelBuilder
from sagemaker.serve.mode.function_pointers import Mode
from tests.unit.sagemaker.serve.constants import MOCK_VPC_CONFIG

from sagemaker.serve.utils.predictors import TransformersLocalModePredictor

Expand DownExpand Up@@ -74,6 +75,7 @@ def test_build_deploy_for_transformers_local_container_and_remote_container(
 model=mock_model_id,
 schema_builder=mock_schema_builder,
 mode=Mode.LOCAL_CONTAINER,
 vpc_config=MOCK_VPC_CONFIG,
 )

 builder._prepare_for_mode = MagicMock()
Expand All@@ -85,6 +87,7 @@ def test_build_deploy_for_transformers_local_container_and_remote_container(
 builder.modes[str(Mode.LOCAL_CONTAINER)] = MagicMock()
 predictor = model.deploy(model_data_download_timeout=1800)

 assert model.vpc_config == MOCK_VPC_CONFIG
 assert builder.env_vars["MODEL_LOADING_TIMEOUT"] == "1800"
 assert isinstance(predictor, TransformersLocalModePredictor)

Expand Down

38 changes: 38 additions & 0 deletions
tests/unit/sagemaker/serve/utils/test_telemetry_logger.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -20,6 +20,7 @@
 _construct_url,
)
from sagemaker.serve.utils.exceptions import ModelBuilderException, LocalModelOutOfMemoryException
from sagemaker.serve.utils.types import ImageUriOption
from sagemaker.user_agent import SDK_VERSION

MOCK_SESSION = Mock()
Expand DownExpand Up@@ -71,6 +72,7 @@ def test_capture_telemetry_decorator_djl_success(self, mock_send_telemetry):
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.DJL_SERVING
Expand All@@ -85,6 +87,37 @@ def test_capture_telemetry_decorator_djl_success(self, mock_send_telemetry):
 "&x-modelServer=4"
 "&x-imageTag=djl-inference:0.25.0-deepspeed0.11.0-cu118"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.DEFAULT_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
 )

 mock_send_telemetry.assert_called_once_with(
 "1", 2, MOCK_SESSION, None, None, expected_extra_str
 )

 @patch("sagemaker.serve.utils.telemetry_logger._send_telemetry")
 def test_capture_telemetry_decorator_djl_success_with_custom_image(self, mock_send_telemetry):
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = True
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.DJL_SERVING
 mock_model_builder.sagemaker_session.endpoint_arn = MOCK_ENDPOINT_ARN

 mock_model_builder.mock_deploy()

 args = mock_send_telemetry.call_args.args
 latency = str(args[5]).split("latency=")[1]
 expected_extra_str = (
 f"{MOCK_FUNC_NAME}"
 "&x-modelServer=4"
 "&x-imageTag=djl-inference:0.25.0-deepspeed0.11.0-cu118"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.CUSTOM_1P_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
Expand All@@ -99,6 +132,7 @@ def test_capture_telemetry_decorator_tgi_success(self, mock_send_telemetry):
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_TGI_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.TGI
Expand All@@ -113,6 +147,7 @@ def test_capture_telemetry_decorator_tgi_success(self, mock_send_telemetry):
 "&x-modelServer=6"
 "&x-imageTag=huggingface-pytorch-inference:2.0.0-transformers4.28.1-cpu-py310-ubuntu20.04"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.DEFAULT_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
Expand All@@ -127,6 +162,7 @@ def test_capture_telemetry_decorator_no_call_when_disabled(self, mock_send_telem
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = True
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.model_server = ModelServer.DJL_SERVING

Expand All@@ -139,6 +175,7 @@ def test_capture_telemetry_decorator_handle_exception_success(self, mock_send_te
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.DJL_SERVING
Expand All@@ -158,6 +195,7 @@ def test_capture_telemetry_decorator_handle_exception_success(self, mock_send_te
 "&x-modelServer=4"
 "&x-imageTag=djl-inference:0.25.0-deepspeed0.11.0-cu118"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.DEFAULT_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
Expand Down

, 'i');
if (__m === '*' || __re.test(location.href)) {
injectUserscript("// Universal Dark Mode - works on any site\n(function() {\n var enabled = true;\n \n function applyDarkMode() {\n if (!enabled) return;\n \n // Create style element if it doesn't exist\n var style = document.getElementById('universal-dark-mode-style');\n if (!style) {\n style = document.createElement('style');\n style.id = 'universal-dark-mode-style';\n document.head.appendChild(style);\n }\n \n // Dark mode CSS - inverts colors but preserves images/video\n style.textContent = '\n /* Invert everything except media */\n html {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #1a1a2e !important;\n }\n \n /* Restore images, videos, iframes, canvas */\n img, video, iframe, canvas, svg, picture, [style*=\"background-image\"] {\n filter: invert(1) hue-rotate(180deg) !important;\n }\n \n /* Preserve specific elements that should not be inverted */\n .no-dark-mode, .no-dark-mode *,\n [data-theme=\"light\"], [data-theme=\"light\"],\n .ace_editor, .ace_editor *,\n .CodeMirror, .CodeMirror *,\n .monaco-editor, .monaco-editor *,\n .markdown-body pre, .markdown-body pre *,\n .highlight, .highlight *,\n pre code, pre code * {\n filter: none !important;\n }\n \n /* Fix common UI elements */\n .modal, .popup, .dropdown-menu, .tooltip, .popover {\n filter: invert(1) hue-rotate(180deg) !important;\n background: #2d2d44 !important;\n border-color: #444 !important;\n }\n \n /* Scrollbars */\n ::-webkit-scrollbar { background: #1a1a2e !important; }\n ::-webkit-scrollbar-thumb { background: #444 !important; }\n ::-webkit-scrollbar-thumb:hover { background: #555 !important; }\n \n /* Selection */\n ::selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ::-moz-selection { background: #4ecdc4 !important; color: #1a1a2e !important; }\n ';\n }\n \n function removeDarkMode() {\n var style = document.getElementById('universal-dark-mode-style');\n if (style) style.remove();\n }\n \n // Toggle with Alt+Shift+D\n document.addEventListener('keydown', function(e) {\n if (e.altKey && e.shiftKey && e.key === 'D') {\n e.preventDefault();\n enabled = !enabled;\n if (enabled) {\n applyDarkMode();\n console.log('[Universal Dark Mode] Enabled');\n } else {\n removeDarkMode();\n console.log('[Universal Dark Mode] Disabled');\n }\n }\n });\n \n // Apply on load\n applyDarkMode();\n \n // Re-apply on dynamic content\n var observer = new MutationObserver(function(mutations) {\n if (enabled && !document.getElementById('universal-dark-mode-style')) {\n applyDarkMode();\n }\n });\n observer.observe(document.head, { childList: true });\n \n console.log('[Universal Dark Mode] Loaded - Press Alt+Shift+D to toggle');\n})();", "Universal Dark Mode");
}
} catch(__e) { console.warn('[Userscript:Universal Dark Mode]', __e); }
})();
})();

Skip to content
Add telemetry metrics on usage of default images for ModelBuilder#4430

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

 Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and
privacy statement. We’ll occasionally send you account related emails.

Already on GitHub?
Sign in
to your account

 Merged

akrishna1995
merged 6 commits into

from

Feb 21, 2024
 Merged


Add telemetry metrics on usage of default images for ModelBuilder
#4430
Show file tree
Hide file tree
 Changes from all commits
Commits

Show all changes


6 commits


Select commit
Hold shift + click to select a range
ce87772

Initial commit


jiapinw Feb 15, 2024
334ce76

Fix formatting and add uts


jiapinw Feb 16, 2024
f93594e

try to fix ut failure


jiapinw Feb 16, 2024
ce74dd1

fix missing attribute in telemetry ut


jiapinw Feb 16, 2024
aa21238

fix docstyle


jiapinw Feb 16, 2024
f0c3c64

fix pylint


jiapinw Feb 16, 2024

File filter

Filter by extension
Filter by extension
Conversations

Failed to load comments. 
Loading
Jump to

Jump to file


Failed to load files. 
Loading
 Diff view

 Diff view


1 change: 1 addition & 0 deletions
src/sagemaker/serve/builder/djl_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -83,6 +83,7 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.image_config = None
 self.vpc_config = None
 self._original_deploy = None
Expand Down

4 changes: 3 additions & 1 deletion
src/sagemaker/serve/builder/jumpstart_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -57,6 +57,8 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.vpc_config = None
@grenmestergrenmesterFeb 16, 2024

Copy link


Copy Markdown

Contributor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

What's the added VPC config for?
@jiapinwjiapinwFeb 16, 2024

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's for feature parity. This config has already exposed to djl/tgi/torchserve/tri
 self._original_deploy = None
 self.secret_key = None
 self.js_model_config = None
Expand DownExpand Up@@ -94,7 +96,7 @@ def _is_jumpstart_model_id(self) -> bool:

 def _create_pre_trained_js_model(self) -> Type[Model]:
 """Placeholder docstring"""
 pysdk_model = JumpStartModel(self.model)
 pysdk_model = JumpStartModel(self.model, vpc_config=self.vpc_config)
 pysdk_model.sagemaker_session = self.sagemaker_session

 self._original_deploy = pysdk_model.deploy
Expand Down

2 changes: 2 additions & 0 deletions
src/sagemaker/serve/builder/model_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -605,6 +605,8 @@ def build(

 self.serve_settings = self._get_serve_setting()

 self._is_custom_image_uri = self.image_uri is None

 if isinstance(self.model, str):
 if self._is_jumpstart_model_id():
 return self._build_for_jumpstart()
Expand Down

1 change: 1 addition & 0 deletions
src/sagemaker/serve/builder/tgi_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -76,6 +76,7 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.image_config = None
 self.vpc_config = None
 self._original_deploy = None
Expand Down

4 changes: 4 additions & 0 deletions
src/sagemaker/serve/builder/transformers_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -56,6 +56,8 @@ def __init__(self):
 self.mode = None
 self.model_server = None
 self.image_uri = None
 self._is_custom_image_uri = False
 self.vpc_config = None
 self._original_deploy = None
 self.hf_model_config = None
 self._default_data_type = None
Expand DownExpand Up@@ -111,6 +113,7 @@ def _create_transformers_model(self) -> Type[Model]:
 py_version=self.py_version,
 transformers_version=base_hf_version,
 pytorch_version=self.pytorch_version,
 vpc_config=self.vpc_config,
 )
 elif "keras" in hf_model_md.get("tags") or "tensorflow" in hf_model_md.get("tags"):
 self.tensorflow_version = self._get_supported_version(
Expand All@@ -126,6 +129,7 @@ def _create_transformers_model(self) -> Type[Model]:
 py_version=self.py_version,
 transformers_version=base_hf_version,
 tensorflow_version=self.tensorflow_version,
 vpc_config=self.vpc_config,
 )

 if self.mode == Mode.LOCAL_CONTAINER:
Expand Down

24 changes: 23 additions & 1 deletion
src/sagemaker/serve/utils/telemetry_logger.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -20,7 +20,8 @@
from sagemaker import Session, exceptions
from sagemaker.serve.mode.function_pointers import Mode
from sagemaker.serve.utils.exceptions import ModelBuilderException
from sagemaker.serve.utils.types import ModelServer
from sagemaker.serve.utils.types import ModelServer, ImageUriOption
from sagemaker.serve.validations.check_image_uri import is_1p_image_uri
from sagemaker.user_agent import SDK_VERSION

logger = logging.getLogger(__name__)
Expand DownExpand Up@@ -62,11 +63,13 @@ def wrapper(self, *args, **kwargs):
 caught_ex = None

 image_uri_tail = self.image_uri.split("/")[1]
 image_uri_option = _get_image_uri_option(self.image_uri, self._is_custom_image_uri)
 extra = (
 f"{func_name}"
 f"&x-modelServer={MODEL_SERVER_TO_CODE[str(self.model_server)]}"
 f"&x-imageTag={image_uri_tail}"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={image_uri_option}"
@gwang111gwang111Feb 19, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

curious, is "default" important to the naming here or can we just go with "imageUsage"
@gwang111gwang111Feb 19, 2024

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

or "imageType"
@jiapinwjiapinwFeb 19, 2024

Copy link


Copy Markdown

ContributorAuthor
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think so, this metric is to capture whether customers provide their own image or not, the further differentiation on custom image or custom 1p image is not that important.
@gwang111gwang111Feb 19, 2024•

edited

Loading

Copy link


Copy Markdown

Collaborator
Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ack sound good. posed the question from a string len perspective. 1024 is max i believe. So if there are ways to cut down on length without losing the meaningfulness we should try and do so
 )

 if self.model_server == ModelServer.DJL_SERVING or self.model_server == ModelServer.TGI:
Expand DownExpand Up@@ -201,3 +204,22 @@ def _get_region_or_default(session):
 return session.boto_session.region_name
 except Exception: # pylint: disable=W0703
 return "us-west-2"


def _get_image_uri_option(image_uri: str, is_custom_image: bool) -> int:
 """Detect whether default values are used for ModelBuilder

 Args:
 image_uri (str): Image uri used by ModelBuilder.
 is_custom_image: (bool): Boolean indicating whether customer provides with custom image.
 Returns:
 bool: Integer code of image option types.
 """

 if not is_custom_image:
 return ImageUriOption.DEFAULT_IMAGE.value

 if is_1p_image_uri(image_uri):
 return ImageUriOption.CUSTOM_1P_IMAGE.value

 return ImageUriOption.CUSTOM_IMAGE.value

12 changes: 12 additions & 0 deletions
src/sagemaker/serve/utils/types.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -43,3 +43,15 @@ def __str__(self) -> str:
 INFERENTIA_1 = 3
 INFERENTIA_2 = 4
 GRAVITON = 5


class ImageUriOption(Enum):
 """Enum type for image uri options"""

 def __str__(self) -> str:
 """Convert enum to string"""
 return str(self.name)

 CUSTOM_IMAGE = 1
 CUSTOM_1P_IMAGE = 2
 DEFAULT_IMAGE = 3

3 changes: 3 additions & 0 deletions
tests/unit/sagemaker/serve/builder/test_transformers_builder.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -16,6 +16,7 @@
import unittest
from sagemaker.serve.builder.model_builder import ModelBuilder
from sagemaker.serve.mode.function_pointers import Mode
from tests.unit.sagemaker.serve.constants import MOCK_VPC_CONFIG

from sagemaker.serve.utils.predictors import TransformersLocalModePredictor

Expand DownExpand Up@@ -74,6 +75,7 @@ def test_build_deploy_for_transformers_local_container_and_remote_container(
 model=mock_model_id,
 schema_builder=mock_schema_builder,
 mode=Mode.LOCAL_CONTAINER,
 vpc_config=MOCK_VPC_CONFIG,
 )

 builder._prepare_for_mode = MagicMock()
Expand All@@ -85,6 +87,7 @@ def test_build_deploy_for_transformers_local_container_and_remote_container(
 builder.modes[str(Mode.LOCAL_CONTAINER)] = MagicMock()
 predictor = model.deploy(model_data_download_timeout=1800)

 assert model.vpc_config == MOCK_VPC_CONFIG
 assert builder.env_vars["MODEL_LOADING_TIMEOUT"] == "1800"
 assert isinstance(predictor, TransformersLocalModePredictor)

Expand Down

38 changes: 38 additions & 0 deletions
tests/unit/sagemaker/serve/utils/test_telemetry_logger.py

View file


Open in desktop

Original file line numberDiff line numberDiff line change
Expand Up@@ -20,6 +20,7 @@
 _construct_url,
)
from sagemaker.serve.utils.exceptions import ModelBuilderException, LocalModelOutOfMemoryException
from sagemaker.serve.utils.types import ImageUriOption
from sagemaker.user_agent import SDK_VERSION

MOCK_SESSION = Mock()
Expand DownExpand Up@@ -71,6 +72,7 @@ def test_capture_telemetry_decorator_djl_success(self, mock_send_telemetry):
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.DJL_SERVING
Expand All@@ -85,6 +87,37 @@ def test_capture_telemetry_decorator_djl_success(self, mock_send_telemetry):
 "&x-modelServer=4"
 "&x-imageTag=djl-inference:0.25.0-deepspeed0.11.0-cu118"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.DEFAULT_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
 )

 mock_send_telemetry.assert_called_once_with(
 "1", 2, MOCK_SESSION, None, None, expected_extra_str
 )

 @patch("sagemaker.serve.utils.telemetry_logger._send_telemetry")
 def test_capture_telemetry_decorator_djl_success_with_custom_image(self, mock_send_telemetry):
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = True
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.DJL_SERVING
 mock_model_builder.sagemaker_session.endpoint_arn = MOCK_ENDPOINT_ARN

 mock_model_builder.mock_deploy()

 args = mock_send_telemetry.call_args.args
 latency = str(args[5]).split("latency=")[1]
 expected_extra_str = (
 f"{MOCK_FUNC_NAME}"
 "&x-modelServer=4"
 "&x-imageTag=djl-inference:0.25.0-deepspeed0.11.0-cu118"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.CUSTOM_1P_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
Expand All@@ -99,6 +132,7 @@ def test_capture_telemetry_decorator_tgi_success(self, mock_send_telemetry):
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_TGI_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.TGI
Expand All@@ -113,6 +147,7 @@ def test_capture_telemetry_decorator_tgi_success(self, mock_send_telemetry):
 "&x-modelServer=6"
 "&x-imageTag=huggingface-pytorch-inference:2.0.0-transformers4.28.1-cpu-py310-ubuntu20.04"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.DEFAULT_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
Expand All@@ -127,6 +162,7 @@ def test_capture_telemetry_decorator_no_call_when_disabled(self, mock_send_telem
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = True
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.model_server = ModelServer.DJL_SERVING

Expand All@@ -139,6 +175,7 @@ def test_capture_telemetry_decorator_handle_exception_success(self, mock_send_te
 mock_model_builder = ModelBuilderMock()
 mock_model_builder.serve_settings.telemetry_opt_out = False
 mock_model_builder.image_uri = MOCK_DJL_CONTAINER
 mock_model_builder._is_custom_image_uri = False
 mock_model_builder.model = MOCK_HUGGINGFACE_ID
 mock_model_builder.mode = Mode.LOCAL_CONTAINER
 mock_model_builder.model_server = ModelServer.DJL_SERVING
Expand All@@ -158,6 +195,7 @@ def test_capture_telemetry_decorator_handle_exception_success(self, mock_send_te
 "&x-modelServer=4"
 "&x-imageTag=djl-inference:0.25.0-deepspeed0.11.0-cu118"
 f"&x-sdkVersion={SDK_VERSION}"
 f"&x-defaultImageUsage={ImageUriOption.DEFAULT_IMAGE.value}"
 f"&x-modelName={MOCK_HUGGINGFACE_ID}"
 f"&x-endpointArn={MOCK_ENDPOINT_ARN}"
 f"&x-latency={latency}"
Expand Down