Skip to content
New issue

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

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

Already on GitHub? Sign in to your account

Enable Event Sharing V2 in SnowCLI #1823

Open
wants to merge 2 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions RELEASE-NOTES.md
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
## New additions
* Added `--retain-comments` option to `snow sql` command to allow passing comments to Snowflake.
* Added `--replace` and `--if-not-exists` options to `snow object create` command.
* Added support for event sharing, which now can be specified under the `telemetry` section of an application entity. Two fields are supported: `authorize_event_sharing` and `optional_shared_events`.
* `snow stage copy` supports `--recursive` flag to copy local files and subdirectories recursively to stage. Including
glob support.

Expand Down
65 changes: 56 additions & 9 deletions src/snowflake/cli/_plugins/connection/util.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,20 +17,16 @@
import json
import logging
import os
from typing import Optional
from enum import Enum
from functools import lru_cache
from typing import Any, Dict, Optional

from click.exceptions import ClickException
from snowflake.connector import SnowflakeConnection
from snowflake.connector.cursor import DictCursor

log = logging.getLogger(__name__)

REGIONLESS_QUERY = """
select value['value'] as REGIONLESS from table(flatten(
input => parse_json(SYSTEM$BOOTSTRAP_DATA_REQUEST()),
path => 'clientParamsInfo'
)) where value['name'] = 'UI_SNOWSIGHT_ENABLE_REGIONLESS_REDIRECT';
"""

ALLOWLIST_QUERY = "SELECT SYSTEM$ALLOWLIST()"
SNOWFLAKE_DEPLOYMENT = "SNOWFLAKE_DEPLOYMENT"
Expand All @@ -54,6 +50,53 @@ def __init__(self, host: str | None):
)


class UIParameter(Enum):
ENABLE_REGIONLESS_REDIRECT = "UI_SNOWSIGHT_ENABLE_REGIONLESS_REDIRECT"
EVENT_SHARING_V2 = "ENABLE_EVENT_SHARING_V2_IN_THE_SAME_ACCOUNT"
ENFORCE_MANDATORY_FILTERS = (
"ENFORCE_MANDATORY_FILTERS_FOR_SAME_ACCOUNT_INSTALLATION"
)


def get_ui_parameter(
conn: SnowflakeConnection, parameter: UIParameter, default: Any
) -> str:
"""
Returns the value of a single UI parameter.
If the parameter is not found, the default value is returned.
"""

if not isinstance(parameter, UIParameter):
raise ValueError("Parameter must be a UIParameters enum")

ui_parameters = get_ui_parameters(conn)
return ui_parameters.get(parameter, default)


@lru_cache(maxsize=None)
def get_ui_parameters(conn: SnowflakeConnection) -> Dict[UIParameter, Any]:
"""
Returns the UI parameters from the SYSTEM$BOOTSTRAP_DATA_REQUEST function
"""

parameters_to_fetch = sorted(
[param.value for param in UIParameter.__members__.values()]
)

query = f"""
select value['value']::string as PARAM_VALUE, value['name']::string as PARAM_NAME from table(flatten(
input => parse_json(SYSTEM$BOOTSTRAP_DATA_REQUEST()),
sfc-gh-turbaszek marked this conversation as resolved.
Show resolved Hide resolved
path => 'clientParamsInfo'
)) where value['name'] in ('{"', '".join(parameters_to_fetch)}');
"""

*_, cursor = conn.execute_string(query, cursor_class=DictCursor)

return {
UIParameter(row["PARAM_NAME"]): row["PARAM_VALUE"] for row in cursor.fetchall()
}


