SNOW-1041753: oob telemetry for snowpark python local testing #1265
Conversation
sfc-gh-aling
changed the title
sfc-gh-aling
requested review from
sfc-gh-jdu,
sfc-gh-yuwang and
sfc-gh-jrose
sfc-gh-aling
force-pushed
the
SNOW-1041753-feature-oob-telemetry-code-change
branch
from
13a67ad to
c6f52e3
Compare
|
|
||
|
|
||
| class LocalTestOOBTelemetryService(TelemetryService): | ||
| PROD = "https://client-telemetry.c1.us-west-2.aws.app.snowflake.com/enqueue" |
There was a problem hiding this comment.
Does the client have to auth to access this endpoint?
There was a problem hiding this comment.
no, this endpoint is publicly available.
this is out-of-band service so anyone can access this endpoint, but the telemetry data has to comply with the format otherwise the service will ignore the request
| logger.debug("Telemetry request success=%s", success) | ||
|
|
||
| def add(self, event) -> None: | ||
| """Adds a telemetry event to the queue. If the event is urgent, upload all telemetry events right away.""" |
There was a problem hiding this comment.
I don't see anything that would do an immediate upload.
There was a problem hiding this comment.
sorry about the comment being confusing -- I copied it from the connector telemetry code.
I removed the comment.
I don't think the concept "urgent" is applicable to local test, doing batch sending should be enough for local testing telemetry events. in the connector, urgent is for ocsp validation error.
sfc-gh-aling
force-pushed
the
SNOW-1041753-feature-oob-telemetry-code-change
branch
from
c6f52e3 to
6366cb7
Compare
| @@ -229,6 +236,47 @@ def __init__(self, options: Optional[Dict[str, Any]] = None) -> None: | |||
| self._active_schema = self._options.get( | |||
| "schema", snowflake.snowpark.mock._constants.CURRENT_SCHEMA | |||
| ) | |||
| self._connection_uuid = str(uuid.uuid4()) | |||
There was a problem hiding this comment.
Shall we create a TODO for to use the .snowsql approach so we can track telemetry per installation instead?
There was a problem hiding this comment.
yes, the JIRA is created here: https://snowflakecomputing.atlassian.net/browse/SNOW-1179161 and tracked in post GA epic
Please answer these questions before submitting your pull requests. Thanks!
This PR implement a OOB Telemetry client for Snowpark Python Local Testing only which overrides the OOBTelemetry in the snowflake-connector-python, it implements the following features:
please be aware that this PR only focuses telemetry for local testing, it doesn't take other oob telemetry into consideration or any refactor work.
one improvement identified is to consolidate the existing oob telemetry in snowflake, but because this is a new telemetry endpoint, we need to test the existing oob logic with the new end point, this is beyond our current goal and requires extra amount of work.
What GitHub issue is this PR addressing? Make sure that there is an accompanying issue to your PR.
Fixes #NNNN
Fill out the following pre-review checklist:
Please describe how your code solves the related issue.
Please write a short description of how your code change solves the related issue.