diff --git a/samcli/commands/_utils/custom_options/hook_name_option.py b/samcli/commands/_utils/custom_options/hook_name_option.py index af730ee2f5..eb333b37bd 100644 --- a/samcli/commands/_utils/custom_options/hook_name_option.py +++ b/samcli/commands/_utils/custom_options/hook_name_option.py @@ -4,6 +4,7 @@ import logging import os +from typing import Any, Mapping import click @@ -18,9 +19,12 @@ ) from samcli.lib.hook.exceptions import InvalidHookWrapperException from samcli.lib.hook.hook_wrapper import IacHookWrapper, get_available_hook_packages_ids +from samcli.lib.telemetry.event import EventName, EventTracker LOG = logging.getLogger(__name__) +PLAN_FILE_OPTION = "terraform_plan_file" + class HookNameOption(click.Option): """ @@ -66,6 +70,8 @@ def handle_parse_result(self, ctx, opts, args): # capture exceptions from prepare hook to emit in track_command c = Context.get_current_context() c.exception = ex + finally: + record_hook_telemetry(opts, ctx) return super().handle_parse_result(ctx, opts, args) @@ -215,3 +221,19 @@ def _read_parameter_value(param_name, opts, ctx, default_value=None): Read SAM CLI parameter value either from the parameters list or from the samconfig values """ return opts.get(param_name, ctx.default_map.get(param_name, default_value)) + + +def record_hook_telemetry(opts: Mapping[str, Any], ctx: click.Context): + """ + Emit metrics related to hooks based on the options passed into the command + + Parameters + ---------- + opts: Mapping[str, Any] + Mapping between a command line option and its value + ctx: Context + Command context properties + """ + plan_file_param = _read_parameter_value(PLAN_FILE_OPTION, opts, ctx) + if plan_file_param: + EventTracker.track_event(EventName.HOOK_CONFIGURATIONS_USED.value, "TerraformPlanFile") diff --git a/samcli/lib/telemetry/event.py b/samcli/lib/telemetry/event.py index 2c35748951..96e5fe45cb 100644 --- a/samcli/lib/telemetry/event.py +++ b/samcli/lib/telemetry/event.py @@ -28,6 +28,7 @@ class EventName(Enum): SYNC_FLOW_END = "SyncFlowEnd" BUILD_WORKFLOW_USED = "BuildWorkflowUsed" CONFIG_FILE_EXTENSION = "SamConfigFileExtension" + HOOK_CONFIGURATIONS_USED = "HookConfigurationsUsed" class UsedFeature(Enum): @@ -61,6 +62,10 @@ class EventType: ] _WORKFLOWS = [f"{config.language}-{config.dependency_manager}" for config in ALL_CONFIGS] + _HOOK_CONFIGURATIONS = [ + "TerraformPlanFile", + ] + _event_values = { # Contains allowable values for Events EventName.USED_FEATURE: [event.value for event in UsedFeature], EventName.BUILD_FUNCTION_RUNTIME: INIT_RUNTIMES, @@ -72,6 +77,7 @@ class EventType: EventName.SYNC_FLOW_END: _SYNC_FLOWS, EventName.BUILD_WORKFLOW_USED: _WORKFLOWS, EventName.CONFIG_FILE_EXTENSION: list(FILE_MANAGER_MAPPER.keys()), + EventName.HOOK_CONFIGURATIONS_USED: _HOOK_CONFIGURATIONS, } @staticmethod @@ -148,6 +154,7 @@ class EventTracker: _events: List[Event] = [] _event_lock = threading.Lock() _session_id: Optional[str] = None + _command_name: Optional[str] = None MAX_EVENTS: int = 50 # Maximum number of events to store before sending @@ -205,8 +212,9 @@ def track_event( Event(event_name, event_value, thread_id=thread_id, exception_name=exception_name) ) - # Get the session ID (needed for multithreading sending) - EventTracker._set_session_id() + # Get properties from the click context (needed for multithreading sending) + EventTracker._set_context_property("_session_id", "session_id") + EventTracker._set_context_property("_command_name", "command_path") if len(EventTracker._events) >= EventTracker.MAX_EVENTS: should_send = True @@ -235,17 +243,27 @@ def send_events() -> threading.Thread: return send_thread @staticmethod - def _set_session_id() -> None: + def _set_context_property(event_prop: str, context_prop: str) -> None: """ - Get the session ID from click and save it locally. + Set a click context property in the event so that it is emitted when the metric is sent. + This is required since the event is sent in a separate thread and no longer has access + to the click context that the command was initially called with. As a workaround, we set + the property here first so that it's available when calling the metrics endpoint. + + Parameters + ---------- + event_prop: str + Property name to be stored in the event and consumed when emitting the metric + context_prop: str + Property name for the target property from the context object """ - if not EventTracker._session_id: + if not getattr(EventTracker, event_prop): try: ctx = Context.get_current_context() if ctx: - EventTracker._session_id = ctx.session_id + setattr(EventTracker, event_prop, getattr(ctx, context_prop)) except RuntimeError: - LOG.debug("EventTracker: Unable to obtain session ID") + LOG.debug("EventTracker: Unable to obtain %s", context_prop) @staticmethod def _send_events_in_thread(): @@ -264,6 +282,7 @@ def _send_events_in_thread(): telemetry = Telemetry() metric = Metric("events") metric.add_data("sessionId", EventTracker._session_id) + metric.add_data("commandName", EventTracker._command_name) metric.add_data("metricSpecificAttributes", msa) telemetry.emit(metric) diff --git a/tests/unit/commands/_utils/custom_options/test_hook_package_id_option.py b/tests/unit/commands/_utils/custom_options/test_hook_package_id_option.py index ad6656fdd2..5675d5e772 100644 --- a/tests/unit/commands/_utils/custom_options/test_hook_package_id_option.py +++ b/tests/unit/commands/_utils/custom_options/test_hook_package_id_option.py @@ -1,10 +1,10 @@ from unittest import TestCase -from unittest.mock import MagicMock, patch +from unittest.mock import MagicMock, patch, Mock import os import click -from samcli.commands._utils.custom_options.hook_name_option import HookNameOption +from samcli.commands._utils.custom_options.hook_name_option import HookNameOption, record_hook_telemetry from samcli.lib.hook.exceptions import InvalidHookWrapperException @@ -68,12 +68,18 @@ def test_invalid_coexist_options(self, iac_hook_wrapper_mock): f"Parameters hook-name, and {','.join(self.invalid_coexist_options)} cannot be used together", ) + @patch("samcli.commands._utils.custom_options.hook_name_option.record_hook_telemetry") @patch("samcli.commands._utils.custom_options.hook_name_option.update_experimental_context") @patch("samcli.commands._utils.custom_options.hook_name_option.prompt_experimental") @patch("samcli.commands._utils.custom_options.hook_name_option.os.getcwd") @patch("samcli.commands._utils.custom_options.hook_name_option.IacHookWrapper") def test_valid_hook_package_with_only_hook_id_option( - self, iac_hook_wrapper_mock, getcwd_mock, prompt_experimental_mock, update_experimental_context_mock + self, + iac_hook_wrapper_mock, + getcwd_mock, + prompt_experimental_mock, + update_experimental_context_mock, + record_hook_telemetry_mock, ): iac_hook_wrapper_mock.return_value = self.iac_hook_wrapper_instance_mock prompt_experimental_mock.return_value = True @@ -104,12 +110,18 @@ def test_valid_hook_package_with_only_hook_id_option( ) self.assertEqual(opts.get("template_file"), self.metadata_path) + @patch("samcli.commands._utils.custom_options.hook_name_option.record_hook_telemetry") @patch("samcli.commands._utils.custom_options.hook_name_option.update_experimental_context") @patch("samcli.commands._utils.custom_options.hook_name_option.prompt_experimental") @patch("samcli.commands._utils.custom_options.hook_name_option.os.getcwd") @patch("samcli.commands._utils.custom_options.hook_name_option.IacHookWrapper") def test_valid_hook_package_with_other_options( - self, iac_hook_wrapper_mock, getcwd_mock, prompt_experimental_mock, update_experimental_context_mock + self, + iac_hook_wrapper_mock, + getcwd_mock, + prompt_experimental_mock, + update_experimental_context_mock, + record_hook_telemetry_mock, ): iac_hook_wrapper_mock.return_value = self.iac_hook_wrapper_instance_mock prompt_experimental_mock.return_value = True @@ -145,13 +157,20 @@ def test_valid_hook_package_with_other_options( "/path/path", ) self.assertEqual(opts.get("template_file"), self.metadata_path) + record_hook_telemetry_mock.assert_called_once() + @patch("samcli.commands._utils.custom_options.hook_name_option.record_hook_telemetry") @patch("samcli.commands._utils.custom_options.hook_name_option.update_experimental_context") @patch("samcli.commands._utils.custom_options.hook_name_option.prompt_experimental") @patch("samcli.commands._utils.custom_options.hook_name_option.os.getcwd") @patch("samcli.commands._utils.custom_options.hook_name_option.IacHookWrapper") def test_valid_hook_package_with_other_options_from_sam_config( - self, iac_hook_wrapper_mock, getcwd_mock, prompt_experimental_mock, update_experimental_context_mock + self, + iac_hook_wrapper_mock, + getcwd_mock, + prompt_experimental_mock, + update_experimental_context_mock, + record_hook_telemetry_mock, ): iac_hook_wrapper_mock.return_value = self.iac_hook_wrapper_instance_mock prompt_experimental_mock.return_value = True @@ -188,6 +207,7 @@ def test_valid_hook_package_with_other_options_from_sam_config( ) self.assertEqual(opts.get("template_file"), self.metadata_path) + @patch("samcli.commands._utils.custom_options.hook_name_option.record_hook_telemetry") @patch("samcli.commands._utils.custom_options.hook_name_option.update_experimental_context") @patch("samcli.commands._utils.custom_options.hook_name_option.prompt_experimental") @patch("samcli.commands._utils.custom_options.hook_name_option.os.getcwd") @@ -200,6 +220,7 @@ def test_valid_hook_package_with_skipping_prepare_hook_and_built_path_exists( getcwd_mock, prompt_experimental_mock, update_experimental_context_mock, + record_hook_telemetry_mock, ): iac_hook_wrapper_mock.return_value = self.iac_hook_wrapper_instance_mock prompt_experimental_mock.return_value = True @@ -222,6 +243,7 @@ def test_valid_hook_package_with_skipping_prepare_hook_and_built_path_exists( self.iac_hook_wrapper_instance_mock.prepare.assert_not_called() self.assertEqual(opts.get("template_file"), None) + @patch("samcli.commands._utils.custom_options.hook_name_option.record_hook_telemetry") @patch("samcli.commands._utils.custom_options.hook_name_option.GlobalConfig") @patch("samcli.commands._utils.custom_options.hook_name_option.update_experimental_context") @patch("samcli.commands._utils.custom_options.hook_name_option.prompt_experimental") @@ -236,6 +258,7 @@ def test_valid_hook_package_with_disable_terraform_beta_feature( prompt_experimental_mock, update_experimental_context_mock, global_config_mock, + record_hook_telemetry_mock, ): iac_hook_wrapper_mock.return_value = self.iac_hook_wrapper_instance_mock prompt_experimental_mock.return_value = False @@ -260,6 +283,7 @@ def test_valid_hook_package_with_disable_terraform_beta_feature( self.iac_hook_wrapper_instance_mock.prepare.assert_not_called() self.assertEqual(opts.get("template_file"), None) + @patch("samcli.commands._utils.custom_options.hook_name_option.record_hook_telemetry") @patch("samcli.commands._utils.custom_options.hook_name_option.update_experimental_context") @patch("samcli.commands._utils.custom_options.hook_name_option.prompt_experimental") @patch("samcli.commands._utils.custom_options.hook_name_option.os.getcwd") @@ -272,6 +296,7 @@ def test_valid_hook_package_with_no_beta_feature_option( getcwd_mock, prompt_experimental_mock, update_experimental_context_mock, + record_hook_telemetry_mock, ): iac_hook_wrapper_mock.return_value = self.iac_hook_wrapper_instance_mock prompt_experimental_mock.return_value = False @@ -294,6 +319,7 @@ def test_valid_hook_package_with_no_beta_feature_option( self.iac_hook_wrapper_instance_mock.prepare.assert_not_called() self.assertEqual(opts.get("template_file"), None) + @patch("samcli.commands._utils.custom_options.hook_name_option.record_hook_telemetry") @patch("samcli.commands._utils.custom_options.hook_name_option.update_experimental_context") @patch("samcli.commands._utils.custom_options.hook_name_option.prompt_experimental") @patch("samcli.commands._utils.custom_options.hook_name_option.os.getcwd") @@ -306,6 +332,7 @@ def test_valid_hook_package_with_beta_feature_option( getcwd_mock, prompt_experimental_mock, update_experimental_context_mock, + record_hook_telemetry_mock, ): iac_hook_wrapper_mock.return_value = self.iac_hook_wrapper_instance_mock prompt_experimental_mock.return_value = False @@ -338,6 +365,7 @@ def test_valid_hook_package_with_beta_feature_option( ) self.assertEqual(opts.get("template_file"), self.metadata_path) + @patch("samcli.commands._utils.custom_options.hook_name_option.record_hook_telemetry") @patch("samcli.commands._utils.custom_options.hook_name_option.update_experimental_context") @patch("samcli.commands._utils.custom_options.hook_name_option.prompt_experimental") @patch("samcli.commands._utils.custom_options.hook_name_option.os.getcwd") @@ -350,6 +378,7 @@ def test_valid_hook_package_with_beta_feature_option_in_sam_config( getcwd_mock, prompt_experimental_mock, update_experimental_context_mock, + record_hook_telemetry_mock, ): metadata_path = "path/metadata.json" cwd_path = "path/current" @@ -387,6 +416,7 @@ def test_valid_hook_package_with_beta_feature_option_in_sam_config( ) self.assertEqual(opts.get("template_file"), metadata_path) + @patch("samcli.commands._utils.custom_options.hook_name_option.record_hook_telemetry") @patch("samcli.commands._utils.custom_options.hook_name_option.GlobalConfig") @patch("samcli.commands._utils.custom_options.hook_name_option.update_experimental_context") @patch("samcli.commands._utils.custom_options.hook_name_option.prompt_experimental") @@ -401,6 +431,7 @@ def test_valid_hook_package_with_beta_feature_option_in_environment_variable( prompt_experimental_mock, update_experimental_context_mock, global_config_mock, + record_hook_telemetry_mock, ): metadata_path = "path/metadata.json" cwd_path = "path/current" @@ -441,6 +472,7 @@ def test_valid_hook_package_with_beta_feature_option_in_environment_variable( ) self.assertEqual(opts.get("template_file"), metadata_path) + @patch("samcli.commands._utils.custom_options.hook_name_option.record_hook_telemetry") @patch("samcli.commands._utils.custom_options.hook_name_option.update_experimental_context") @patch("samcli.commands._utils.custom_options.hook_name_option.prompt_experimental") @patch("samcli.commands._utils.custom_options.hook_name_option.os.getcwd") @@ -453,6 +485,7 @@ def test_valid_hook_package_with_skipping_prepare_hook_and_built_path_does_not_e getcwd_mock, prompt_experimental_mock, update_experimental_context_mock, + record_hook_telemetry_mock, ): iac_hook_wrapper_mock.return_value = self.iac_hook_wrapper_instance_mock prompt_experimental_mock.return_value = True @@ -485,6 +518,7 @@ def test_valid_hook_package_with_skipping_prepare_hook_and_built_path_does_not_e ) self.assertEqual(opts.get("template_file"), self.metadata_path) + @patch("samcli.commands._utils.custom_options.hook_name_option.record_hook_telemetry") @patch("samcli.commands._utils.custom_options.hook_name_option.update_experimental_context") @patch("samcli.commands._utils.custom_options.hook_name_option.prompt_experimental") @patch("samcli.commands._utils.custom_options.hook_name_option.os.getcwd") @@ -497,6 +531,7 @@ def test_valid_hook_package_with_use_container_and_build_image( getcwd_mock, prompt_experimental_mock, update_experimental_context_mock, + record_hook_telemetry_mock, ): iac_hook_wrapper_mock.return_value = self.iac_hook_wrapper_instance_mock prompt_experimental_mock.return_value = True @@ -532,6 +567,7 @@ def test_valid_hook_package_with_use_container_and_build_image( ) self.assertEqual(opts.get("template_file"), self.metadata_path) + @patch("samcli.commands._utils.custom_options.hook_name_option.record_hook_telemetry") @patch("samcli.commands._utils.custom_options.hook_name_option.update_experimental_context") @patch("samcli.commands._utils.custom_options.hook_name_option.prompt_experimental") @patch("samcli.commands._utils.custom_options.hook_name_option.os.getcwd") @@ -544,6 +580,7 @@ def test_invalid_hook_package_with_use_container_and_no_build_image( getcwd_mock, prompt_experimental_mock, update_experimental_context_mock, + record_hook_telemetry_mock, ): iac_hook_wrapper_mock.return_value = self.iac_hook_wrapper_instance_mock prompt_experimental_mock.return_value = True @@ -570,6 +607,7 @@ def test_invalid_hook_package_with_use_container_and_no_build_image( ): hook_name_option.handle_parse_result(ctx, opts, args) + @patch("samcli.commands._utils.custom_options.hook_name_option.record_hook_telemetry") @patch("samcli.commands._utils.custom_options.hook_name_option.update_experimental_context") @patch("samcli.commands._utils.custom_options.hook_name_option.prompt_experimental") @patch("samcli.commands._utils.custom_options.hook_name_option.os.getcwd") @@ -580,6 +618,7 @@ def test_invalid_parameter_hook_with_invalid_project_root_directory( getcwd_mock, prompt_experimental_mock, update_experimental_context_mock, + record_hook_telemetry_mock, ): iac_hook_wrapper_mock.return_value = self.iac_hook_wrapper_instance_mock prompt_experimental_mock.return_value = True @@ -606,6 +645,7 @@ def test_invalid_parameter_hook_with_invalid_project_root_directory( ): hook_name_option.handle_parse_result(ctx, opts, args) + @patch("samcli.commands._utils.custom_options.hook_name_option.record_hook_telemetry") @patch("samcli.commands._utils.custom_options.hook_name_option.update_experimental_context") @patch("samcli.commands._utils.custom_options.hook_name_option.prompt_experimental") @patch("samcli.commands._utils.custom_options.hook_name_option.os.getcwd") @@ -620,6 +660,7 @@ def test_valid_parameter_hook_with_valid_absolute_project_root_directory( getcwd_mock, prompt_experimental_mock, update_experimental_context_mock, + record_hook_telemetry_mock, ): iac_hook_wrapper_mock.return_value = self.iac_hook_wrapper_instance_mock prompt_experimental_mock.return_value = True @@ -655,6 +696,7 @@ def test_valid_parameter_hook_with_valid_absolute_project_root_directory( ) self.assertEqual(opts.get("template_file"), self.metadata_path) + @patch("samcli.commands._utils.custom_options.hook_name_option.record_hook_telemetry") @patch("samcli.commands._utils.custom_options.hook_name_option.update_experimental_context") @patch("samcli.commands._utils.custom_options.hook_name_option.prompt_experimental") @patch("samcli.commands._utils.custom_options.hook_name_option.os.getcwd") @@ -669,6 +711,7 @@ def test_valid_parameter_hook_with_valid_relative_project_root_directory( getcwd_mock, prompt_experimental_mock, update_experimental_context_mock, + record_hook_telemetry_mock, ): iac_hook_wrapper_mock.return_value = self.iac_hook_wrapper_instance_mock prompt_experimental_mock.return_value = True @@ -704,6 +747,7 @@ def test_valid_parameter_hook_with_valid_relative_project_root_directory( ) self.assertEqual(opts.get("template_file"), self.metadata_path) + @patch("samcli.commands._utils.custom_options.hook_name_option.record_hook_telemetry") @patch("samcli.commands._utils.custom_options.hook_name_option.update_experimental_context") @patch("samcli.commands._utils.custom_options.hook_name_option.prompt_experimental") @patch("samcli.commands._utils.custom_options.hook_name_option.os.getcwd") @@ -716,6 +760,7 @@ def test_valid_hook_package_with_use_container_false_and_no_build_image( getcwd_mock, prompt_experimental_mock, update_experimental_context_mock, + record_hook_telemetry_mock, ): iac_hook_wrapper_mock.return_value = self.iac_hook_wrapper_instance_mock prompt_experimental_mock.return_value = True @@ -749,3 +794,9 @@ def test_valid_hook_package_with_use_container_false_and_no_build_image( None, ) self.assertEqual(opts.get("template_file"), self.metadata_path) + + @patch("samcli.commands._utils.custom_options.hook_name_option.EventTracker") + def test_record_hook_telemetry(self, event_tracker_mock): + opts = {"terraform_plan_file": "my_plan.json"} + record_hook_telemetry(opts, Mock()) + event_tracker_mock.track_event.assert_called_once_with("HookConfigurationsUsed", "TerraformPlanFile") diff --git a/tests/unit/lib/telemetry/test_event.py b/tests/unit/lib/telemetry/test_event.py index b8dc01fd34..33202303cf 100644 --- a/tests/unit/lib/telemetry/test_event.py +++ b/tests/unit/lib/telemetry/test_event.py @@ -7,6 +7,9 @@ from typing import List, Tuple from unittest import TestCase from unittest.mock import ANY, Mock, patch + +from parameterized import parameterized + from samcli.cli.context import Context from uuid import UUID, uuid4 @@ -163,6 +166,7 @@ def test_events_get_sent(self, telemetry_mock): "ci": ANY, "pyversion": ANY, "samcliVersion": ANY, + "commandName": ANY, "metricSpecificAttributes": { "events": [ { @@ -212,10 +216,44 @@ def test_session_id_set(self, context_mock): context_mock.return_value = mock EventTracker._session_id = None - EventTracker._set_session_id() + EventTracker._set_context_property("_session_id", "session_id") self.assertEqual(EventTracker._session_id, "123") + @patch("samcli.cli.context.Context.get_current_context") + def test_command_name_set(self, context_mock): + mock = Mock() + mock.command_path = "sam cool command" + context_mock.return_value = mock + + EventTracker._command_name = None + EventTracker._set_context_property("_command_name", "command_path") + + self.assertEqual(EventTracker._command_name, "sam cool command") + + @patch("samcli.cli.context.Context.get_current_context") + def test_set_context_properties(self, context_mock): + event_prop = "event_prop" + context_prop = "context_prop" + context_value = "context_value" + + mock = Mock() + setattr(mock, context_prop, context_value) + context_mock.return_value = mock + + setattr(EventTracker, event_prop, None) + EventTracker._set_context_property(event_prop, context_prop) + + self.assertEqual(getattr(EventTracker, event_prop), context_value) + + @patch("samcli.lib.telemetry.event.LOG") + @patch("samcli.cli.context.Context.get_current_context") + def test_throws_handles_context_not_read(self, context_mock, log_mock): + context_mock.side_effect = RuntimeError("Failed") + EventTracker._command_name = None + EventTracker._set_context_property("_command_name", "command_path") + log_mock.debug.assert_called_once_with("EventTracker: Unable to obtain %s", "command_path") + class TestTrackLongEvent(TestCase): @patch("samcli.lib.telemetry.event.EventTracker.send_events")