From a2a833e8eaf228de891d1ef9c973a1eac23e1487 Mon Sep 17 00:00:00 2001 From: Marcus Chok Date: Fri, 8 Nov 2024 14:34:31 -0500 Subject: [PATCH 01/16] draft PR for feedback on instrumentation --- src/snowflake/cli/_app/loggers.py | 2 +- src/snowflake/cli/_app/telemetry.py | 10 +++++++--- .../cli/_plugins/nativeapp/codegen/compiler.py | 2 +- .../_plugins/nativeapp/entities/application.py | 10 ++++++---- .../nativeapp/entities/application_package.py | 2 ++ src/snowflake/cli/_plugins/workspace/manager.py | 5 ++++- src/snowflake/cli/api/cli_global_context.py | 2 +- src/snowflake/cli/api/console/abc.py | 1 + src/snowflake/cli/api/console/console.py | 15 ++++++++++++--- src/snowflake/cli/api/entities/utils.py | 7 ++++++- .../nativeapp/test_feature_metrics.py | 9 ++++++--- 11 files changed, 47 insertions(+), 18 deletions(-) diff --git a/src/snowflake/cli/_app/loggers.py b/src/snowflake/cli/_app/loggers.py index bc2b46838e..6697975a45 100644 --- a/src/snowflake/cli/_app/loggers.py +++ b/src/snowflake/cli/_app/loggers.py @@ -75,7 +75,7 @@ class DefaultLoggingConfig: default_factory=lambda: { "snowflake.cli": LoggerConfig(handlers=["console", "file"]), "snowflake": LoggerConfig(), - "snowflake.connector.telemetry": LoggerConfig(level=logging.CRITICAL), + "snowflake.connector.telemetry": LoggerConfig(), } ) diff --git a/src/snowflake/cli/_app/telemetry.py b/src/snowflake/cli/_app/telemetry.py index 9c48eb49d4..3aa3bcd7c9 100644 --- a/src/snowflake/cli/_app/telemetry.py +++ b/src/snowflake/cli/_app/telemetry.py @@ -67,6 +67,7 @@ class CLITelemetryField(Enum): CONFIG_FEATURE_FLAGS = "config_feature_flags" # Metrics COUNTERS = "counters" + SPANS = "spans" # Information EVENT = "event" ERROR_MSG = "error_msg" @@ -129,9 +130,12 @@ def _get_command_metrics() -> TelemetryDict: cli_context = get_cli_context() return { - CLITelemetryField.COUNTERS: { - **cli_context.metrics.counters, - } + CLITelemetryField.COUNTERS: cli_context.metrics.counters, + CLITelemetryField.SPANS: { + "completed_spans": cli_context.metrics.completed_spans, + "num_spans_past_depth_limit": cli_context.metrics.num_spans_past_depth_limit, + "num_spans_past_total_limit": cli_context.metrics.num_spans_past_total_limit, + }, } diff --git a/src/snowflake/cli/_plugins/nativeapp/codegen/compiler.py b/src/snowflake/cli/_plugins/nativeapp/codegen/compiler.py index 05456503fe..611da5232c 100644 --- a/src/snowflake/cli/_plugins/nativeapp/codegen/compiler.py +++ b/src/snowflake/cli/_plugins/nativeapp/codegen/compiler.py @@ -88,7 +88,7 @@ def compile_artifacts(self): if not self._should_invoke_processors(): return - with cc.phase("Invoking artifact processors"): + with cc.phase("Invoking artifact processors", span_name="compile_artifacts"): if self._bundle_ctx.generated_root.exists(): raise ClickException( f"Path {self._bundle_ctx.generated_root} already exists. Please choose a different name for your generated directory in the project definition file." diff --git a/src/snowflake/cli/_plugins/nativeapp/entities/application.py b/src/snowflake/cli/_plugins/nativeapp/entities/application.py index c9c6295b86..1c831ab16d 100644 --- a/src/snowflake/cli/_plugins/nativeapp/entities/application.py +++ b/src/snowflake/cli/_plugins/nativeapp/entities/application.py @@ -43,6 +43,7 @@ from snowflake.cli._plugins.nativeapp.sf_facade import get_snowflake_facade from snowflake.cli._plugins.nativeapp.utils import needs_confirmation from snowflake.cli._plugins.workspace.context import ActionContext +from snowflake.cli.api.cli_global_context import get_cli_context from snowflake.cli.api.entities.common import EntityBase, get_sql_executor from snowflake.cli.api.entities.utils import ( drop_generic_object, @@ -420,6 +421,7 @@ def get_objects_owned_by_application(self) -> List[ApplicationOwnedObject]: ).fetchall() return [{"name": row[1], "type": row[2]} for row in results] + @get_cli_context().metrics.start_span("create_or_upgrade_app") def create_or_upgrade_app( self, package: ApplicationPackageEntity, @@ -513,10 +515,10 @@ def create_or_upgrade_app( create_cursor = sql_executor.execute_query( dedent( f"""\ - create application {self.name} - from application package {package.name} {using_clause} {debug_mode_clause} - comment = {SPECIAL_COMMENT} - """ + create application {self.name} + from application package {package.name} {using_clause} {debug_mode_clause} + comment = {SPECIAL_COMMENT} + """ ), ) print_messages(console, create_cursor) diff --git a/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py b/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py index 10c6e9550c..c354fb9430 100644 --- a/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py +++ b/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py @@ -43,6 +43,7 @@ from snowflake.cli._plugins.stage.diff import DiffResult from snowflake.cli._plugins.stage.manager import StageManager from snowflake.cli._plugins.workspace.context import ActionContext +from snowflake.cli.api.cli_global_context import get_cli_context from snowflake.cli.api.entities.common import EntityBase, get_sql_executor from snowflake.cli.api.entities.utils import ( drop_generic_object, @@ -915,6 +916,7 @@ def validate_setup_script( if validation_result["status"] == "FAIL": raise SetupScriptFailedValidation() + @get_cli_context().metrics.start_span("get_validation_result") def get_validation_result( self, use_scratch_stage: bool, interactive: bool, force: bool ): diff --git a/src/snowflake/cli/_plugins/workspace/manager.py b/src/snowflake/cli/_plugins/workspace/manager.py index 25b56d542f..d1e525fa66 100644 --- a/src/snowflake/cli/_plugins/workspace/manager.py +++ b/src/snowflake/cli/_plugins/workspace/manager.py @@ -61,7 +61,10 @@ def perform_action(self, entity_id: str, action: EntityActions, *args, **kwargs) action_ctx = ActionContext( get_entity=self.get_entity, ) - return entity.perform(action, action_ctx, *args, **kwargs) + with get_cli_context().metrics.start_span( + f"{type(entity).__name__}.{action.value}" + ): + return entity.perform(action, action_ctx, *args, **kwargs) else: raise ValueError(f'This entity type does not support "{action.value}"') diff --git a/src/snowflake/cli/api/cli_global_context.py b/src/snowflake/cli/api/cli_global_context.py index a703e60299..4ed16cd134 100644 --- a/src/snowflake/cli/api/cli_global_context.py +++ b/src/snowflake/cli/api/cli_global_context.py @@ -157,7 +157,7 @@ def enable_tracebacks(self) -> bool: return self._manager.enable_tracebacks @property - def metrics(self): + def metrics(self) -> CLIMetrics: return self._manager.metrics @property diff --git a/src/snowflake/cli/api/console/abc.py b/src/snowflake/cli/api/console/abc.py index d8b2e3d683..22b1aa2cb5 100644 --- a/src/snowflake/cli/api/console/abc.py +++ b/src/snowflake/cli/api/console/abc.py @@ -68,6 +68,7 @@ def phase( self, enter_message: str, exit_message: Optional[str] = None, + span_name: Optional[str] = None, ) -> Iterator[Callable[[str], None]]: """A context manager for organising steps into logical group.""" diff --git a/src/snowflake/cli/api/console/console.py b/src/snowflake/cli/api/console/console.py index 79031d23a7..97a959092b 100644 --- a/src/snowflake/cli/api/console/console.py +++ b/src/snowflake/cli/api/console/console.py @@ -14,11 +14,12 @@ from __future__ import annotations -from contextlib import contextmanager +from contextlib import contextmanager, nullcontext from typing import Optional from rich.style import Style from rich.text import Text +from snowflake.cli.api.cli_global_context import get_cli_context from snowflake.cli.api.console.abc import AbstractConsole from snowflake.cli.api.console.enum import Output @@ -68,7 +69,12 @@ def _format_message(self, message: str, output: Output) -> Text: return text @contextmanager - def phase(self, enter_message: str, exit_message: Optional[str] = None): + def phase( + self, + enter_message: str, + exit_message: Optional[str] = None, + span_name: Optional[str] = None, + ): """A context manager for organising steps into logical group.""" if self.in_phase: raise CliConsoleNestingProhibitedError("Only one phase allowed at a time.") @@ -81,7 +87,10 @@ def phase(self, enter_message: str, exit_message: Optional[str] = None): self._in_phase = True try: - yield self.step + with get_cli_context().metrics.start_span( + span_name + ) if span_name else nullcontext(): + yield self.step finally: self._in_phase = False if exit_message: diff --git a/src/snowflake/cli/api/entities/utils.py b/src/snowflake/cli/api/entities/utils.py index a8b706b193..c8110fc113 100644 --- a/src/snowflake/cli/api/entities/utils.py +++ b/src/snowflake/cli/api/entities/utils.py @@ -76,6 +76,7 @@ def _get_stage_paths_to_sync( return stage_paths +@get_cli_context().metrics.start_span("sync_deploy_root_with_stage") def sync_deploy_root_with_stage( console: AbstractConsole, deploy_root: Path, @@ -219,7 +220,10 @@ def execute_post_deploy_hooks( get_cli_context().metrics.set_counter(CLICounterField.POST_DEPLOY_SCRIPTS, 1) - with console.phase(f"Executing {deployed_object_type} post-deploy actions"): + with console.phase( + f"Executing {deployed_object_type} post-deploy actions", + span_name=f"{deployed_object_type}.execute_post_deploy_hooks", + ): sql_scripts_paths = [] display_paths = [] for hook in post_deploy_hooks: @@ -304,6 +308,7 @@ def validation_item_to_str(item: dict[str, str | int]): return s +@get_cli_context().metrics.start_span("drop_generic_object") def drop_generic_object( console: AbstractConsole, object_type: str, diff --git a/tests_integration/nativeapp/test_feature_metrics.py b/tests_integration/nativeapp/test_feature_metrics.py index 5ea61226c9..0e85fcca54 100644 --- a/tests_integration/nativeapp/test_feature_metrics.py +++ b/tests_integration/nativeapp/test_feature_metrics.py @@ -15,7 +15,7 @@ from typing import Dict from unittest import mock -from snowflake.cli._app.telemetry import TelemetryEvent +from snowflake.cli._app.telemetry import TelemetryEvent, CLITelemetryField from snowflake.cli.api.metrics import CLICounterField from tests.project.fixtures import * from tests_integration.test_utils import extract_first_telemetry_message_of_type @@ -53,7 +53,10 @@ def test_sql_templating_emits_counter( mock_telemetry, TelemetryEvent.CMD_EXECUTION_RESULT.value ) - assert message["counters"][CLICounterField.SQL_TEMPLATES] == expected_counter + assert ( + message[CLITelemetryField.COUNTERS][CLICounterField.SQL_TEMPLATES] + == expected_counter + ) @pytest.mark.integration @@ -143,4 +146,4 @@ def test_nativeapp_feature_counter_has_expected_value( mock_telemetry, TelemetryEvent.CMD_EXECUTION_RESULT.value ) - assert message["counters"] == expected_counters + assert message[CLITelemetryField.COUNTERS] == expected_counters From 46a30283b8edf898871c1dfbc9944f14805f20b4 Mon Sep 17 00:00:00 2001 From: Marcus Chok Date: Fri, 8 Nov 2024 14:35:28 -0500 Subject: [PATCH 02/16] remove changes on loggers --- src/snowflake/cli/_app/loggers.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/snowflake/cli/_app/loggers.py b/src/snowflake/cli/_app/loggers.py index 6697975a45..bc2b46838e 100644 --- a/src/snowflake/cli/_app/loggers.py +++ b/src/snowflake/cli/_app/loggers.py @@ -75,7 +75,7 @@ class DefaultLoggingConfig: default_factory=lambda: { "snowflake.cli": LoggerConfig(handlers=["console", "file"]), "snowflake": LoggerConfig(), - "snowflake.connector.telemetry": LoggerConfig(), + "snowflake.connector.telemetry": LoggerConfig(level=logging.CRITICAL), } ) From d26af10d34e26bb47740ec2ab11b753b0c99ab5b Mon Sep 17 00:00:00 2001 From: Marcus Chok Date: Fri, 8 Nov 2024 15:26:22 -0500 Subject: [PATCH 03/16] extract enum value for proper key to message dict --- tests_integration/nativeapp/test_feature_metrics.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests_integration/nativeapp/test_feature_metrics.py b/tests_integration/nativeapp/test_feature_metrics.py index 0e85fcca54..271626f393 100644 --- a/tests_integration/nativeapp/test_feature_metrics.py +++ b/tests_integration/nativeapp/test_feature_metrics.py @@ -54,7 +54,7 @@ def test_sql_templating_emits_counter( ) assert ( - message[CLITelemetryField.COUNTERS][CLICounterField.SQL_TEMPLATES] + message[CLITelemetryField.COUNTERS.value][CLICounterField.SQL_TEMPLATES] == expected_counter ) @@ -146,4 +146,4 @@ def test_nativeapp_feature_counter_has_expected_value( mock_telemetry, TelemetryEvent.CMD_EXECUTION_RESULT.value ) - assert message[CLITelemetryField.COUNTERS] == expected_counters + assert message[CLITelemetryField.COUNTERS.value] == expected_counters From 6b3d12ee902caabe18c5a1afb2d6234f7a0a2d54 Mon Sep 17 00:00:00 2001 From: Marcus Chok Date: Mon, 11 Nov 2024 17:19:58 -0500 Subject: [PATCH 04/16] write custom decorator for spans, add tests --- src/snowflake/cli/_app/telemetry.py | 9 +- .../nativeapp/entities/application.py | 4 +- .../nativeapp/entities/application_package.py | 4 +- src/snowflake/cli/api/cli_global_context.py | 21 ++ src/snowflake/cli/api/entities/utils.py | 8 +- .../nativeapp/test_command_metric_counters.py | 248 ++++++++++++++++++ .../nativeapp/test_command_metric_spans.py | 164 ++++++++++++ .../nativeapp/test_feature_metrics.py | 149 ----------- 8 files changed, 447 insertions(+), 160 deletions(-) create mode 100644 tests_integration/nativeapp/test_command_metric_counters.py create mode 100644 tests_integration/nativeapp/test_command_metric_spans.py delete mode 100644 tests_integration/nativeapp/test_feature_metrics.py diff --git a/src/snowflake/cli/_app/telemetry.py b/src/snowflake/cli/_app/telemetry.py index 3aa3bcd7c9..fb7a8597cd 100644 --- a/src/snowflake/cli/_app/telemetry.py +++ b/src/snowflake/cli/_app/telemetry.py @@ -68,6 +68,9 @@ class CLITelemetryField(Enum): # Metrics COUNTERS = "counters" SPANS = "spans" + COMPLETED_SPANS = "completed_spans" + NUM_SPANS_PAST_DEPTH_LIMIT = "num_spans_past_depth_limit" + NUM_SPANS_PAST_TOTAL_LIMIT = "num_spans_past_total_limit" # Information EVENT = "event" ERROR_MSG = "error_msg" @@ -132,9 +135,9 @@ def _get_command_metrics() -> TelemetryDict: return { CLITelemetryField.COUNTERS: cli_context.metrics.counters, CLITelemetryField.SPANS: { - "completed_spans": cli_context.metrics.completed_spans, - "num_spans_past_depth_limit": cli_context.metrics.num_spans_past_depth_limit, - "num_spans_past_total_limit": cli_context.metrics.num_spans_past_total_limit, + CLITelemetryField.COMPLETED_SPANS.value: cli_context.metrics.completed_spans, + CLITelemetryField.NUM_SPANS_PAST_DEPTH_LIMIT.value: cli_context.metrics.num_spans_past_depth_limit, + CLITelemetryField.NUM_SPANS_PAST_TOTAL_LIMIT.value: cli_context.metrics.num_spans_past_total_limit, }, } diff --git a/src/snowflake/cli/_plugins/nativeapp/entities/application.py b/src/snowflake/cli/_plugins/nativeapp/entities/application.py index 1c831ab16d..8eac01eded 100644 --- a/src/snowflake/cli/_plugins/nativeapp/entities/application.py +++ b/src/snowflake/cli/_plugins/nativeapp/entities/application.py @@ -43,7 +43,7 @@ from snowflake.cli._plugins.nativeapp.sf_facade import get_snowflake_facade from snowflake.cli._plugins.nativeapp.utils import needs_confirmation from snowflake.cli._plugins.workspace.context import ActionContext -from snowflake.cli.api.cli_global_context import get_cli_context +from snowflake.cli.api.cli_global_context import start_cli_metrics_span from snowflake.cli.api.entities.common import EntityBase, get_sql_executor from snowflake.cli.api.entities.utils import ( drop_generic_object, @@ -421,7 +421,7 @@ def get_objects_owned_by_application(self) -> List[ApplicationOwnedObject]: ).fetchall() return [{"name": row[1], "type": row[2]} for row in results] - @get_cli_context().metrics.start_span("create_or_upgrade_app") + @start_cli_metrics_span("create_or_upgrade_app") def create_or_upgrade_app( self, package: ApplicationPackageEntity, diff --git a/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py b/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py index c354fb9430..ed8f7e7fff 100644 --- a/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py +++ b/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py @@ -43,7 +43,7 @@ from snowflake.cli._plugins.stage.diff import DiffResult from snowflake.cli._plugins.stage.manager import StageManager from snowflake.cli._plugins.workspace.context import ActionContext -from snowflake.cli.api.cli_global_context import get_cli_context +from snowflake.cli.api.cli_global_context import start_cli_metrics_span from snowflake.cli.api.entities.common import EntityBase, get_sql_executor from snowflake.cli.api.entities.utils import ( drop_generic_object, @@ -916,7 +916,7 @@ def validate_setup_script( if validation_result["status"] == "FAIL": raise SetupScriptFailedValidation() - @get_cli_context().metrics.start_span("get_validation_result") + @start_cli_metrics_span("get_validation_result") def get_validation_result( self, use_scratch_stage: bool, interactive: bool, force: bool ): diff --git a/src/snowflake/cli/api/cli_global_context.py b/src/snowflake/cli/api/cli_global_context.py index 4ed16cd134..f58b576b28 100644 --- a/src/snowflake/cli/api/cli_global_context.py +++ b/src/snowflake/cli/api/cli_global_context.py @@ -17,6 +17,7 @@ from contextlib import contextmanager from contextvars import ContextVar from dataclasses import dataclass, field, replace +from functools import wraps from pathlib import Path from typing import TYPE_CHECKING, Iterator @@ -213,6 +214,26 @@ def get_cli_context() -> _CliGlobalContextAccess: return _CliGlobalContextAccess(get_cli_context_manager()) +def start_cli_metrics_span(span_name: str): + """ + Decorator to start a command metrics span that encompasses a whole function + + Must be used instead of directly calling @get_cli_context().metrics.start_span(span_name) + as a decorator to ensure that the cli context is grabbed at run time instead of at + module load time, which would not reflect forking + """ + + def decorator(func): + @wraps(func) + def wrapper(*args, **kwargs): + with get_cli_context().metrics.start_span(span_name): + return func(*args, **kwargs) + + return wrapper + + return decorator + + @contextmanager def fork_cli_context( connection_overrides: dict | None = None, diff --git a/src/snowflake/cli/api/entities/utils.py b/src/snowflake/cli/api/entities/utils.py index c8110fc113..1eb1719a8c 100644 --- a/src/snowflake/cli/api/entities/utils.py +++ b/src/snowflake/cli/api/entities/utils.py @@ -23,7 +23,7 @@ to_stage_path, ) from snowflake.cli._plugins.stage.utils import print_diff_to_console -from snowflake.cli.api.cli_global_context import get_cli_context +from snowflake.cli.api.cli_global_context import get_cli_context, start_cli_metrics_span from snowflake.cli.api.console.abc import AbstractConsole from snowflake.cli.api.entities.common import get_sql_executor from snowflake.cli.api.errno import ( @@ -76,7 +76,7 @@ def _get_stage_paths_to_sync( return stage_paths -@get_cli_context().metrics.start_span("sync_deploy_root_with_stage") +@start_cli_metrics_span("sync_deploy_root_with_stage") def sync_deploy_root_with_stage( console: AbstractConsole, deploy_root: Path, @@ -222,7 +222,7 @@ def execute_post_deploy_hooks( with console.phase( f"Executing {deployed_object_type} post-deploy actions", - span_name=f"{deployed_object_type}.execute_post_deploy_hooks", + span_name="execute_post_deploy_hooks", ): sql_scripts_paths = [] display_paths = [] @@ -308,7 +308,7 @@ def validation_item_to_str(item: dict[str, str | int]): return s -@get_cli_context().metrics.start_span("drop_generic_object") +@start_cli_metrics_span("drop_generic_object") def drop_generic_object( console: AbstractConsole, object_type: str, diff --git a/tests_integration/nativeapp/test_command_metric_counters.py b/tests_integration/nativeapp/test_command_metric_counters.py new file mode 100644 index 0000000000..8a834473ba --- /dev/null +++ b/tests_integration/nativeapp/test_command_metric_counters.py @@ -0,0 +1,248 @@ +# Copyright (c) 2024 Snowflake Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +from shlex import split +from typing import Dict, Callable +from unittest import mock + +from snowflake.cli._app.telemetry import TelemetryEvent, CLITelemetryField +from snowflake.cli.api.metrics import CLICounterField +from tests.nativeapp.factories import ( + ProjectV11Factory, + ProjectV2Factory, + ApplicationPackageEntityModelFactory, + ApplicationEntityModelFactory, +) +from tests.project.fixtures import * +from tests_common import temp_dir +from tests_integration.test_utils import extract_first_telemetry_message_of_type + + +@pytest.mark.integration +@pytest.mark.parametrize( + "command,expected_counter", + [ + ( + [ + "sql", + "-q", + "select '<% ctx.env.test %>'", + "--env", + "test=value_from_cli", + ], + 1, + ), + (["sql", "-q", "select 'string'"], 0), + ], +) +@mock.patch("snowflake.connector.telemetry.TelemetryClient.try_add_log_to_batch") +def test_sql_templating_emits_counter( + mock_telemetry, + command: List[str], + expected_counter, + runner, +): + result = runner.invoke_with_connection_json(command) + + assert result.exit_code == 0 + + message = extract_first_telemetry_message_of_type( + mock_telemetry, TelemetryEvent.CMD_EXECUTION_RESULT.value + ) + + assert ( + message[CLITelemetryField.COUNTERS.value][CLICounterField.SQL_TEMPLATES] + == expected_counter + ) + + +@pytest.mark.integration +@pytest.mark.parametrize( + "command,project_factory,expected_counters", + [ + ( + "app deploy", + lambda: ProjectV11Factory( + pdf__native_app__artifacts=["README.md", "setup.sql", "manifest.yml"], + pdf__native_app__package__post_deploy=[ + {"sql_script": "post_deploy1.sql"}, + ], + files={ + "README.md": "", + "setup.sql": "select 1", + "manifest.yml": "\n", + "post_deploy1.sql": "\n", + }, + ), + { + CLICounterField.SNOWPARK_PROCESSOR: 0, + CLICounterField.TEMPLATES_PROCESSOR: 0, + CLICounterField.PDF_TEMPLATES: 0, + CLICounterField.POST_DEPLOY_SCRIPTS: 1, + CLICounterField.PACKAGE_SCRIPTS: 0, + }, + ), + ( + "ws bundle --entity-id=pkg", + lambda: ProjectV2Factory( + pdf__entities=dict( + pkg=ApplicationPackageEntityModelFactory( + identifier="myapp_pkg_<% ctx.env.USER %>", + artifacts=[ + "setup.sql", + "README.md", + "manifest.yml", + # just needs to have the templates processor to nest phases + {"src": "app/*", "dest": "./", "processors": ["templates"]}, + ], + ), + app=ApplicationEntityModelFactory( + identifier="myapp", + fromm__target="pkg", + meta__post_deploy=[ + {"sql_script": "app_post_deploy1.sql"}, + ], + ), + ), + files={ + "app_post_deploy1.sql": "\n", + "setup.sql": "CREATE OR ALTER VERSIONED SCHEMA core;", + "README.md": "\n", + "manifest.yml": "\n", + "app/dummy_file.md": "\n", + }, + ), + { + CLICounterField.SNOWPARK_PROCESSOR: 0, + CLICounterField.TEMPLATES_PROCESSOR: 1, + CLICounterField.PDF_TEMPLATES: 1, + }, + ), + ( + "app run", + lambda: ProjectV2Factory( + pdf__entities=dict( + pkg=ApplicationPackageEntityModelFactory( + identifier="myapp_pkg", + artifacts=[ + "setup.sql", + "README.md", + "manifest.yml", + {"src": "app/*", "dest": "./", "processors": ["templates"]}, + ], + ), + app=ApplicationEntityModelFactory( + identifier="myapp", + fromm__target="pkg", + ), + ), + files={ + "setup.sql": "CREATE OR ALTER VERSIONED SCHEMA core;", + "README.md": "\n", + "manifest.yml": "\n", + "app/dummy_file.md": "\n", + }, + ), + { + CLICounterField.SNOWPARK_PROCESSOR: 0, + CLICounterField.TEMPLATES_PROCESSOR: 1, + CLICounterField.PDF_TEMPLATES: 0, + CLICounterField.POST_DEPLOY_SCRIPTS: 0, + }, + ), + ( + "app deploy", + lambda: ProjectV11Factory( + pdf__native_app__package__scripts=["scripts/package_script1.sql"], + pdf__native_app__artifacts=["README.md", "setup.sql", "manifest.yml"], + pdf__native_app__package__warehouse="non_existent_warehouse", + files={ + "README.md": "", + "setup.sql": "select 1", + "manifest.yml": "\n", + "scripts/package_script1.sql": "\n", + }, + ), + { + CLICounterField.SNOWPARK_PROCESSOR: 0, + CLICounterField.TEMPLATES_PROCESSOR: 0, + CLICounterField.PDF_TEMPLATES: 0, + CLICounterField.POST_DEPLOY_SCRIPTS: 1, + CLICounterField.PACKAGE_SCRIPTS: 1, + }, + ), + ( + "app deploy", + lambda: ProjectV2Factory( + pdf__entities=dict( + pkg=ApplicationPackageEntityModelFactory( + identifier="myapp_pkg", + meta__post_deploy=[ + {"sql_script": "scripts/pkg_post_deploy1.sql"}, + ], + ), + app=ApplicationEntityModelFactory( + identifier="myapp_<% ctx.env.USER %>", + fromm__target="pkg", + ), + ), + files={ + "setup.sql": "CREATE OR ALTER VERSIONED SCHEMA core;", + "README.md": "\n", + "manifest.yml": "\n", + "scripts/pkg_post_deploy1.sql": "-- package post-deploy script\n", + }, + ), + { + CLICounterField.SNOWPARK_PROCESSOR: 0, + CLICounterField.TEMPLATES_PROCESSOR: 0, + CLICounterField.PDF_TEMPLATES: 1, + CLICounterField.POST_DEPLOY_SCRIPTS: 1, + }, + ), + ], + ids=[ + "v1_post_deploy_set_and_package_scripts_available", + "v2_post_deploy_not_available_in_only_bundle", + "v2_templates_processor_set", + "v1_package_scripts_converted_to_post_deploy_both_set", + "v2_post_deploy_set_and_package_scripts_not_available", + ], +) +@mock.patch("snowflake.connector.telemetry.TelemetryClient.try_add_log_to_batch") +def test_nativeapp_feature_counter_has_expected_value( + mock_telemetry, + runner, + nativeapp_teardown, + temp_dir, + command: str, + project_factory: Callable, + expected_counters: Dict[str, int], +): + local_test_env = { + "APP_DIR": "app", + "schema_name": "test_schema", + "table_name": "test_table", + "value": "test_value", + } + + project_factory() + + with nativeapp_teardown(project_dir=Path(temp_dir)): + runner.invoke_with_connection(split(command), env=local_test_env) + + message = extract_first_telemetry_message_of_type( + mock_telemetry, TelemetryEvent.CMD_EXECUTION_RESULT.value + ) + + assert message[CLITelemetryField.COUNTERS.value] == expected_counters diff --git a/tests_integration/nativeapp/test_command_metric_spans.py b/tests_integration/nativeapp/test_command_metric_spans.py new file mode 100644 index 0000000000..ba9396161a --- /dev/null +++ b/tests_integration/nativeapp/test_command_metric_spans.py @@ -0,0 +1,164 @@ +# Copyright (c) 2024 Snowflake Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +from shlex import split +from typing import Dict, Callable +from unittest import mock + +from snowflake.cli._app.telemetry import TelemetryEvent, CLITelemetryField +from snowflake.cli.api.metrics import CLICounterField, CLIMetricsSpan, CLIMetrics +from tests.nativeapp.factories import ( + ProjectV11Factory, + ProjectV2Factory, + ApplicationPackageEntityModelFactory, + ApplicationEntityModelFactory, +) +from tests.project.fixtures import * +from tests_common import temp_dir +from tests_integration.test_utils import extract_first_telemetry_message_of_type + +SPAN_KEYS_TO_CHECK = [CLIMetricsSpan.NAME_KEY, CLIMetricsSpan.PARENT_KEY] + + +@pytest.mark.integration +@pytest.mark.parametrize( + "command,project_factory,expected_spans", + [ + ( + "ws bundle --entity-id=pkg", + lambda: ProjectV2Factory( + pdf__entities=dict( + pkg=ApplicationPackageEntityModelFactory( + identifier="myapp_pkg_<% ctx.env.USER %>", + artifacts=[ + "setup.sql", + "README.md", + "manifest.yml", + ], + ), + app=ApplicationEntityModelFactory( + identifier="myapp", + fromm__target="pkg", + meta__post_deploy=[ + {"sql_script": "app_post_deploy1.sql"}, + ], + ), + ), + files={ + "app_post_deploy1.sql": "\n", + "setup.sql": "CREATE OR ALTER VERSIONED SCHEMA core;", + "README.md": "\n", + "manifest.yml": "\n", + }, + ), + [{"name": "ApplicationPackageEntity.action_bundle", "parent": None}], + ), + ( + "app run", + lambda: ProjectV2Factory( + pdf__entities=dict( + pkg=ApplicationPackageEntityModelFactory( + identifier="myapp_pkg", + artifacts=[ + "setup.sql", + "README.md", + "manifest.yml", + {"src": "app/*", "dest": "./", "processors": ["templates"]}, + ], + ), + app=ApplicationEntityModelFactory( + identifier="myapp", + fromm__target="pkg", + meta__post_deploy=[ + {"sql_script": "scripts/app_post_deploy1.sql"}, + ], + ), + ), + files={ + "setup.sql": "CREATE OR ALTER VERSIONED SCHEMA core;", + "README.md": "\n", + "manifest.yml": "\n", + "app/dummy_file.md": "\n", + "scripts/app_post_deploy1.sql": "-- package post-deploy script\n", + }, + ), + [ + { + "name": "ApplicationEntity.action_deploy", + "parent": None, + }, + { + "name": "compile_artifacts", + "parent": "ApplicationEntity.action_deploy", + }, + { + "name": "sync_deploy_root_with_stage", + "parent": "ApplicationEntity.action_deploy", + }, + { + "name": "get_validation_result", + "parent": "ApplicationEntity.action_deploy", + }, + { + "name": "create_or_upgrade_app", + "parent": "ApplicationEntity.action_deploy", + }, + { + "name": "execute_post_deploy_hooks", + "parent": "create_or_upgrade_app", + }, + ], + ), + ], + ids=["bundle_barebones_no_other_spans", "run_with_all_features_with_spans"], +) +@mock.patch("snowflake.connector.telemetry.TelemetryClient.try_add_log_to_batch") +def test_nativeapp_command_has_expected_spans( + mock_telemetry, + runner, + nativeapp_teardown, + temp_dir, + command: str, + project_factory: Callable, + expected_spans: Dict, +): + local_test_env = { + "APP_DIR": "app", + "schema_name": "test_schema", + "table_name": "test_table", + "value": "test_value", + } + + project_factory() + + with nativeapp_teardown(project_dir=Path(temp_dir)): + runner.invoke_with_connection(split(command), env=local_test_env) + + message = extract_first_telemetry_message_of_type( + mock_telemetry, TelemetryEvent.CMD_EXECUTION_RESULT.value + ) + + spans = message[CLITelemetryField.SPANS.value] + + # ensure spans are within our defined limits + assert spans[CLITelemetryField.NUM_SPANS_PAST_DEPTH_LIMIT.value] == 0 + assert spans[CLITelemetryField.NUM_SPANS_PAST_TOTAL_LIMIT.value] == 0 + assert all( + span[CLIMetricsSpan.TRIMMED_KEY] == False + for span in spans[CLITelemetryField.COMPLETED_SPANS.value] + ) + + assert expected_spans == [ + {key: span[key] for key in SPAN_KEYS_TO_CHECK} + for span in spans[CLITelemetryField.COMPLETED_SPANS.value] + ] diff --git a/tests_integration/nativeapp/test_feature_metrics.py b/tests_integration/nativeapp/test_feature_metrics.py deleted file mode 100644 index 271626f393..0000000000 --- a/tests_integration/nativeapp/test_feature_metrics.py +++ /dev/null @@ -1,149 +0,0 @@ -# Copyright (c) 2024 Snowflake Inc. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -from shlex import split -from typing import Dict -from unittest import mock - -from snowflake.cli._app.telemetry import TelemetryEvent, CLITelemetryField -from snowflake.cli.api.metrics import CLICounterField -from tests.project.fixtures import * -from tests_integration.test_utils import extract_first_telemetry_message_of_type - - -@pytest.mark.integration -@pytest.mark.parametrize( - "command,expected_counter", - [ - ( - [ - "sql", - "-q", - "select '<% ctx.env.test %>'", - "--env", - "test=value_from_cli", - ], - 1, - ), - (["sql", "-q", "select 'string'"], 0), - ], -) -@mock.patch("snowflake.connector.telemetry.TelemetryClient.try_add_log_to_batch") -def test_sql_templating_emits_counter( - mock_telemetry, - command: List[str], - expected_counter, - runner, -): - result = runner.invoke_with_connection_json(command) - - assert result.exit_code == 0 - - message = extract_first_telemetry_message_of_type( - mock_telemetry, TelemetryEvent.CMD_EXECUTION_RESULT.value - ) - - assert ( - message[CLITelemetryField.COUNTERS.value][CLICounterField.SQL_TEMPLATES] - == expected_counter - ) - - -@pytest.mark.integration -@pytest.mark.parametrize( - "command," "test_project," "expected_counters", - [ - # ensure that post deploy scripts are picked up for v1 - ( - "app deploy", - "napp_application_post_deploy_v1", - { - CLICounterField.SNOWPARK_PROCESSOR: 0, - CLICounterField.TEMPLATES_PROCESSOR: 0, - CLICounterField.PDF_TEMPLATES: 0, - CLICounterField.POST_DEPLOY_SCRIPTS: 1, - CLICounterField.PACKAGE_SCRIPTS: 0, - }, - ), - # post deploy scripts should not be available for bundling since there is no deploy - ( - "ws bundle --entity-id=pkg", - "napp_templates_processors_v2", - { - CLICounterField.SNOWPARK_PROCESSOR: 0, - CLICounterField.TEMPLATES_PROCESSOR: 1, - CLICounterField.PDF_TEMPLATES: 1, - }, - ), - # ensure that templates processor is picked up - ( - "app run", - "napp_templates_processors_v1", - { - CLICounterField.SNOWPARK_PROCESSOR: 0, - CLICounterField.TEMPLATES_PROCESSOR: 1, - CLICounterField.PDF_TEMPLATES: 0, - CLICounterField.POST_DEPLOY_SCRIPTS: 0, - CLICounterField.PACKAGE_SCRIPTS: 0, - }, - ), - # package scripts are auto-converted to post deploy scripts in v1 - ( - "app deploy", - "integration_external", - { - CLICounterField.SNOWPARK_PROCESSOR: 0, - CLICounterField.TEMPLATES_PROCESSOR: 0, - CLICounterField.PDF_TEMPLATES: 1, - CLICounterField.POST_DEPLOY_SCRIPTS: 1, - CLICounterField.PACKAGE_SCRIPTS: 1, - }, - ), - # ensure post deploy scripts are picked up for v2 - ( - "app deploy", - "integration_external_v2", - { - CLICounterField.SNOWPARK_PROCESSOR: 0, - CLICounterField.TEMPLATES_PROCESSOR: 0, - CLICounterField.PDF_TEMPLATES: 1, - CLICounterField.POST_DEPLOY_SCRIPTS: 1, - }, - ), - ], -) -@mock.patch("snowflake.connector.telemetry.TelemetryClient.try_add_log_to_batch") -def test_nativeapp_feature_counter_has_expected_value( - mock_telemetry, - runner, - nativeapp_teardown, - nativeapp_project_directory, - command: str, - test_project: str, - expected_counters: Dict[str, int], -): - local_test_env = { - "APP_DIR": "app", - "schema_name": "test_schema", - "table_name": "test_table", - "value": "test_value", - } - - with nativeapp_project_directory(test_project): - runner.invoke_with_connection(split(command), env=local_test_env) - - message = extract_first_telemetry_message_of_type( - mock_telemetry, TelemetryEvent.CMD_EXECUTION_RESULT.value - ) - - assert message[CLITelemetryField.COUNTERS.value] == expected_counters From 4edd8b290bc4aabb49140d85eeb2ed6362397826 Mon Sep 17 00:00:00 2001 From: Marcus Chok Date: Tue, 12 Nov 2024 14:54:41 -0500 Subject: [PATCH 05/16] rename spans as logical steps, remove extra param from console phase, un-parametrize tests, merge test files to keep with covention --- .github/CODEOWNERS | 2 +- .../cli/_plugins/nativeapp/artifacts.py | 2 + .../_plugins/nativeapp/codegen/compiler.py | 5 +- .../codegen/snowpark/python_processor.py | 3 +- .../codegen/templates/templates_processor.py | 3 +- .../nativeapp/entities/application.py | 5 +- .../nativeapp/entities/application_package.py | 6 +- .../cli/_plugins/workspace/manager.py | 5 +- src/snowflake/cli/api/cli_global_context.py | 4 +- src/snowflake/cli/api/console/abc.py | 1 - src/snowflake/cli/api/console/console.py | 9 +- src/snowflake/cli/api/entities/utils.py | 8 +- src/snowflake/cli/api/metrics.py | 2 +- tests/api/metrics/__init__.py | 0 tests/api/metrics/test_metrics_counters.py | 101 ---- tests/api/test_global_cli_context.py | 4 +- .../test_metrics_spans.py => test_metrics.py} | 107 +++- .../nativeapp/test_command_metric_counters.py | 248 -------- .../nativeapp/test_command_metric_spans.py | 164 ------ tests_integration/nativeapp/test_metrics.py | 534 ++++++++++++++++++ 20 files changed, 660 insertions(+), 553 deletions(-) delete mode 100644 tests/api/metrics/__init__.py delete mode 100644 tests/api/metrics/test_metrics_counters.py rename tests/api/{metrics/test_metrics_spans.py => test_metrics.py} (81%) delete mode 100644 tests_integration/nativeapp/test_command_metric_counters.py delete mode 100644 tests_integration/nativeapp/test_command_metric_spans.py create mode 100644 tests_integration/nativeapp/test_metrics.py diff --git a/.github/CODEOWNERS b/.github/CODEOWNERS index 10e430295e..bc5af947d9 100644 --- a/.github/CODEOWNERS +++ b/.github/CODEOWNERS @@ -7,7 +7,7 @@ src/snowflake/cli/api/metrics.py @snowflakedb/snowcli @snowflakedb/nade src/snowflake/cli/api/project/definition_conversion.py @snowflakedb/snowcli @snowflakedb/nade src/snowflake/cli/api/utils/ @snowflakedb/snowcli @snowflakedb/nade -tests/api/metrics/ @snowflakedb/snowcli @snowflakedb/nade +tests/api/test_metrics.py @snowflakedb/snowcli @snowflakedb/nade tests_common/__init__.py @snowflakedb/snowcli @snowflakedb/nade tests_common/conftest.py @snowflakedb/snowcli @snowflakedb/nade tests_common/deflake.py @snowflakedb/snowcli @snowflakedb/nade diff --git a/src/snowflake/cli/_plugins/nativeapp/artifacts.py b/src/snowflake/cli/_plugins/nativeapp/artifacts.py index dcdecd6006..ee7051b483 100644 --- a/src/snowflake/cli/_plugins/nativeapp/artifacts.py +++ b/src/snowflake/cli/_plugins/nativeapp/artifacts.py @@ -21,6 +21,7 @@ from typing import Any, Callable, Dict, Iterable, Iterator, List, Optional, Tuple, Union from click.exceptions import ClickException +from snowflake.cli.api.cli_global_context import span from snowflake.cli.api.constants import DEFAULT_SIZE_LIMIT_MB from snowflake.cli.api.project.schemas.v1.native_app.path_mapping import PathMapping from snowflake.cli.api.project.util import to_identifier @@ -648,6 +649,7 @@ def resolve_without_follow(path: Path) -> Path: return Path(os.path.abspath(path)) +@span("build_initial_bundle") def build_bundle( project_root: Path, deploy_root: Path, diff --git a/src/snowflake/cli/_plugins/nativeapp/codegen/compiler.py b/src/snowflake/cli/_plugins/nativeapp/codegen/compiler.py index 611da5232c..07302e6356 100644 --- a/src/snowflake/cli/_plugins/nativeapp/codegen/compiler.py +++ b/src/snowflake/cli/_plugins/nativeapp/codegen/compiler.py @@ -88,7 +88,10 @@ def compile_artifacts(self): if not self._should_invoke_processors(): return - with cc.phase("Invoking artifact processors", span_name="compile_artifacts"): + with ( + cc.phase("Invoking artifact processors"), + get_cli_context().metrics.span("artifact_processors"), + ): if self._bundle_ctx.generated_root.exists(): raise ClickException( f"Path {self._bundle_ctx.generated_root} already exists. Please choose a different name for your generated directory in the project definition file." diff --git a/src/snowflake/cli/_plugins/nativeapp/codegen/snowpark/python_processor.py b/src/snowflake/cli/_plugins/nativeapp/codegen/snowpark/python_processor.py index 9665aa9bd6..0241899388 100644 --- a/src/snowflake/cli/_plugins/nativeapp/codegen/snowpark/python_processor.py +++ b/src/snowflake/cli/_plugins/nativeapp/codegen/snowpark/python_processor.py @@ -48,7 +48,7 @@ NativeAppExtensionFunction, ) from snowflake.cli._plugins.stage.diff import to_stage_path -from snowflake.cli.api.cli_global_context import get_cli_context +from snowflake.cli.api.cli_global_context import get_cli_context, span from snowflake.cli.api.console import cli_console as cc from snowflake.cli.api.metrics import CLICounterField from snowflake.cli.api.project.schemas.v1.native_app.path_mapping import ( @@ -167,6 +167,7 @@ class SnowparkAnnotationProcessor(ArtifactProcessor): def __init__(self, *args, **kwargs): super().__init__(*args, **kwargs) + @span("snowpark_processor") def process( self, artifact_to_process: PathMapping, diff --git a/src/snowflake/cli/_plugins/nativeapp/codegen/templates/templates_processor.py b/src/snowflake/cli/_plugins/nativeapp/codegen/templates/templates_processor.py index 06ad67f164..9e38eecc2c 100644 --- a/src/snowflake/cli/_plugins/nativeapp/codegen/templates/templates_processor.py +++ b/src/snowflake/cli/_plugins/nativeapp/codegen/templates/templates_processor.py @@ -23,7 +23,7 @@ ArtifactProcessor, ) from snowflake.cli._plugins.nativeapp.exceptions import InvalidTemplateInFileError -from snowflake.cli.api.cli_global_context import get_cli_context +from snowflake.cli.api.cli_global_context import get_cli_context, span from snowflake.cli.api.console import cli_console as cc from snowflake.cli.api.metrics import CLICounterField from snowflake.cli.api.project.schemas.v1.native_app.path_mapping import ( @@ -91,6 +91,7 @@ def expand_templates_in_file( if expanded_template != file.contents: file.edited_contents = expanded_template + @span("templates_processor") def process( self, artifact_to_process: PathMapping, diff --git a/src/snowflake/cli/_plugins/nativeapp/entities/application.py b/src/snowflake/cli/_plugins/nativeapp/entities/application.py index 8eac01eded..29ae85b762 100644 --- a/src/snowflake/cli/_plugins/nativeapp/entities/application.py +++ b/src/snowflake/cli/_plugins/nativeapp/entities/application.py @@ -43,7 +43,7 @@ from snowflake.cli._plugins.nativeapp.sf_facade import get_snowflake_facade from snowflake.cli._plugins.nativeapp.utils import needs_confirmation from snowflake.cli._plugins.workspace.context import ActionContext -from snowflake.cli.api.cli_global_context import start_cli_metrics_span +from snowflake.cli.api.cli_global_context import span from snowflake.cli.api.entities.common import EntityBase, get_sql_executor from snowflake.cli.api.entities.utils import ( drop_generic_object, @@ -147,6 +147,7 @@ def post_deploy_hooks(self) -> list[PostDeployHook] | None: model = self._entity_model return model.meta and model.meta.post_deploy + @span("deploy_app") def action_deploy( self, action_ctx: ActionContext, @@ -231,6 +232,7 @@ def action_deploy( interactive=interactive, ) + @span("drop_app") def action_drop( self, action_ctx: ActionContext, @@ -421,7 +423,6 @@ def get_objects_owned_by_application(self) -> List[ApplicationOwnedObject]: ).fetchall() return [{"name": row[1], "type": row[2]} for row in results] - @start_cli_metrics_span("create_or_upgrade_app") def create_or_upgrade_app( self, package: ApplicationPackageEntity, diff --git a/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py b/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py index ed8f7e7fff..d3f276e896 100644 --- a/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py +++ b/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py @@ -43,7 +43,7 @@ from snowflake.cli._plugins.stage.diff import DiffResult from snowflake.cli._plugins.stage.manager import StageManager from snowflake.cli._plugins.workspace.context import ActionContext -from snowflake.cli.api.cli_global_context import start_cli_metrics_span +from snowflake.cli.api.cli_global_context import span from snowflake.cli.api.entities.common import EntityBase, get_sql_executor from snowflake.cli.api.entities.utils import ( drop_generic_object, @@ -219,6 +219,7 @@ def action_deploy( force=force, ) + @span("drop_app_package") def action_drop(self, action_ctx: ActionContext, force_drop: bool, *args, **kwargs): console = self._workspace_ctx.console sql_executor = get_sql_executor() @@ -549,6 +550,7 @@ def _bundle(self): compiler.compile_artifacts() return bundle_map + @span("deploy_app_package") def _deploy( self, bundle_map: BundleMap | None, @@ -916,7 +918,7 @@ def validate_setup_script( if validation_result["status"] == "FAIL": raise SetupScriptFailedValidation() - @start_cli_metrics_span("get_validation_result") + @span("validate_setup_script") def get_validation_result( self, use_scratch_stage: bool, interactive: bool, force: bool ): diff --git a/src/snowflake/cli/_plugins/workspace/manager.py b/src/snowflake/cli/_plugins/workspace/manager.py index d1e525fa66..25b56d542f 100644 --- a/src/snowflake/cli/_plugins/workspace/manager.py +++ b/src/snowflake/cli/_plugins/workspace/manager.py @@ -61,10 +61,7 @@ def perform_action(self, entity_id: str, action: EntityActions, *args, **kwargs) action_ctx = ActionContext( get_entity=self.get_entity, ) - with get_cli_context().metrics.start_span( - f"{type(entity).__name__}.{action.value}" - ): - return entity.perform(action, action_ctx, *args, **kwargs) + return entity.perform(action, action_ctx, *args, **kwargs) else: raise ValueError(f'This entity type does not support "{action.value}"') diff --git a/src/snowflake/cli/api/cli_global_context.py b/src/snowflake/cli/api/cli_global_context.py index f58b576b28..ed5b49db12 100644 --- a/src/snowflake/cli/api/cli_global_context.py +++ b/src/snowflake/cli/api/cli_global_context.py @@ -214,7 +214,7 @@ def get_cli_context() -> _CliGlobalContextAccess: return _CliGlobalContextAccess(get_cli_context_manager()) -def start_cli_metrics_span(span_name: str): +def span(span_name: str): """ Decorator to start a command metrics span that encompasses a whole function @@ -226,7 +226,7 @@ def start_cli_metrics_span(span_name: str): def decorator(func): @wraps(func) def wrapper(*args, **kwargs): - with get_cli_context().metrics.start_span(span_name): + with get_cli_context().metrics.span(span_name): return func(*args, **kwargs) return wrapper diff --git a/src/snowflake/cli/api/console/abc.py b/src/snowflake/cli/api/console/abc.py index 22b1aa2cb5..d8b2e3d683 100644 --- a/src/snowflake/cli/api/console/abc.py +++ b/src/snowflake/cli/api/console/abc.py @@ -68,7 +68,6 @@ def phase( self, enter_message: str, exit_message: Optional[str] = None, - span_name: Optional[str] = None, ) -> Iterator[Callable[[str], None]]: """A context manager for organising steps into logical group.""" diff --git a/src/snowflake/cli/api/console/console.py b/src/snowflake/cli/api/console/console.py index 97a959092b..75a9acf1bc 100644 --- a/src/snowflake/cli/api/console/console.py +++ b/src/snowflake/cli/api/console/console.py @@ -14,12 +14,11 @@ from __future__ import annotations -from contextlib import contextmanager, nullcontext +from contextlib import contextmanager from typing import Optional from rich.style import Style from rich.text import Text -from snowflake.cli.api.cli_global_context import get_cli_context from snowflake.cli.api.console.abc import AbstractConsole from snowflake.cli.api.console.enum import Output @@ -73,7 +72,6 @@ def phase( self, enter_message: str, exit_message: Optional[str] = None, - span_name: Optional[str] = None, ): """A context manager for organising steps into logical group.""" if self.in_phase: @@ -87,10 +85,7 @@ def phase( self._in_phase = True try: - with get_cli_context().metrics.start_span( - span_name - ) if span_name else nullcontext(): - yield self.step + yield self.step finally: self._in_phase = False if exit_message: diff --git a/src/snowflake/cli/api/entities/utils.py b/src/snowflake/cli/api/entities/utils.py index 1eb1719a8c..4aeb1d6f5e 100644 --- a/src/snowflake/cli/api/entities/utils.py +++ b/src/snowflake/cli/api/entities/utils.py @@ -23,7 +23,7 @@ to_stage_path, ) from snowflake.cli._plugins.stage.utils import print_diff_to_console -from snowflake.cli.api.cli_global_context import get_cli_context, start_cli_metrics_span +from snowflake.cli.api.cli_global_context import get_cli_context, span from snowflake.cli.api.console.abc import AbstractConsole from snowflake.cli.api.entities.common import get_sql_executor from snowflake.cli.api.errno import ( @@ -76,7 +76,7 @@ def _get_stage_paths_to_sync( return stage_paths -@start_cli_metrics_span("sync_deploy_root_with_stage") +@span("sync_remote_and_local_files") def sync_deploy_root_with_stage( console: AbstractConsole, deploy_root: Path, @@ -222,8 +222,7 @@ def execute_post_deploy_hooks( with console.phase( f"Executing {deployed_object_type} post-deploy actions", - span_name="execute_post_deploy_hooks", - ): + ), get_cli_context().metrics.span("post_deploy_hooks"): sql_scripts_paths = [] display_paths = [] for hook in post_deploy_hooks: @@ -308,7 +307,6 @@ def validation_item_to_str(item: dict[str, str | int]): return s -@start_cli_metrics_span("drop_generic_object") def drop_generic_object( console: AbstractConsole, object_type: str, diff --git a/src/snowflake/cli/api/metrics.py b/src/snowflake/cli/api/metrics.py index bf84bd4bc8..dc8c8c1e21 100644 --- a/src/snowflake/cli/api/metrics.py +++ b/src/snowflake/cli/api/metrics.py @@ -216,7 +216,7 @@ def current_span(self) -> Optional[CLIMetricsSpan]: return self._in_progress_spans[-1] if len(self._in_progress_spans) > 0 else None @contextmanager - def start_span(self, name: str) -> Iterator[CLIMetricsSpan]: + def span(self, name: str) -> Iterator[CLIMetricsSpan]: """ Starts a new span that tracks various metrics throughout its execution diff --git a/tests/api/metrics/__init__.py b/tests/api/metrics/__init__.py deleted file mode 100644 index e69de29bb2..0000000000 diff --git a/tests/api/metrics/test_metrics_counters.py b/tests/api/metrics/test_metrics_counters.py deleted file mode 100644 index 133269dda5..0000000000 --- a/tests/api/metrics/test_metrics_counters.py +++ /dev/null @@ -1,101 +0,0 @@ -# Copyright (c) 2024 Snowflake Inc. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. - -from snowflake.cli.api.metrics import CLIMetrics - - -def test_metrics_no_counters(): - # given - metrics = CLIMetrics() - - # when - - # then - assert metrics.counters == {} - assert metrics.get_counter("counter1") is None - - -def test_metrics_set_one_counter(): - # given - metrics = CLIMetrics() - - # when - metrics.set_counter("counter1", 1) - - # then - assert metrics.counters == {"counter1": 1} - assert metrics.get_counter("counter1") == 1 - - -def test_metrics_increment_new_counter(): - # given - metrics = CLIMetrics() - - # when - metrics.increment_counter("counter1") - - # then - assert metrics.counters == {"counter1": 1} - assert metrics.get_counter("counter1") == 1 - - -def test_metrics_increment_existing_counter(): - # given - metrics = CLIMetrics() - - # when - metrics.set_counter("counter1", 2) - metrics.increment_counter(name="counter1", value=2) - - # then - assert metrics.counters == {"counter1": 4} - assert metrics.get_counter("counter1") == 4 - - -def test_metrics_set_multiple_counters(): - # given - metrics = CLIMetrics() - - # when - metrics.set_counter("counter1", 1) - metrics.set_counter("counter2", 0) - metrics.set_counter(name="counter2", value=2) - - # then - assert metrics.counters == {"counter1": 1, "counter2": 2} - assert metrics.get_counter("counter1") == 1 - assert metrics.get_counter("counter2") == 2 - - -def test_metrics_set_default_new_counter(): - # given - metrics = CLIMetrics() - - # when - metrics.set_counter_default("c1", 3) - - # then - assert metrics.counters == {"c1": 3} - - -def test_metrics_set_default_existing_counter(): - # given - metrics = CLIMetrics() - - # when - metrics.set_counter("c2", 2) - metrics.set_counter_default("c2", 1) - - # then - assert metrics.counters == {"c2": 2} diff --git a/tests/api/test_global_cli_context.py b/tests/api/test_global_cli_context.py index 975ed3026d..a62bdfd5c5 100644 --- a/tests/api/test_global_cli_context.py +++ b/tests/api/test_global_cli_context.py @@ -90,10 +90,10 @@ def test_forked_context(): def test_forked_metrics_spans(): outer_metrics = get_cli_context_manager().metrics - with outer_metrics.start_span("outer_span"): + with outer_metrics.span("outer_span"): with fork_cli_context() as inner: inner_metrics = inner.metrics - with inner_metrics.start_span("inner_span"): + with inner_metrics.span("inner_span"): pass assert outer_metrics != inner_metrics diff --git a/tests/api/metrics/test_metrics_spans.py b/tests/api/test_metrics.py similarity index 81% rename from tests/api/metrics/test_metrics_spans.py rename to tests/api/test_metrics.py index 1a6934935e..115f53da52 100644 --- a/tests/api/metrics/test_metrics_spans.py +++ b/tests/api/test_metrics.py @@ -11,6 +11,7 @@ # WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. # See the License for the specific language governing permissions and # limitations under the License. + import uuid from itertools import count from unittest import mock @@ -23,6 +24,92 @@ ) +def test_metrics_no_counters(): + # given + metrics = CLIMetrics() + + # when + + # then + assert metrics.counters == {} + assert metrics.get_counter("counter1") is None + + +def test_metrics_set_one_counter(): + # given + metrics = CLIMetrics() + + # when + metrics.set_counter("counter1", 1) + + # then + assert metrics.counters == {"counter1": 1} + assert metrics.get_counter("counter1") == 1 + + +def test_metrics_increment_new_counter(): + # given + metrics = CLIMetrics() + + # when + metrics.increment_counter("counter1") + + # then + assert metrics.counters == {"counter1": 1} + assert metrics.get_counter("counter1") == 1 + + +def test_metrics_increment_existing_counter(): + # given + metrics = CLIMetrics() + + # when + metrics.set_counter("counter1", 2) + metrics.increment_counter(name="counter1", value=2) + + # then + assert metrics.counters == {"counter1": 4} + assert metrics.get_counter("counter1") == 4 + + +def test_metrics_set_multiple_counters(): + # given + metrics = CLIMetrics() + + # when + metrics.set_counter("counter1", 1) + metrics.set_counter("counter2", 0) + metrics.set_counter(name="counter2", value=2) + + # then + assert metrics.counters == {"counter1": 1, "counter2": 2} + assert metrics.get_counter("counter1") == 1 + assert metrics.get_counter("counter2") == 2 + + +def test_metrics_set_default_new_counter(): + # given + metrics = CLIMetrics() + + # when + metrics.set_counter_default("c1", 3) + + # then + assert metrics.counters == {"c1": 3} + + +def test_metrics_set_default_existing_counter(): + # given + metrics = CLIMetrics() + + # when + metrics.set_counter("c2", 2) + metrics.set_counter_default("c2", 1) + + # then + assert metrics.counters == {"c2": 2} + + # we need to mock time.monotonic because on windows it does not # capture enough precision for these tests to not be flaky @pytest.fixture @@ -39,7 +126,7 @@ def create_span(num_spans: int): if num_spans <= 0: return - with metrics.start_span(f"span-{next(counter)}"): + with metrics.span(f"span-{next(counter)}"): create_span(num_spans - 1) for _ in range(width): @@ -64,7 +151,7 @@ def test_metrics_spans_single_span_no_error_or_parent(mock_time_monotonic): metrics = CLIMetrics() # when - with metrics.start_span("span1") as span1: + with metrics.span("span1") as span1: assert metrics.current_span is span1 assert metrics.current_span is None @@ -92,7 +179,7 @@ def test_metrics_spans_finish_early_is_idempotent(): metrics = CLIMetrics() # when - with metrics.start_span("span1") as span1: + with metrics.span("span1") as span1: start_time = span1.start_time span1.finish() execution_time = span1.execution_time @@ -109,10 +196,10 @@ def test_metrics_spans_parent_with_one_child(mock_time_monotonic): metrics = CLIMetrics() # when - with metrics.start_span("parent") as parent: + with metrics.span("parent") as parent: assert metrics.current_span is parent - with metrics.start_span("child") as child: + with metrics.span("child") as child: assert metrics.current_span is child assert metrics.current_span is parent @@ -162,15 +249,15 @@ def test_metrics_spans_parent_with_two_children_same_name(mock_time_monotonic): metrics = CLIMetrics() # when - with metrics.start_span("parent") as parent: + with metrics.span("parent") as parent: assert metrics.current_span is parent - with metrics.start_span("child") as child1: + with metrics.span("child") as child1: assert metrics.current_span is child1 assert metrics.current_span is parent - with metrics.start_span("child") as child2: + with metrics.span("child") as child2: assert metrics.current_span is child2 assert metrics.current_span is parent @@ -249,7 +336,7 @@ def test_metrics_spans_error_is_propagated(): # when with pytest.raises(RuntimeError): - with metrics.start_span("step1"): + with metrics.span("step1"): raise RuntimeError() # then @@ -264,7 +351,7 @@ def test_metrics_spans_empty_name_raises_error(): # when with pytest.raises(CLIMetricsInvalidUsageError) as err: - with metrics.start_span(""): + with metrics.span(""): pass # then diff --git a/tests_integration/nativeapp/test_command_metric_counters.py b/tests_integration/nativeapp/test_command_metric_counters.py deleted file mode 100644 index 8a834473ba..0000000000 --- a/tests_integration/nativeapp/test_command_metric_counters.py +++ /dev/null @@ -1,248 +0,0 @@ -# Copyright (c) 2024 Snowflake Inc. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -from shlex import split -from typing import Dict, Callable -from unittest import mock - -from snowflake.cli._app.telemetry import TelemetryEvent, CLITelemetryField -from snowflake.cli.api.metrics import CLICounterField -from tests.nativeapp.factories import ( - ProjectV11Factory, - ProjectV2Factory, - ApplicationPackageEntityModelFactory, - ApplicationEntityModelFactory, -) -from tests.project.fixtures import * -from tests_common import temp_dir -from tests_integration.test_utils import extract_first_telemetry_message_of_type - - -@pytest.mark.integration -@pytest.mark.parametrize( - "command,expected_counter", - [ - ( - [ - "sql", - "-q", - "select '<% ctx.env.test %>'", - "--env", - "test=value_from_cli", - ], - 1, - ), - (["sql", "-q", "select 'string'"], 0), - ], -) -@mock.patch("snowflake.connector.telemetry.TelemetryClient.try_add_log_to_batch") -def test_sql_templating_emits_counter( - mock_telemetry, - command: List[str], - expected_counter, - runner, -): - result = runner.invoke_with_connection_json(command) - - assert result.exit_code == 0 - - message = extract_first_telemetry_message_of_type( - mock_telemetry, TelemetryEvent.CMD_EXECUTION_RESULT.value - ) - - assert ( - message[CLITelemetryField.COUNTERS.value][CLICounterField.SQL_TEMPLATES] - == expected_counter - ) - - -@pytest.mark.integration -@pytest.mark.parametrize( - "command,project_factory,expected_counters", - [ - ( - "app deploy", - lambda: ProjectV11Factory( - pdf__native_app__artifacts=["README.md", "setup.sql", "manifest.yml"], - pdf__native_app__package__post_deploy=[ - {"sql_script": "post_deploy1.sql"}, - ], - files={ - "README.md": "", - "setup.sql": "select 1", - "manifest.yml": "\n", - "post_deploy1.sql": "\n", - }, - ), - { - CLICounterField.SNOWPARK_PROCESSOR: 0, - CLICounterField.TEMPLATES_PROCESSOR: 0, - CLICounterField.PDF_TEMPLATES: 0, - CLICounterField.POST_DEPLOY_SCRIPTS: 1, - CLICounterField.PACKAGE_SCRIPTS: 0, - }, - ), - ( - "ws bundle --entity-id=pkg", - lambda: ProjectV2Factory( - pdf__entities=dict( - pkg=ApplicationPackageEntityModelFactory( - identifier="myapp_pkg_<% ctx.env.USER %>", - artifacts=[ - "setup.sql", - "README.md", - "manifest.yml", - # just needs to have the templates processor to nest phases - {"src": "app/*", "dest": "./", "processors": ["templates"]}, - ], - ), - app=ApplicationEntityModelFactory( - identifier="myapp", - fromm__target="pkg", - meta__post_deploy=[ - {"sql_script": "app_post_deploy1.sql"}, - ], - ), - ), - files={ - "app_post_deploy1.sql": "\n", - "setup.sql": "CREATE OR ALTER VERSIONED SCHEMA core;", - "README.md": "\n", - "manifest.yml": "\n", - "app/dummy_file.md": "\n", - }, - ), - { - CLICounterField.SNOWPARK_PROCESSOR: 0, - CLICounterField.TEMPLATES_PROCESSOR: 1, - CLICounterField.PDF_TEMPLATES: 1, - }, - ), - ( - "app run", - lambda: ProjectV2Factory( - pdf__entities=dict( - pkg=ApplicationPackageEntityModelFactory( - identifier="myapp_pkg", - artifacts=[ - "setup.sql", - "README.md", - "manifest.yml", - {"src": "app/*", "dest": "./", "processors": ["templates"]}, - ], - ), - app=ApplicationEntityModelFactory( - identifier="myapp", - fromm__target="pkg", - ), - ), - files={ - "setup.sql": "CREATE OR ALTER VERSIONED SCHEMA core;", - "README.md": "\n", - "manifest.yml": "\n", - "app/dummy_file.md": "\n", - }, - ), - { - CLICounterField.SNOWPARK_PROCESSOR: 0, - CLICounterField.TEMPLATES_PROCESSOR: 1, - CLICounterField.PDF_TEMPLATES: 0, - CLICounterField.POST_DEPLOY_SCRIPTS: 0, - }, - ), - ( - "app deploy", - lambda: ProjectV11Factory( - pdf__native_app__package__scripts=["scripts/package_script1.sql"], - pdf__native_app__artifacts=["README.md", "setup.sql", "manifest.yml"], - pdf__native_app__package__warehouse="non_existent_warehouse", - files={ - "README.md": "", - "setup.sql": "select 1", - "manifest.yml": "\n", - "scripts/package_script1.sql": "\n", - }, - ), - { - CLICounterField.SNOWPARK_PROCESSOR: 0, - CLICounterField.TEMPLATES_PROCESSOR: 0, - CLICounterField.PDF_TEMPLATES: 0, - CLICounterField.POST_DEPLOY_SCRIPTS: 1, - CLICounterField.PACKAGE_SCRIPTS: 1, - }, - ), - ( - "app deploy", - lambda: ProjectV2Factory( - pdf__entities=dict( - pkg=ApplicationPackageEntityModelFactory( - identifier="myapp_pkg", - meta__post_deploy=[ - {"sql_script": "scripts/pkg_post_deploy1.sql"}, - ], - ), - app=ApplicationEntityModelFactory( - identifier="myapp_<% ctx.env.USER %>", - fromm__target="pkg", - ), - ), - files={ - "setup.sql": "CREATE OR ALTER VERSIONED SCHEMA core;", - "README.md": "\n", - "manifest.yml": "\n", - "scripts/pkg_post_deploy1.sql": "-- package post-deploy script\n", - }, - ), - { - CLICounterField.SNOWPARK_PROCESSOR: 0, - CLICounterField.TEMPLATES_PROCESSOR: 0, - CLICounterField.PDF_TEMPLATES: 1, - CLICounterField.POST_DEPLOY_SCRIPTS: 1, - }, - ), - ], - ids=[ - "v1_post_deploy_set_and_package_scripts_available", - "v2_post_deploy_not_available_in_only_bundle", - "v2_templates_processor_set", - "v1_package_scripts_converted_to_post_deploy_both_set", - "v2_post_deploy_set_and_package_scripts_not_available", - ], -) -@mock.patch("snowflake.connector.telemetry.TelemetryClient.try_add_log_to_batch") -def test_nativeapp_feature_counter_has_expected_value( - mock_telemetry, - runner, - nativeapp_teardown, - temp_dir, - command: str, - project_factory: Callable, - expected_counters: Dict[str, int], -): - local_test_env = { - "APP_DIR": "app", - "schema_name": "test_schema", - "table_name": "test_table", - "value": "test_value", - } - - project_factory() - - with nativeapp_teardown(project_dir=Path(temp_dir)): - runner.invoke_with_connection(split(command), env=local_test_env) - - message = extract_first_telemetry_message_of_type( - mock_telemetry, TelemetryEvent.CMD_EXECUTION_RESULT.value - ) - - assert message[CLITelemetryField.COUNTERS.value] == expected_counters diff --git a/tests_integration/nativeapp/test_command_metric_spans.py b/tests_integration/nativeapp/test_command_metric_spans.py deleted file mode 100644 index ba9396161a..0000000000 --- a/tests_integration/nativeapp/test_command_metric_spans.py +++ /dev/null @@ -1,164 +0,0 @@ -# Copyright (c) 2024 Snowflake Inc. -# -# Licensed under the Apache License, Version 2.0 (the "License"); -# you may not use this file except in compliance with the License. -# You may obtain a copy of the License at -# -# http://www.apache.org/licenses/LICENSE-2.0 -# -# Unless required by applicable law or agreed to in writing, software -# distributed under the License is distributed on an "AS IS" BASIS, -# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. -# See the License for the specific language governing permissions and -# limitations under the License. -from shlex import split -from typing import Dict, Callable -from unittest import mock - -from snowflake.cli._app.telemetry import TelemetryEvent, CLITelemetryField -from snowflake.cli.api.metrics import CLICounterField, CLIMetricsSpan, CLIMetrics -from tests.nativeapp.factories import ( - ProjectV11Factory, - ProjectV2Factory, - ApplicationPackageEntityModelFactory, - ApplicationEntityModelFactory, -) -from tests.project.fixtures import * -from tests_common import temp_dir -from tests_integration.test_utils import extract_first_telemetry_message_of_type - -SPAN_KEYS_TO_CHECK = [CLIMetricsSpan.NAME_KEY, CLIMetricsSpan.PARENT_KEY] - - -@pytest.mark.integration -@pytest.mark.parametrize( - "command,project_factory,expected_spans", - [ - ( - "ws bundle --entity-id=pkg", - lambda: ProjectV2Factory( - pdf__entities=dict( - pkg=ApplicationPackageEntityModelFactory( - identifier="myapp_pkg_<% ctx.env.USER %>", - artifacts=[ - "setup.sql", - "README.md", - "manifest.yml", - ], - ), - app=ApplicationEntityModelFactory( - identifier="myapp", - fromm__target="pkg", - meta__post_deploy=[ - {"sql_script": "app_post_deploy1.sql"}, - ], - ), - ), - files={ - "app_post_deploy1.sql": "\n", - "setup.sql": "CREATE OR ALTER VERSIONED SCHEMA core;", - "README.md": "\n", - "manifest.yml": "\n", - }, - ), - [{"name": "ApplicationPackageEntity.action_bundle", "parent": None}], - ), - ( - "app run", - lambda: ProjectV2Factory( - pdf__entities=dict( - pkg=ApplicationPackageEntityModelFactory( - identifier="myapp_pkg", - artifacts=[ - "setup.sql", - "README.md", - "manifest.yml", - {"src": "app/*", "dest": "./", "processors": ["templates"]}, - ], - ), - app=ApplicationEntityModelFactory( - identifier="myapp", - fromm__target="pkg", - meta__post_deploy=[ - {"sql_script": "scripts/app_post_deploy1.sql"}, - ], - ), - ), - files={ - "setup.sql": "CREATE OR ALTER VERSIONED SCHEMA core;", - "README.md": "\n", - "manifest.yml": "\n", - "app/dummy_file.md": "\n", - "scripts/app_post_deploy1.sql": "-- package post-deploy script\n", - }, - ), - [ - { - "name": "ApplicationEntity.action_deploy", - "parent": None, - }, - { - "name": "compile_artifacts", - "parent": "ApplicationEntity.action_deploy", - }, - { - "name": "sync_deploy_root_with_stage", - "parent": "ApplicationEntity.action_deploy", - }, - { - "name": "get_validation_result", - "parent": "ApplicationEntity.action_deploy", - }, - { - "name": "create_or_upgrade_app", - "parent": "ApplicationEntity.action_deploy", - }, - { - "name": "execute_post_deploy_hooks", - "parent": "create_or_upgrade_app", - }, - ], - ), - ], - ids=["bundle_barebones_no_other_spans", "run_with_all_features_with_spans"], -) -@mock.patch("snowflake.connector.telemetry.TelemetryClient.try_add_log_to_batch") -def test_nativeapp_command_has_expected_spans( - mock_telemetry, - runner, - nativeapp_teardown, - temp_dir, - command: str, - project_factory: Callable, - expected_spans: Dict, -): - local_test_env = { - "APP_DIR": "app", - "schema_name": "test_schema", - "table_name": "test_table", - "value": "test_value", - } - - project_factory() - - with nativeapp_teardown(project_dir=Path(temp_dir)): - runner.invoke_with_connection(split(command), env=local_test_env) - - message = extract_first_telemetry_message_of_type( - mock_telemetry, TelemetryEvent.CMD_EXECUTION_RESULT.value - ) - - spans = message[CLITelemetryField.SPANS.value] - - # ensure spans are within our defined limits - assert spans[CLITelemetryField.NUM_SPANS_PAST_DEPTH_LIMIT.value] == 0 - assert spans[CLITelemetryField.NUM_SPANS_PAST_TOTAL_LIMIT.value] == 0 - assert all( - span[CLIMetricsSpan.TRIMMED_KEY] == False - for span in spans[CLITelemetryField.COMPLETED_SPANS.value] - ) - - assert expected_spans == [ - {key: span[key] for key in SPAN_KEYS_TO_CHECK} - for span in spans[CLITelemetryField.COMPLETED_SPANS.value] - ] diff --git a/tests_integration/nativeapp/test_metrics.py b/tests_integration/nativeapp/test_metrics.py new file mode 100644 index 0000000000..8b73208cc9 --- /dev/null +++ b/tests_integration/nativeapp/test_metrics.py @@ -0,0 +1,534 @@ +# Copyright (c) 2024 Snowflake Inc. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +from shlex import split +from typing import Dict, Callable +from unittest import mock + +from snowflake.cli._app.telemetry import TelemetryEvent, CLITelemetryField +from snowflake.cli.api.metrics import CLICounterField, CLIMetricsSpan +from tests.nativeapp.factories import ( + ProjectV11Factory, + ProjectV2Factory, + ApplicationPackageEntityModelFactory, + ApplicationEntityModelFactory, +) +from tests.project.fixtures import * +from tests_common import temp_dir +from tests_integration.test_utils import extract_first_telemetry_message_of_type + + +@pytest.mark.integration +@pytest.mark.parametrize( + "command,expected_counter", + [ + ( + [ + "sql", + "-q", + "select '<% ctx.env.test %>'", + "--env", + "test=value_from_cli", + ], + 1, + ), + (["sql", "-q", "select 'string'"], 0), + ], +) +@mock.patch("snowflake.connector.telemetry.TelemetryClient.try_add_log_to_batch") +def test_sql_templating_emits_counter( + mock_telemetry, + command: List[str], + expected_counter, + runner, +): + result = runner.invoke_with_connection_json(command) + + assert result.exit_code == 0 + + message = extract_first_telemetry_message_of_type( + mock_telemetry, TelemetryEvent.CMD_EXECUTION_RESULT.value + ) + + assert ( + message[CLITelemetryField.COUNTERS.value][CLICounterField.SQL_TEMPLATES] + == expected_counter + ) + + +@pytest.mark.integration +@mock.patch("snowflake.connector.telemetry.TelemetryClient.try_add_log_to_batch") +def test_feature_counters_v1_post_deploy_set_and_package_scripts_available( + mock_telemetry, + runner, + nativeapp_teardown, + temp_dir, +): + ProjectV11Factory( + pdf__native_app__artifacts=["README.md", "setup.sql", "manifest.yml"], + pdf__native_app__package__post_deploy=[ + {"sql_script": "post_deploy1.sql"}, + ], + files={ + "README.md": "", + "setup.sql": "select 1", + "manifest.yml": "\n", + "post_deploy1.sql": "\n", + }, + ) + + with nativeapp_teardown(project_dir=Path(temp_dir)): + runner.invoke_with_connection(["app", "deploy"]) + + message = extract_first_telemetry_message_of_type( + mock_telemetry, TelemetryEvent.CMD_EXECUTION_RESULT.value + ) + + assert message[CLITelemetryField.COUNTERS.value] == { + CLICounterField.SNOWPARK_PROCESSOR: 0, + CLICounterField.TEMPLATES_PROCESSOR: 0, + CLICounterField.PDF_TEMPLATES: 0, + CLICounterField.POST_DEPLOY_SCRIPTS: 1, + CLICounterField.PACKAGE_SCRIPTS: 0, + } + + +@pytest.mark.integration +@mock.patch("snowflake.connector.telemetry.TelemetryClient.try_add_log_to_batch") +def test_feature_counters_v2_post_deploy_not_available_in_bundle( + mock_telemetry, + runner, + nativeapp_teardown, + temp_dir, +): + ProjectV2Factory( + pdf__entities=dict( + pkg=ApplicationPackageEntityModelFactory( + identifier="myapp_pkg_<% ctx.env.USER %>", + artifacts=[ + "setup.sql", + "README.md", + "manifest.yml", + ], + ), + app=ApplicationEntityModelFactory( + identifier="myapp", + fromm__target="pkg", + meta__post_deploy=[ + {"sql_script": "app_post_deploy1.sql"}, + ], + ), + ), + files={ + "app_post_deploy1.sql": "\n", + "setup.sql": "CREATE OR ALTER VERSIONED SCHEMA core;", + "README.md": "\n", + "manifest.yml": "\n", + }, + ) + + with nativeapp_teardown(project_dir=Path(temp_dir)): + runner.invoke_with_connection(["ws", "bundle", "--entity-id=pkg"]) + + message = extract_first_telemetry_message_of_type( + mock_telemetry, TelemetryEvent.CMD_EXECUTION_RESULT.value + ) + + assert message[CLITelemetryField.COUNTERS.value] == { + CLICounterField.SNOWPARK_PROCESSOR: 0, + CLICounterField.TEMPLATES_PROCESSOR: 0, + CLICounterField.PDF_TEMPLATES: 1, + } + + +@pytest.mark.integration +@mock.patch("snowflake.connector.telemetry.TelemetryClient.try_add_log_to_batch") +def test_feature_counter_v2_templates_processor_set( + mock_telemetry, + runner, + nativeapp_teardown, + temp_dir, +): + ProjectV2Factory( + pdf__entities=dict( + pkg=ApplicationPackageEntityModelFactory( + identifier="myapp_pkg", + artifacts=[ + "setup.sql", + "README.md", + "manifest.yml", + {"src": "app/*", "dest": "./", "processors": ["templates"]}, + ], + ), + app=ApplicationEntityModelFactory( + identifier="myapp", + fromm__target="pkg", + ), + ), + files={ + "setup.sql": "CREATE OR ALTER VERSIONED SCHEMA core;", + "README.md": "\n", + "manifest.yml": "\n", + "app/dummy_file.md": "\n", + }, + ) + + with nativeapp_teardown(project_dir=Path(temp_dir)): + runner.invoke_with_connection(["app", "run"]) + + message = extract_first_telemetry_message_of_type( + mock_telemetry, TelemetryEvent.CMD_EXECUTION_RESULT.value + ) + + assert message[CLITelemetryField.COUNTERS.value] == { + CLICounterField.SNOWPARK_PROCESSOR: 0, + CLICounterField.TEMPLATES_PROCESSOR: 1, + CLICounterField.PDF_TEMPLATES: 0, + CLICounterField.POST_DEPLOY_SCRIPTS: 0, + } + + +@pytest.mark.integration +@mock.patch("snowflake.connector.telemetry.TelemetryClient.try_add_log_to_batch") +def test_feature_counter_v1_package_scripts_converted_to_post_deploy_and_both_set( + mock_telemetry, + runner, + nativeapp_teardown, + temp_dir, +): + ProjectV11Factory( + pdf__native_app__package__scripts=["scripts/package_script1.sql"], + pdf__native_app__artifacts=["README.md", "setup.sql", "manifest.yml"], + files={ + "README.md": "", + "setup.sql": "select 1", + "manifest.yml": "\n", + "scripts/package_script1.sql": "\n", + }, + ) + + with nativeapp_teardown(project_dir=Path(temp_dir)): + runner.invoke_with_connection(["app", "deploy"]) + + message = extract_first_telemetry_message_of_type( + mock_telemetry, TelemetryEvent.CMD_EXECUTION_RESULT.value + ) + + assert message[CLITelemetryField.COUNTERS.value] == { + CLICounterField.SNOWPARK_PROCESSOR: 0, + CLICounterField.TEMPLATES_PROCESSOR: 0, + CLICounterField.PDF_TEMPLATES: 0, + CLICounterField.POST_DEPLOY_SCRIPTS: 1, + CLICounterField.PACKAGE_SCRIPTS: 1, + } + + +@pytest.mark.integration +@mock.patch("snowflake.connector.telemetry.TelemetryClient.try_add_log_to_batch") +def test_feature_counter_v2_post_deploy_set_and_package_scripts_not_available( + mock_telemetry, + runner, + nativeapp_teardown, + temp_dir, +): + ProjectV2Factory( + pdf__entities=dict( + pkg=ApplicationPackageEntityModelFactory( + identifier="myapp_pkg", + meta__post_deploy=[ + {"sql_script": "scripts/pkg_post_deploy1.sql"}, + ], + ), + app=ApplicationEntityModelFactory( + identifier="myapp_<% ctx.env.USER %>", + fromm__target="pkg", + ), + ), + files={ + "setup.sql": "CREATE OR ALTER VERSIONED SCHEMA core;", + "README.md": "\n", + "manifest.yml": "\n", + "scripts/pkg_post_deploy1.sql": "-- package post-deploy script\n", + }, + ) + + with nativeapp_teardown(project_dir=Path(temp_dir)): + runner.invoke_with_connection(["app", "deploy"]) + + message = extract_first_telemetry_message_of_type( + mock_telemetry, TelemetryEvent.CMD_EXECUTION_RESULT.value + ) + + assert message[CLITelemetryField.COUNTERS.value] == { + CLICounterField.SNOWPARK_PROCESSOR: 0, + CLICounterField.TEMPLATES_PROCESSOR: 0, + CLICounterField.PDF_TEMPLATES: 1, + CLICounterField.POST_DEPLOY_SCRIPTS: 1, + } + + +def assert_spans_within_limits(spans: Dict): + assert spans[CLITelemetryField.NUM_SPANS_PAST_DEPTH_LIMIT.value] == 0 + assert spans[CLITelemetryField.NUM_SPANS_PAST_TOTAL_LIMIT.value] == 0 + assert all( + span[CLIMetricsSpan.TRIMMED_KEY] == False + for span in spans[CLITelemetryField.COMPLETED_SPANS.value] + ) + + +def extract_span_keys_to_check(spans: Dict): + SPAN_KEYS_TO_CHECK = [CLIMetricsSpan.NAME_KEY, CLIMetricsSpan.PARENT_KEY] + + return [ + {key: span[key] for key in SPAN_KEYS_TO_CHECK} + for span in spans[CLITelemetryField.COMPLETED_SPANS.value] + ] + + +@pytest.mark.integration +@mock.patch("snowflake.connector.telemetry.TelemetryClient.try_add_log_to_batch") +def test_spans_barebones_bundle_contains_no_spans( + mock_telemetry, + runner, + nativeapp_teardown, + temp_dir, +): + ProjectV2Factory( + pdf__entities=dict( + pkg=ApplicationPackageEntityModelFactory( + identifier="myapp_pkg", + artifacts=[ + "setup.sql", + "README.md", + "manifest.yml", + ], + ), + app=ApplicationEntityModelFactory( + identifier="myapp", + fromm__target="pkg", + ), + ), + files={ + "setup.sql": "CREATE OR ALTER VERSIONED SCHEMA core;", + "README.md": "\n", + "manifest.yml": "\n", + }, + ) + + with nativeapp_teardown(project_dir=Path(temp_dir)): + runner.invoke_with_connection(["ws", "bundle", "--entity-id=pkg"]) + + message = extract_first_telemetry_message_of_type( + mock_telemetry, TelemetryEvent.CMD_EXECUTION_RESULT.value + ) + + spans = message[CLITelemetryField.SPANS.value] + + assert_spans_within_limits(spans) + + assert extract_span_keys_to_check(spans) == [] + + +@pytest.mark.integration +@mock.patch("snowflake.connector.telemetry.TelemetryClient.try_add_log_to_batch") +def test_spans_run_with_all_features( + mock_telemetry, + runner, + nativeapp_teardown, + temp_dir, +): + ProjectV2Factory( + pdf__entities=dict( + pkg=ApplicationPackageEntityModelFactory( + identifier="myapp_pkg", + artifacts=[ + "setup.sql", + "README.md", + "manifest.yml", + {"src": "app/*", "dest": "./", "processors": ["templates"]}, + ], + meta__post_deploy=[ + {"sql_script": "scripts/pkg_post_deploy1.sql"}, + ], + ), + app=ApplicationEntityModelFactory( + identifier="myapp", + fromm__target="pkg", + meta__post_deploy=[ + {"sql_script": "scripts/app_post_deploy1.sql"}, + ], + ), + ), + files={ + "setup.sql": "CREATE OR ALTER VERSIONED SCHEMA core;", + "README.md": "\n", + "manifest.yml": "\n", + "app/dummy_file.md": "\n", + "scripts/app_post_deploy1.sql": "select '';", + "scripts/pkg_post_deploy1.sql": "select '';", + }, + ) + + with nativeapp_teardown(project_dir=Path(temp_dir)): + runner.invoke_with_connection(["app", "run"]) + + message = extract_first_telemetry_message_of_type( + mock_telemetry, TelemetryEvent.CMD_EXECUTION_RESULT.value + ) + + spans = message[CLITelemetryField.SPANS.value] + + assert_spans_within_limits(spans) + + assert extract_span_keys_to_check(spans) == [ + { + "name": "deploy_app", + "parent": None, + }, + { + "name": "deploy_app_package", + "parent": "deploy_app", + }, + { + "name": "build_initial_bundle", + "parent": "deploy_app_package", + }, + { + "name": "artifact_processors", + "parent": "deploy_app_package", + }, + { + "name": "templates_processor", + "parent": "artifact_processors", + }, + { + "name": "sync_remote_and_local_files", + "parent": "deploy_app_package", + }, + { + "name": "post_deploy_hooks", + "parent": "deploy_app_package", + }, + { + "name": "validate_setup_script", + "parent": "deploy_app_package", + }, + { + "name": "post_deploy_hooks", + "parent": "deploy_app", + }, + ] + + +@pytest.mark.integration +@mock.patch("snowflake.connector.telemetry.TelemetryClient.try_add_log_to_batch") +def test_spans_validate( + mock_telemetry, + runner, + nativeapp_teardown, + temp_dir, +): + ProjectV2Factory( + pdf__entities=dict( + pkg=ApplicationPackageEntityModelFactory( + identifier="myapp_pkg", + artifacts=[ + "setup.sql", + "README.md", + "manifest.yml", + ], + ), + app=ApplicationEntityModelFactory( + identifier="myapp", + fromm__target="pkg", + ), + ), + files={ + "setup.sql": "CREATE OR ALTER VERSIONED SCHEMA core;", + "README.md": "\n", + "manifest.yml": "\n", + }, + ) + + with nativeapp_teardown(project_dir=Path(temp_dir)): + runner.invoke_with_connection(["app", "validate"]) + + message = extract_first_telemetry_message_of_type( + mock_telemetry, TelemetryEvent.CMD_EXECUTION_RESULT.value + ) + + spans = message[CLITelemetryField.SPANS.value] + + assert_spans_within_limits(spans) + + assert extract_span_keys_to_check(spans) == [ + { + "name": "validate_setup_script", + "parent": None, + }, + { + "name": "deploy_app_package", + "parent": "validate_setup_script", + }, + { + "name": "sync_remote_and_local_files", + "parent": "deploy_app_package", + }, + ] + + +@pytest.mark.integration +@mock.patch("snowflake.connector.telemetry.TelemetryClient.try_add_log_to_batch") +def test_spans_teardown( + mock_telemetry, + runner, + nativeapp_teardown, + temp_dir, +): + ProjectV2Factory( + pdf__entities=dict( + pkg=ApplicationPackageEntityModelFactory( + identifier="myapp_pkg", + artifacts=[ + "setup.sql", + "README.md", + "manifest.yml", + ], + ), + app=ApplicationEntityModelFactory( + identifier="myapp", + fromm__target="pkg", + ), + ), + files={ + "setup.sql": "CREATE OR ALTER VERSIONED SCHEMA core;", + "README.md": "\n", + "manifest.yml": "\n", + }, + ) + + with nativeapp_teardown(project_dir=Path(temp_dir)): + runner.invoke_with_connection(["app", "teardown"]) + + message = extract_first_telemetry_message_of_type( + mock_telemetry, TelemetryEvent.CMD_EXECUTION_RESULT.value + ) + + spans = message[CLITelemetryField.SPANS.value] + + assert_spans_within_limits(spans) + + assert extract_span_keys_to_check(spans) == [ + {"name": "drop_app", "parent": None}, + {"name": "drop_app_package", "parent": None}, + ] From f99a2758f21f716159afa939fb8ad0f7ae060985 Mon Sep 17 00:00:00 2001 From: Marcus Chok Date: Tue, 12 Nov 2024 15:27:50 -0500 Subject: [PATCH 06/16] adjust tests for new spans --- tests_integration/nativeapp/test_metrics.py | 14 ++++++++++---- 1 file changed, 10 insertions(+), 4 deletions(-) diff --git a/tests_integration/nativeapp/test_metrics.py b/tests_integration/nativeapp/test_metrics.py index 8b73208cc9..d40dd19324 100644 --- a/tests_integration/nativeapp/test_metrics.py +++ b/tests_integration/nativeapp/test_metrics.py @@ -277,7 +277,7 @@ def test_feature_counter_v2_post_deploy_set_and_package_scripts_not_available( } -def assert_spans_within_limits(spans: Dict): +def assert_spans_within_limits(spans: Dict) -> None: assert spans[CLITelemetryField.NUM_SPANS_PAST_DEPTH_LIMIT.value] == 0 assert spans[CLITelemetryField.NUM_SPANS_PAST_TOTAL_LIMIT.value] == 0 assert all( @@ -286,7 +286,7 @@ def assert_spans_within_limits(spans: Dict): ) -def extract_span_keys_to_check(spans: Dict): +def extract_span_keys_to_check(spans: Dict) -> List[Dict]: SPAN_KEYS_TO_CHECK = [CLIMetricsSpan.NAME_KEY, CLIMetricsSpan.PARENT_KEY] return [ @@ -297,7 +297,7 @@ def extract_span_keys_to_check(spans: Dict): @pytest.mark.integration @mock.patch("snowflake.connector.telemetry.TelemetryClient.try_add_log_to_batch") -def test_spans_barebones_bundle_contains_no_spans( +def test_spans_bundle( mock_telemetry, runner, nativeapp_teardown, @@ -336,7 +336,9 @@ def test_spans_barebones_bundle_contains_no_spans( assert_spans_within_limits(spans) - assert extract_span_keys_to_check(spans) == [] + assert extract_span_keys_to_check(spans) == [ + {"name": "build_initial_bundle", "parent": None} + ] @pytest.mark.integration @@ -480,6 +482,10 @@ def test_spans_validate( "name": "deploy_app_package", "parent": "validate_setup_script", }, + { + "name": "build_initial_bundle", + "parent": "deploy_app_package", + }, { "name": "sync_remote_and_local_files", "parent": "deploy_app_package", From db00d949030c91777696a29122ac3b3d00d1bd8e Mon Sep 17 00:00:00 2001 From: Marcus Chok Date: Tue, 12 Nov 2024 15:52:20 -0500 Subject: [PATCH 07/16] time.monotonic() -> time.perf_counter() for proper ordering on windows --- src/snowflake/cli/api/metrics.py | 17 ++++++++--------- tests/api/test_metrics.py | 19 ++++--------------- 2 files changed, 12 insertions(+), 24 deletions(-) diff --git a/src/snowflake/cli/api/metrics.py b/src/snowflake/cli/api/metrics.py index dc8c8c1e21..6bd4ed4bf1 100644 --- a/src/snowflake/cli/api/metrics.py +++ b/src/snowflake/cli/api/metrics.py @@ -109,10 +109,8 @@ class for holding metrics span data and encapsulating related operations children: Set[CLIMetricsSpan] = field(init=False, default_factory=set) # private state - # start time of the step from the monotonic clock in order to calculate execution time - _monotonic_start: float = field( - init=False, default_factory=lambda: time.monotonic() - ) + # start time of the step from a performance counter in order to calculate execution time + _start_time: float = field(init=False, default_factory=time.perf_counter) def __hash__(self) -> int: return hash(self.span_id) @@ -147,7 +145,7 @@ def finish(self, error: Optional[BaseException] = None) -> None: if error: self.error = error - self.execution_time = time.monotonic() - self._monotonic_start + self.execution_time = time.perf_counter() - self._start_time def to_dict(self) -> Dict: """ @@ -184,9 +182,10 @@ class CLIMetrics: _in_progress_spans: List[CLIMetricsSpan] = field(init=False, default_factory=list) # list of finished steps for telemetry to process _completed_spans: List[CLIMetricsSpan] = field(init=False, default_factory=list) - # monotonic clock time of when this class was initialized to approximate when the command first started executing - _monotonic_start: float = field( - init=False, default_factory=lambda: time.monotonic(), compare=False + # clock time of a performance counter when this class was initialized + # to approximate when the command first started executing + _start_time: float = field( + init=False, default_factory=time.perf_counter, compare=False ) def clone(self) -> CLIMetrics: @@ -229,7 +228,7 @@ def span(self, name: str) -> Iterator[CLIMetricsSpan]: """ new_span = CLIMetricsSpan( name=name, - start_time=time.monotonic() - self._monotonic_start, + start_time=time.perf_counter() - self._start_time, parent=self.current_span, ) diff --git a/tests/api/test_metrics.py b/tests/api/test_metrics.py index 115f53da52..aef4399097 100644 --- a/tests/api/test_metrics.py +++ b/tests/api/test_metrics.py @@ -14,7 +14,6 @@ import uuid from itertools import count -from unittest import mock import pytest from snowflake.cli.api.metrics import ( @@ -110,14 +109,6 @@ def test_metrics_set_default_existing_counter(): assert metrics.counters == {"c2": 2} -# we need to mock time.monotonic because on windows it does not -# capture enough precision for these tests to not be flaky -@pytest.fixture -def mock_time_monotonic(): - with mock.patch("time.monotonic", side_effect=count()) as mocked_time_monotonic: - yield mocked_time_monotonic - - # helper for testing span limits def create_spans(metrics: CLIMetrics, width: int, depth: int): counter = count() @@ -146,7 +137,7 @@ def test_metrics_spans_initialization_empty(): assert metrics.num_spans_past_total_limit == 0 -def test_metrics_spans_single_span_no_error_or_parent(mock_time_monotonic): +def test_metrics_spans_single_span_no_error_or_parent(): # given metrics = CLIMetrics() @@ -191,7 +182,7 @@ def test_metrics_spans_finish_early_is_idempotent(): assert span1_dict[CLIMetricsSpan.EXECUTION_TIME_KEY] == execution_time -def test_metrics_spans_parent_with_one_child(mock_time_monotonic): +def test_metrics_spans_parent_with_one_child(): # given metrics = CLIMetrics() @@ -244,7 +235,7 @@ def test_metrics_spans_parent_with_one_child(mock_time_monotonic): ) -def test_metrics_spans_parent_with_two_children_same_name(mock_time_monotonic): +def test_metrics_spans_parent_with_two_children_same_name(): # given metrics = CLIMetrics() @@ -358,9 +349,7 @@ def test_metrics_spans_empty_name_raises_error(): assert err.match("span name must not be empty") -def test_metrics_spans_passing_depth_limit_should_add_to_counter_and_not_emit( - mock_time_monotonic, -): +def test_metrics_spans_passing_depth_limit_should_add_to_counter_and_not_emit(): # given metrics = CLIMetrics() From 84209f0c77ddb2ac4a30b7ad2acd52da496d5354 Mon Sep 17 00:00:00 2001 From: Marcus Chok Date: Tue, 12 Nov 2024 16:20:01 -0500 Subject: [PATCH 08/16] add span for getting snowsight url since it is a blind spot --- src/snowflake/cli/_plugins/nativeapp/entities/application.py | 1 + tests_integration/nativeapp/test_metrics.py | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/src/snowflake/cli/_plugins/nativeapp/entities/application.py b/src/snowflake/cli/_plugins/nativeapp/entities/application.py index 29ae85b762..bc33f58799 100644 --- a/src/snowflake/cli/_plugins/nativeapp/entities/application.py +++ b/src/snowflake/cli/_plugins/nativeapp/entities/application.py @@ -772,6 +772,7 @@ def stream_events( except KeyboardInterrupt: return + @span("get_snowsight_url_for_app") def get_snowsight_url(self) -> str: """Returns the URL that can be used to visit this app via Snowsight.""" name = identifier_for_url(self.name) diff --git a/tests_integration/nativeapp/test_metrics.py b/tests_integration/nativeapp/test_metrics.py index d40dd19324..557b1323ce 100644 --- a/tests_integration/nativeapp/test_metrics.py +++ b/tests_integration/nativeapp/test_metrics.py @@ -429,6 +429,10 @@ def test_spans_run_with_all_features( "name": "post_deploy_hooks", "parent": "deploy_app", }, + { + "name": "get_snowsight_url_for_app", + "parent": None, + }, ] From d5e2376a7c4d7329e782cfd11c11009ed1ec24ef Mon Sep 17 00:00:00 2001 From: Marcus Chok Date: Wed, 13 Nov 2024 15:39:41 -0500 Subject: [PATCH 09/16] add event sharing counters from merge conflicts to new file --- tests_integration/nativeapp/test_metrics.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/tests_integration/nativeapp/test_metrics.py b/tests_integration/nativeapp/test_metrics.py index 557b1323ce..1e822f628b 100644 --- a/tests_integration/nativeapp/test_metrics.py +++ b/tests_integration/nativeapp/test_metrics.py @@ -195,6 +195,9 @@ def test_feature_counter_v2_templates_processor_set( CLICounterField.TEMPLATES_PROCESSOR: 1, CLICounterField.PDF_TEMPLATES: 0, CLICounterField.POST_DEPLOY_SCRIPTS: 0, + CLICounterField.EVENT_SHARING: 0, + CLICounterField.EVENT_SHARING_ERROR: 0, + CLICounterField.EVENT_SHARING_WARNING: 0, } From 15955f7dcd818f1e2f4a1950e3ad175f9df45488 Mon Sep 17 00:00:00 2001 From: Marcus Chok Date: Wed, 13 Nov 2024 15:59:26 -0500 Subject: [PATCH 10/16] add span for create_or_upgrade_app to address blind spot --- .../cli/_plugins/nativeapp/entities/application.py | 1 + tests_integration/nativeapp/test_metrics.py | 6 +++++- 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/snowflake/cli/_plugins/nativeapp/entities/application.py b/src/snowflake/cli/_plugins/nativeapp/entities/application.py index b3b6525a53..87493a7146 100644 --- a/src/snowflake/cli/_plugins/nativeapp/entities/application.py +++ b/src/snowflake/cli/_plugins/nativeapp/entities/application.py @@ -631,6 +631,7 @@ def get_objects_owned_by_application(self) -> List[ApplicationOwnedObject]: ).fetchall() return [{"name": row[1], "type": row[2]} for row in results] + @span("update_app_object") def create_or_upgrade_app( self, package: ApplicationPackageEntity, diff --git a/tests_integration/nativeapp/test_metrics.py b/tests_integration/nativeapp/test_metrics.py index 1e822f628b..a12a5630e4 100644 --- a/tests_integration/nativeapp/test_metrics.py +++ b/tests_integration/nativeapp/test_metrics.py @@ -429,9 +429,13 @@ def test_spans_run_with_all_features( "parent": "deploy_app_package", }, { - "name": "post_deploy_hooks", + "name": "update_app_object", "parent": "deploy_app", }, + { + "name": "post_deploy_hooks", + "parent": "update_app_object", + }, { "name": "get_snowsight_url_for_app", "parent": None, From b77d13543bc6a04db97b53467d81ff3036c1aebe Mon Sep 17 00:00:00 2001 From: Marcus Chok Date: Thu, 14 Nov 2024 13:48:16 -0500 Subject: [PATCH 11/16] adjust span names for consistency --- .../cli/_plugins/nativeapp/artifacts.py | 2 +- .../nativeapp/entities/application.py | 9 ++-- .../nativeapp/entities/application_package.py | 9 ++-- src/snowflake/cli/api/console/console.py | 6 +-- src/snowflake/cli/api/entities/common.py | 18 ++++++++ src/snowflake/cli/api/entities/utils.py | 9 ++-- tests_integration/nativeapp/test_metrics.py | 44 +++++++++---------- 7 files changed, 58 insertions(+), 39 deletions(-) diff --git a/src/snowflake/cli/_plugins/nativeapp/artifacts.py b/src/snowflake/cli/_plugins/nativeapp/artifacts.py index 9c99cf8047..b5114e30f6 100644 --- a/src/snowflake/cli/_plugins/nativeapp/artifacts.py +++ b/src/snowflake/cli/_plugins/nativeapp/artifacts.py @@ -650,7 +650,7 @@ def resolve_without_follow(path: Path) -> Path: return Path(os.path.abspath(path)) -@span("build_initial_bundle") +@span("bundle") def build_bundle( project_root: Path, deploy_root: Path, diff --git a/src/snowflake/cli/_plugins/nativeapp/entities/application.py b/src/snowflake/cli/_plugins/nativeapp/entities/application.py index 87493a7146..040e60e82f 100644 --- a/src/snowflake/cli/_plugins/nativeapp/entities/application.py +++ b/src/snowflake/cli/_plugins/nativeapp/entities/application.py @@ -57,7 +57,11 @@ from snowflake.cli._plugins.workspace.context import ActionContext from snowflake.cli.api.cli_global_context import get_cli_context, span from snowflake.cli.api.console.abc import AbstractConsole -from snowflake.cli.api.entities.common import EntityBase, get_sql_executor +from snowflake.cli.api.entities.common import ( + EntityBase, + attach_spans_to_entity_actions, + get_sql_executor, +) from snowflake.cli.api.entities.utils import ( drop_generic_object, execute_post_deploy_hooks, @@ -321,6 +325,7 @@ def append_test_resource_suffix_to_identifier( return with_suffix +@attach_spans_to_entity_actions(entity_name="app") class ApplicationEntity(EntityBase[ApplicationEntityModel]): """ A Native App application object, created from an application package. @@ -355,7 +360,6 @@ def post_deploy_hooks(self) -> list[PostDeployHook] | None: model = self._entity_model return model.meta and model.meta.post_deploy - @span("deploy_app") def action_deploy( self, action_ctx: ActionContext, @@ -440,7 +444,6 @@ def action_deploy( interactive=interactive, ) - @span("drop_app") def action_drop( self, action_ctx: ActionContext, diff --git a/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py b/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py index 9f60355d03..52538cd222 100644 --- a/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py +++ b/src/snowflake/cli/_plugins/nativeapp/entities/application_package.py @@ -50,7 +50,11 @@ from snowflake.cli._plugins.stage.manager import StageManager from snowflake.cli._plugins.workspace.context import ActionContext from snowflake.cli.api.cli_global_context import span -from snowflake.cli.api.entities.common import EntityBase, get_sql_executor +from snowflake.cli.api.entities.common import ( + EntityBase, + attach_spans_to_entity_actions, + get_sql_executor, +) from snowflake.cli.api.entities.utils import ( drop_generic_object, execute_post_deploy_hooks, @@ -157,6 +161,7 @@ def validate_source_stage(cls, input_value: str): return input_value +@attach_spans_to_entity_actions(entity_name="app_pkg") class ApplicationPackageEntity(EntityBase[ApplicationPackageEntityModel]): """ A Native App application package. @@ -235,7 +240,6 @@ def action_deploy( force=force, ) - @span("drop_app_package") def action_drop(self, action_ctx: ActionContext, force_drop: bool, *args, **kwargs): console = self._workspace_ctx.console sql_executor = get_sql_executor() @@ -553,7 +557,6 @@ def _bundle(self): compiler.compile_artifacts() return bundle_map - @span("deploy_app_package") def _deploy( self, bundle_map: BundleMap | None, diff --git a/src/snowflake/cli/api/console/console.py b/src/snowflake/cli/api/console/console.py index b294d4ccc6..94c80c9cc9 100644 --- a/src/snowflake/cli/api/console/console.py +++ b/src/snowflake/cli/api/console/console.py @@ -64,11 +64,7 @@ def _format_message(self, message: str, output: Output) -> Text: return text @contextmanager - def phase( - self, - enter_message: str, - exit_message: Optional[str] = None, - ): + def phase(self, enter_message: str, exit_message: Optional[str] = None): """A context manager for organising steps into logical group.""" self._print(self._format_message(enter_message, Output.PHASE)) self._extra_indent += 1 diff --git a/src/snowflake/cli/api/entities/common.py b/src/snowflake/cli/api/entities/common.py index 9f98f970ec..3c377bae2f 100644 --- a/src/snowflake/cli/api/entities/common.py +++ b/src/snowflake/cli/api/entities/common.py @@ -2,6 +2,7 @@ from typing import Generic, Type, TypeVar, get_args from snowflake.cli._plugins.workspace.context import ActionContext, WorkspaceContext +from snowflake.cli.api.cli_global_context import span from snowflake.cli.api.sql_execution import SqlExecutor @@ -20,6 +21,23 @@ class EntityActions(str, Enum): T = TypeVar("T") +def attach_spans_to_entity_actions(entity_name: str): + def decorator(cls): + for attr_name, attr_value in vars(cls).items(): + is_entity_action = attr_name in [ + enum_member for enum_member in EntityActions + ] + + if is_entity_action and callable(attr_value): + attr_name_without_action_prefix = attr_name.partition("_")[2] + span_name = f"action.{entity_name}.{attr_name_without_action_prefix}" + action_with_span = span(span_name)(attr_value) + setattr(cls, attr_name, action_with_span) + return cls + + return decorator + + class EntityBase(Generic[T]): """ Base class for the fully-featured entity classes. diff --git a/src/snowflake/cli/api/entities/utils.py b/src/snowflake/cli/api/entities/utils.py index 39322364ae..4c5b8b0c78 100644 --- a/src/snowflake/cli/api/entities/utils.py +++ b/src/snowflake/cli/api/entities/utils.py @@ -76,7 +76,7 @@ def _get_stage_paths_to_sync( return stage_paths -@span("sync_remote_and_local_files") +@span("sync_deploy_root_with_stage") def sync_deploy_root_with_stage( console: AbstractConsole, deploy_root: Path, @@ -213,9 +213,10 @@ def execute_post_deploy_hooks( get_cli_context().metrics.set_counter(CLICounterField.POST_DEPLOY_SCRIPTS, 1) - with console.phase( - f"Executing {deployed_object_type} post-deploy actions", - ), get_cli_context().metrics.span("post_deploy_hooks"): + with ( + console.phase(f"Executing {deployed_object_type} post-deploy actions"), + get_cli_context().metrics.span("post_deploy_hooks"), + ): sql_scripts_paths = [] display_paths = [] for hook in post_deploy_hooks: diff --git a/tests_integration/nativeapp/test_metrics.py b/tests_integration/nativeapp/test_metrics.py index a12a5630e4..a1678b3c1c 100644 --- a/tests_integration/nativeapp/test_metrics.py +++ b/tests_integration/nativeapp/test_metrics.py @@ -339,9 +339,7 @@ def test_spans_bundle( assert_spans_within_limits(spans) - assert extract_span_keys_to_check(spans) == [ - {"name": "build_initial_bundle", "parent": None} - ] + assert extract_span_keys_to_check(spans) == [{"name": "bundle", "parent": None}] @pytest.mark.integration @@ -397,40 +395,40 @@ def test_spans_run_with_all_features( assert extract_span_keys_to_check(spans) == [ { - "name": "deploy_app", + "name": "action.app.deploy", "parent": None, }, { - "name": "deploy_app_package", - "parent": "deploy_app", + "name": "action.app_pkg.deploy", + "parent": "action.app.deploy", }, { - "name": "build_initial_bundle", - "parent": "deploy_app_package", + "name": "bundle", + "parent": "action.app_pkg.deploy", }, { "name": "artifact_processors", - "parent": "deploy_app_package", + "parent": "action.app_pkg.deploy", }, { "name": "templates_processor", "parent": "artifact_processors", }, { - "name": "sync_remote_and_local_files", - "parent": "deploy_app_package", + "name": "sync_deploy_root_with_stage", + "parent": "action.app_pkg.deploy", }, { "name": "post_deploy_hooks", - "parent": "deploy_app_package", + "parent": "action.app_pkg.deploy", }, { "name": "validate_setup_script", - "parent": "deploy_app_package", + "parent": "action.app_pkg.deploy", }, { "name": "update_app_object", - "parent": "deploy_app", + "parent": "action.app.deploy", }, { "name": "post_deploy_hooks", @@ -486,20 +484,20 @@ def test_spans_validate( assert extract_span_keys_to_check(spans) == [ { - "name": "validate_setup_script", + "name": "action.app_pkg.validate", "parent": None, }, { - "name": "deploy_app_package", - "parent": "validate_setup_script", + "name": "validate_setup_script", + "parent": "action.app_pkg.validate", }, { - "name": "build_initial_bundle", - "parent": "deploy_app_package", + "name": "bundle", + "parent": "validate_setup_script", }, { - "name": "sync_remote_and_local_files", - "parent": "deploy_app_package", + "name": "sync_deploy_root_with_stage", + "parent": "validate_setup_script", }, ] @@ -546,6 +544,6 @@ def test_spans_teardown( assert_spans_within_limits(spans) assert extract_span_keys_to_check(spans) == [ - {"name": "drop_app", "parent": None}, - {"name": "drop_app_package", "parent": None}, + {"name": "action.app.drop", "parent": None}, + {"name": "action.app_pkg.drop", "parent": None}, ] From cef9f194c72279642856d53822ad02b292b9dba6 Mon Sep 17 00:00:00 2001 From: Marcus Chok Date: Thu, 14 Nov 2024 13:55:39 -0500 Subject: [PATCH 12/16] add docstring for entity action span decorator --- src/snowflake/cli/api/entities/common.py | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/src/snowflake/cli/api/entities/common.py b/src/snowflake/cli/api/entities/common.py index 3c377bae2f..021830fcb2 100644 --- a/src/snowflake/cli/api/entities/common.py +++ b/src/snowflake/cli/api/entities/common.py @@ -22,7 +22,15 @@ class EntityActions(str, Enum): def attach_spans_to_entity_actions(entity_name: str): - def decorator(cls): + """ + Class decorator for EntityBase subclasses to automatically wrap + every implemented entity action method with a metrics span + + Args: + entity_name (str): Custom name for entity type to be displayed in metrics + """ + + def decorator(cls: type[EntityBase]) -> type[EntityBase]: for attr_name, attr_value in vars(cls).items(): is_entity_action = attr_name in [ enum_member for enum_member in EntityActions From 8a4519256e980f78137100bc009fb452669a98c7 Mon Sep 17 00:00:00 2001 From: Marcus Chok Date: Thu, 14 Nov 2024 14:25:41 -0500 Subject: [PATCH 13/16] fix spans bundle test --- tests_integration/nativeapp/test_metrics.py | 5 ++++- 1 file changed, 4 insertions(+), 1 deletion(-) diff --git a/tests_integration/nativeapp/test_metrics.py b/tests_integration/nativeapp/test_metrics.py index a1678b3c1c..f83f3895b2 100644 --- a/tests_integration/nativeapp/test_metrics.py +++ b/tests_integration/nativeapp/test_metrics.py @@ -339,7 +339,10 @@ def test_spans_bundle( assert_spans_within_limits(spans) - assert extract_span_keys_to_check(spans) == [{"name": "bundle", "parent": None}] + assert extract_span_keys_to_check(spans) == [ + {"name": "action.app_pkg.bundle", "parent": None}, + {"name": "bundle", "parent": "action.app_pkg.bundle"}, + ] @pytest.mark.integration From ae43f25d8835785a4887339f688dfa953f86ca22 Mon Sep 17 00:00:00 2001 From: Marcus Chok Date: Mon, 18 Nov 2024 09:45:10 -0500 Subject: [PATCH 14/16] sort spans deterministically by creation order --- src/snowflake/cli/api/metrics.py | 12 ++++++++++-- 1 file changed, 10 insertions(+), 2 deletions(-) diff --git a/src/snowflake/cli/api/metrics.py b/src/snowflake/cli/api/metrics.py index d950a92e35..56e958783b 100644 --- a/src/snowflake/cli/api/metrics.py +++ b/src/snowflake/cli/api/metrics.py @@ -18,6 +18,7 @@ from contextlib import contextmanager from dataclasses import dataclass, field, replace from heapq import nsmallest +from itertools import count from typing import ClassVar, Dict, Iterator, List, Optional, Set @@ -99,6 +100,9 @@ class for holding metrics span data and encapsulating related operations # denotes whether direct children were trimmed from telemetry payload TRIMMED_KEY: ClassVar[str] = "trimmed" + # counter for sorting by creation order + _CREATION_COUNTER: ClassVar[count] = count() + # constructor vars name: str start_time: float # relative to when the command first started executing @@ -130,6 +134,8 @@ def __post_init__(self): self.parent.add_child(self) self.span_depth = self.parent.span_depth + 1 + self.creation_key = next(self._CREATION_COUNTER) + def increment_subtree_node_count(self) -> None: self.span_count_in_subtree += 1 @@ -293,9 +299,11 @@ def completed_spans(self) -> List[Dict]: ) ) - # sort by start time to make reading the payload easier sorted_spans_to_report = sorted( - spans_to_report, key=lambda span: span.start_time + spans_to_report, + # start_time can be the same, so we want to sort by something more + # deterministic, so we use a counter to see which spans are started first + key=lambda span: span.creation_key, ) return [ From c618da6c89e7d27be238cb21775dcb3dbd1621bd Mon Sep 17 00:00:00 2001 From: Marcus Chok Date: Tue, 19 Nov 2024 17:26:53 -0500 Subject: [PATCH 15/16] only one sort, remove counter --- src/snowflake/cli/api/metrics.py | 37 +++++--------- tests_integration/nativeapp/test_metrics.py | 56 ++++++++++----------- 2 files changed, 40 insertions(+), 53 deletions(-) diff --git a/src/snowflake/cli/api/metrics.py b/src/snowflake/cli/api/metrics.py index 56e958783b..69778872e1 100644 --- a/src/snowflake/cli/api/metrics.py +++ b/src/snowflake/cli/api/metrics.py @@ -18,7 +18,6 @@ from contextlib import contextmanager from dataclasses import dataclass, field, replace from heapq import nsmallest -from itertools import count from typing import ClassVar, Dict, Iterator, List, Optional, Set @@ -100,9 +99,6 @@ class for holding metrics span data and encapsulating related operations # denotes whether direct children were trimmed from telemetry payload TRIMMED_KEY: ClassVar[str] = "trimmed" - # counter for sorting by creation order - _CREATION_COUNTER: ClassVar[count] = count() - # constructor vars name: str start_time: float # relative to when the command first started executing @@ -134,8 +130,6 @@ def __post_init__(self): self.parent.add_child(self) self.span_depth = self.parent.span_depth + 1 - self.creation_key = next(self._CREATION_COUNTER) - def increment_subtree_node_count(self) -> None: self.span_count_in_subtree += 1 @@ -280,36 +274,29 @@ def num_spans_past_total_limit(self) -> int: @property def completed_spans(self) -> List[Dict]: """ - Returns the completed spans tracked throughout a command, sorted by start time, for reporting telemetry + Returns the completed spans tracked throughout a command for reporting telemetry Ensures that the spans we send are within the configured limits and marks certain spans as trimmed if their children would bypass the limits we set """ # take spans breadth-first within the depth and total limits # since we care more about the big picture than granularity - spans_to_report = set( - nsmallest( - n=self.SPAN_TOTAL_LIMIT, - iterable=( - span - for span in self._completed_spans - if span.span_depth <= self.SPAN_DEPTH_LIMIT - ), - key=lambda span: (span.span_depth, span.start_time), - ) + spans_to_report = nsmallest( + n=self.SPAN_TOTAL_LIMIT, + iterable=( + span + for span in self._completed_spans + if span.span_depth <= self.SPAN_DEPTH_LIMIT + ), + key=lambda span: (span.span_depth, span.start_time, span.execution_time), ) - sorted_spans_to_report = sorted( - spans_to_report, - # start_time can be the same, so we want to sort by something more - # deterministic, so we use a counter to see which spans are started first - key=lambda span: span.creation_key, - ) + spans_to_report_set = set(spans_to_report) return [ { **span.to_dict(), - CLIMetricsSpan.TRIMMED_KEY: not span.children <= spans_to_report, + CLIMetricsSpan.TRIMMED_KEY: not span.children <= spans_to_report_set, } - for span in sorted_spans_to_report + for span in spans_to_report ] diff --git a/tests_integration/nativeapp/test_metrics.py b/tests_integration/nativeapp/test_metrics.py index f83f3895b2..a18d5183a2 100644 --- a/tests_integration/nativeapp/test_metrics.py +++ b/tests_integration/nativeapp/test_metrics.py @@ -28,6 +28,24 @@ from tests_integration.test_utils import extract_first_telemetry_message_of_type +def assert_spans_within_limits(spans: Dict) -> None: + assert spans[CLITelemetryField.NUM_SPANS_PAST_DEPTH_LIMIT.value] == 0 + assert spans[CLITelemetryField.NUM_SPANS_PAST_TOTAL_LIMIT.value] == 0 + assert all( + span[CLIMetricsSpan.TRIMMED_KEY] == False + for span in spans[CLITelemetryField.COMPLETED_SPANS.value] + ) + + +def extract_span_keys_to_check(spans: Dict) -> List[Dict]: + SPAN_KEYS_TO_CHECK = [CLIMetricsSpan.NAME_KEY, CLIMetricsSpan.PARENT_KEY] + + return [ + {key: span[key] for key in SPAN_KEYS_TO_CHECK} + for span in spans[CLITelemetryField.COMPLETED_SPANS.value] + ] + + @pytest.mark.integration @pytest.mark.parametrize( "command,expected_counter", @@ -280,24 +298,6 @@ def test_feature_counter_v2_post_deploy_set_and_package_scripts_not_available( } -def assert_spans_within_limits(spans: Dict) -> None: - assert spans[CLITelemetryField.NUM_SPANS_PAST_DEPTH_LIMIT.value] == 0 - assert spans[CLITelemetryField.NUM_SPANS_PAST_TOTAL_LIMIT.value] == 0 - assert all( - span[CLIMetricsSpan.TRIMMED_KEY] == False - for span in spans[CLITelemetryField.COMPLETED_SPANS.value] - ) - - -def extract_span_keys_to_check(spans: Dict) -> List[Dict]: - SPAN_KEYS_TO_CHECK = [CLIMetricsSpan.NAME_KEY, CLIMetricsSpan.PARENT_KEY] - - return [ - {key: span[key] for key in SPAN_KEYS_TO_CHECK} - for span in spans[CLITelemetryField.COMPLETED_SPANS.value] - ] - - @pytest.mark.integration @mock.patch("snowflake.connector.telemetry.TelemetryClient.try_add_log_to_batch") def test_spans_bundle( @@ -401,10 +401,18 @@ def test_spans_run_with_all_features( "name": "action.app.deploy", "parent": None, }, + { + "name": "get_snowsight_url_for_app", + "parent": None, + }, { "name": "action.app_pkg.deploy", "parent": "action.app.deploy", }, + { + "name": "update_app_object", + "parent": "action.app.deploy", + }, { "name": "bundle", "parent": "action.app_pkg.deploy", @@ -413,10 +421,6 @@ def test_spans_run_with_all_features( "name": "artifact_processors", "parent": "action.app_pkg.deploy", }, - { - "name": "templates_processor", - "parent": "artifact_processors", - }, { "name": "sync_deploy_root_with_stage", "parent": "action.app_pkg.deploy", @@ -429,17 +433,13 @@ def test_spans_run_with_all_features( "name": "validate_setup_script", "parent": "action.app_pkg.deploy", }, - { - "name": "update_app_object", - "parent": "action.app.deploy", - }, { "name": "post_deploy_hooks", "parent": "update_app_object", }, { - "name": "get_snowsight_url_for_app", - "parent": None, + "name": "templates_processor", + "parent": "artifact_processors", }, ] From e96a0ee7c84447d3b16e975ba72fbb88eb0f0123 Mon Sep 17 00:00:00 2001 From: Marcus Chok Date: Thu, 21 Nov 2024 09:48:31 -0500 Subject: [PATCH 16/16] update comment for span decorator to reflect new method name --- src/snowflake/cli/api/cli_global_context.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/snowflake/cli/api/cli_global_context.py b/src/snowflake/cli/api/cli_global_context.py index ed5b49db12..abd4ce09a9 100644 --- a/src/snowflake/cli/api/cli_global_context.py +++ b/src/snowflake/cli/api/cli_global_context.py @@ -218,7 +218,7 @@ def span(span_name: str): """ Decorator to start a command metrics span that encompasses a whole function - Must be used instead of directly calling @get_cli_context().metrics.start_span(span_name) + Must be used instead of directly calling @get_cli_context().metrics.span(span_name) as a decorator to ensure that the cli context is grabbed at run time instead of at module load time, which would not reflect forking """