def is_regionless_redirect(conn: SnowflakeConnection) -> bool:
"""
Determines if the deployment this connection refers to uses
Expand All @@ -62,8 +105,12 @@ def is_regionless_redirect(conn: SnowflakeConnection) -> bool:
assume it's regionless, as this is true for most production deployments.
"""
try:
*_, cursor = conn.execute_string(REGIONLESS_QUERY, cursor_class=DictCursor)
return cursor.fetchone()["REGIONLESS"].lower() == "true"
return (
get_ui_parameter(
conn, UIParameter.ENABLE_REGIONLESS_REDIRECT, "true"
).lower()
== "true"
)
except:
log.warning(
"Cannot determine regionless redirect; assuming True.", exc_info=True
Expand Down
37 changes: 34 additions & 3 deletions src/snowflake/cli/_plugins/nativeapp/artifacts.py
Original file line number Diff line number Diff line change
Expand Up @@ -164,7 +164,7 @@ def put(self, src: Path, dest: Path, dest_is_dir: bool) -> None:
if src_is_dir:
# mark all subdirectories of this source as directories so that we can
# detect accidental clobbering
for (root, _, files) in os.walk(absolute_src, followlinks=True):
for root, _, files in os.walk(absolute_src, followlinks=True):
canonical_subdir = Path(root).relative_to(absolute_src)
canonical_dest_subdir = dest / canonical_subdir
self._update_dest_is_dir(canonical_dest_subdir, is_dir=True)
Expand Down Expand Up @@ -383,7 +383,7 @@ def _expand_artifact_mapping(
if absolute_src.is_dir() and expand_directories:
# both src and dest are directories, and expanding directories was requested. Traverse src, and map each
# file to the dest directory
for (root, subdirs, files) in os.walk(absolute_src, followlinks=True):
for root, subdirs, files in os.walk(absolute_src, followlinks=True):
relative_root = Path(root).relative_to(absolute_src)
for name in itertools.chain(subdirs, files):
src_file_for_output = src_for_output / relative_root / name
Expand Down Expand Up @@ -683,7 +683,7 @@ def build_bundle(
"No artifacts mapping found in project definition, nothing to do."
)

for (absolute_src, absolute_dest) in bundle_map.all_mappings(
for absolute_src, absolute_dest in bundle_map.all_mappings(
absolute=True, expand_directories=False
):
symlink_or_copy(absolute_src, absolute_dest, deploy_root=deploy_root)
Expand Down Expand Up @@ -765,3 +765,34 @@ def find_version_info_in_manifest_file(
patch_number = int(version_info[patch_field])

return version_name, patch_number


def find_mandatory_events_in_manifest_file(
deploy_root: Path,
) -> List[str]:
"""
Find mandatory events, if available, in the manifest.yml file.
Mandatory events can be found under this section in the manifest.yml file:

configuration:
telemetry_event_definitions:
- type: ERRORS_AND_WARNINGS
sharing: MANDATORY
- type: DEBUG_LOGS
sharing: OPTIONAL
"""
manifest_content = find_and_read_manifest_file(deploy_root=deploy_root)

mandatory_events: List[str] = []

configuration_section = manifest_content.get("configuration", None)
if configuration_section:
telemetry_section = configuration_section.get(
"telemetry_event_definitions", None
)
if telemetry_section:
for event in telemetry_section:
if event.get("sharing", None) == "MANDATORY":
mandatory_events.append(event["type"])

return mandatory_events
111 changes: 108 additions & 3 deletions src/snowflake/cli/_plugins/nativeapp/entities/application.py
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,14 @@
import typer
from click import ClickException, UsageError
from pydantic import Field, field_validator
from snowflake.cli._plugins.connection.util import make_snowsight_url
from snowflake.cli._plugins.connection.util import (
UIParameter,
get_ui_parameter,
make_snowsight_url,
)
from snowflake.cli._plugins.nativeapp.artifacts import (
find_mandatory_events_in_manifest_file,
)
from snowflake.cli._plugins.nativeapp.common_flags import (
ForceOption,
InteractiveOption,
Expand All @@ -27,6 +34,9 @@
ApplicationPackageEntity,
ApplicationPackageEntityModel,
)
from snowflake.cli._plugins.nativeapp.entities.models.event_sharing_telemetry import (
EventSharingTelemetry,
)
from snowflake.cli._plugins.nativeapp.exceptions import (
ApplicationPackageDoesNotExistError,
NoEventTableForAccount,
Expand Down Expand Up @@ -73,7 +83,7 @@
to_identifier,
unquote_identifier,
)
from snowflake.connector import DictCursor, ProgrammingError
from snowflake.connector import DictCursor, ProgrammingError, SnowflakeConnection

# Reasons why an `alter application ... upgrade` might fail
UPGRADE_RESTRICTION_CODES = {
Expand All @@ -97,6 +107,10 @@ class ApplicationEntityModel(EntityModelBase):
title="Whether to enable debug mode when using a named stage to create an application object",
default=None,
)
telemetry: Optional[EventSharingTelemetry] = Field(
title="Telemetry configuration for the application",
default=None,
)

@field_validator("identifier")
@classmethod
Expand Down Expand Up @@ -427,6 +441,71 @@ def get_objects_owned_by_application(self) -> List[ApplicationOwnedObject]:
).fetchall()
return [{"name": row[1], "type": row[2]} for row in results]

def _should_authorize_event_sharing(
self,
install_method: SameAccountInstallMethod,
connection: SnowflakeConnection,
deploy_root: str,
) -> Optional[bool]:
"""
Logic to determine whether event sharing should be authorized or not.
If the return value is None, it means that authorize_event_sharing should not be updated.
If the return value is True/False, it means that authorize_event_sharing should be set to True/False respectively.
"""

model = self._entity_model
workspace_ctx = self._workspace_ctx
console = workspace_ctx.console
project_root = workspace_ctx.project_root
Comment on lines +457 to +459
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
workspace_ctx = self._workspace_ctx
console = workspace_ctx.console
project_root = workspace_ctx.project_root
console = self._workspace_ctx.console

is_dev_mode = install_method.is_dev_mode

mandatory_events_found = (
len(find_mandatory_events_in_manifest_file(project_root / deploy_root)) > 0
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
len(find_mandatory_events_in_manifest_file(project_root / deploy_root)) > 0
len(find_mandatory_events_in_manifest_file(self.project_root / deploy_root)) > 0

)
event_sharing_enabled = (
get_ui_parameter(connection, UIParameter.EVENT_SHARING_V2, "true").lower()
== "true"
)
event_sharing_enforced = (
get_ui_parameter(
connection, UIParameter.ENFORCE_MANDATORY_FILTERS, "true"
).lower()
== "true"
)
authorize_event_sharing = (
model.telemetry and model.telemetry.authorize_event_sharing
)

if not event_sharing_enabled:
# We cannot set AUTHORIZE_TELEMETRY_EVENT_SHARING to True or False if event sharing is not enabled,
# so we will ignore the field in both cases, but warn only if it is set to True
if authorize_event_sharing:
console.warning(
"WARNING: Same-account event sharing is not enabled in your account, therefore, application telemetry field will be ignored."
)
return None

if event_sharing_enabled and not event_sharing_enforced:
# if event sharing is enabled but not enforced yet, warn about future enforcement
if mandatory_events_found and not authorize_event_sharing:
console.warning(
"WARNING: Mandatory events are present in the manifest file, but event sharing is not authorized in the application telemetry field. This will soon be required to set in order to deploy applications."
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"WARNING: Mandatory events are present in the manifest file, but event sharing is not authorized in the application telemetry field. This will soon be required to set in order to deploy applications."
"WARNING: Mandatory events are present in the manifest file, but event sharing is not authorized in the application telemetry field. This will soon be required to set in order to deploy this application."

)

if event_sharing_enabled and event_sharing_enforced:
if mandatory_events_found:
if authorize_event_sharing is None and is_dev_mode:
console.warning(
"WARNING: Mandatory events are present in the manifest file. Automatically authorizing event sharing in dev mode. To suppress this warning, please add authorize_event_sharing field in the application telemetry section."
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"WARNING: Mandatory events are present in the manifest file. Automatically authorizing event sharing in dev mode. To suppress this warning, please add authorize_event_sharing field in the application telemetry section."
'WARNING: Mandatory events are present in the manifest file. Automatically authorizing event sharing in dev mode. To suppress this warning, please add "authorize_event_sharing: true" in the application telemetry section.'

)
return True
elif not authorize_event_sharing:
raise ClickException(
"Mandatory events are present in the manifest file, but event sharing is not authorized in the application telemetry field. This is required to deploy applications."
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
"Mandatory events are present in the manifest file, but event sharing is not authorized in the application telemetry field. This is required to deploy applications."
"Mandatory events are present in the manifest file, but event sharing is not authorized in the application telemetry field. This is required to deploy this application."

)

return authorize_event_sharing

def create_or_upgrade_app(
self,
package: ApplicationPackageEntity,
Expand All @@ -443,6 +522,14 @@ def create_or_upgrade_app(

sql_executor = get_sql_executor()
with sql_executor.use_role(self.role):
authorize_event_sharing = self._should_authorize_event_sharing(
install_method,
sql_executor._conn, # noqa: SLF001
package_model.deploy_root,
)
optional_shared_events = (
model.telemetry and model.telemetry.optional_shared_events
)

# 1. Need to use a warehouse to create an application object
with sql_executor.use_warehouse(self.warehouse):
Expand Down Expand Up @@ -470,6 +557,15 @@ def create_or_upgrade_app(
)
print_messages(console, upgrade_cursor)

if authorize_event_sharing is not None:
sql_executor.execute_query(
f"alter application {app_name} set AUTHORIZE_TELEMETRY_EVENT_SHARING = {str(authorize_event_sharing).upper()}"
)
Comment on lines +561 to +563
Copy link
Contributor

Choose a reason for hiding this comment

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

can we put these new queries in the facade? it'll make writing tests easier since you can just mock the function call

if optional_shared_events:
sql_executor.execute_query(
f"""alter application {app_name} set shared telemetry events ('{"', '".join(optional_shared_events)}')"""
)

if install_method.is_dev_mode:
# if debug_mode is present (controlled), ensure it is up-to-date
if debug_mode is not None:
Expand Down Expand Up @@ -517,18 +613,27 @@ def create_or_upgrade_app(
)
debug_mode_clause = f"debug_mode = {initial_debug_mode}"

authorize_telemetry_clause = ""
if authorize_event_sharing is not None:
authorize_telemetry_clause = f" AUTHORIZE_TELEMETRY_EVENT_SHARING = {str(authorize_event_sharing).upper()}"

using_clause = install_method.using_clause(stage_fqn)
create_cursor = sql_executor.execute_query(
dedent(
f"""\
create application {self.name}
from application package {package.name} {using_clause} {debug_mode_clause}
from application package {package.name} {using_clause} {debug_mode_clause}{authorize_telemetry_clause}
comment = {SPECIAL_COMMENT}
"""
),
)
print_messages(console, create_cursor)

if optional_shared_events:
sql_executor.execute_query(
f"""alter application {app_name} set shared telemetry events ('{"', '".join(optional_shared_events)}')"""
)

# hooks always executed after a create or upgrade
self.execute_post_deploy_hooks()

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,48 @@
from typing import List, Optional

from click import ClickException
from pydantic import Field, field_validator, model_validator
from snowflake.cli.api.project.schemas.updatable_model import UpdatableModel


class EventSharingTelemetry(UpdatableModel):
authorize_event_sharing: Optional[bool] = Field(
title="Whether to authorize Snowflake to share application usage data with application package provider. This automatically enables the sharing of required telemetry events.",
default=None,
)
optional_shared_events: Optional[List[str]] = Field(
Comment on lines +9 to +13
Copy link
Contributor

Choose a reason for hiding this comment

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

do these need to be optional to have a default? do we handle these fields differently if they're None vs False/[]?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

i am not sure if optional [] as default will have any side effect (I know it's not recommended in function params)

title="List of optional telemetry events that application owner would like to share with application package provider.",
default=None,
)

@model_validator(mode="after")
@classmethod
def validate_authorize_event_sharing(cls, value):
if value.optional_shared_events and not value.authorize_event_sharing:
raise ClickException(
"telemetry.authorize_event_sharing is required to be true in order to use telemetry.optional_shared_events."
)
return value

@field_validator("optional_shared_events")
@classmethod
def transform_artifacts(
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
def transform_artifacts(
def validate_optional_shared_events(

?

cls, original_shared_events: Optional[List[str]]
) -> Optional[List[str]]:
if original_shared_events is None:
return None

# make sure that each event is made of letters and underscores:
for event in original_shared_events:
if not event.isalpha() and not event.replace("_", "").isalpha():
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if not event.isalpha() and not event.replace("_", "").isalpha():
if not re.fullmatch(r"[a-zA-Z_]", event):

?

Copy link
Contributor

Choose a reason for hiding this comment

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

if you don't want the regex, then you only need

Suggested change
if not event.isalpha() and not event.replace("_", "").isalpha():
if not event.replace("_", "").isalpha():

raise ClickException(
f"Event {event} from optional_shared_events field is not a valid event name."
)

# make sure events are unique:
if len(original_shared_events) != len(set(original_shared_events)):
raise ClickException(
"Events in optional_shared_events field must be unique."
)

return original_shared_events
Loading
Loading