Skip to content

SNOW-2912540: decouple LocalTestOOBTelemetryService from connector.telemetry_oob - #4309

Open
sfc-gh-fpawlowski wants to merge 4 commits into
SNOW-2912540-inline-pandas-staging-helpersfrom
SNOW-2912540-decouple-oob-telemetry
Open

SNOW-2912540: decouple LocalTestOOBTelemetryService from connector.telemetry_oob#4309
sfc-gh-fpawlowski wants to merge 4 commits into
SNOW-2912540-inline-pandas-staging-helpersfrom
SNOW-2912540-decouple-oob-telemetry

Conversation

@sfc-gh-fpawlowski

@sfc-gh-fpawlowski sfc-gh-fpawlowski commented Aug 6, 2026

Copy link
Copy Markdown

The UD's telemetry_oob.TelemetryService is a no-op stub (BD#45) that is missing batch_size, which LocalTestOOBTelemetryService.add() uses — causing an AttributeError at runtime.

Makes LocalTestOOBTelemetryService fully standalone by:

  • Removing from snowflake.connector.telemetry_oob import TelemetryService and the inheritance
  • Adding its own double-checked-lock singleton (get_instance())
  • Initialising self.queue, self.batch_size, and self._enabled directly in __init__
  • Adding close() (delegates to flush(), called by atexit)
  • Fixing the pre-existing self._enable typo (should have been _enabled)

SecretDetector (also from the connector) is now decoupled too — moved into mock/_secret_detector.py as a literal copy of the Universal Driver's _common/secret_detector.py (the shim added in drivers#598 purely to satisfy this one Snowpark import; SecretDetector was never a real UD capability). Only the backward-compatibility-specific mechanism was removed (@backward_compatibility, install_backward_compatibility_getattr), since that has no equivalent in Snowpark — everything else (the class, MaskedMessageData, the 3-tuple return, logging.Formatter integration, masked_text = str(ex) on failure) is unchanged from the UD source, including a known real finding already flagged by the security bot on drivers#598 that isn't fixed as part of this move.

Note that OOB telemetry is disabled on the backend anyway, so this masking is defense-in-depth on the outbound payload, not a response to active exploitation — but export_queue_to_string() still feeds a real requests.Session().post() to a live analytics endpoint, so it's not a safe no-op either. If full secret-masking becomes a real requirement for the Universal Driver itself later, the right fix is a DependencyRequest to implement it once in sf_core, not to re-add a per-language regex port.

After this, mock/_telemetry.py has zero remaining imports from snowflake.connector.

Checklist

  • I acknowledge that I have ensured my changes to be thread-safe

Stack (via Graphite)

🤖 Generated with Claude Code

sfc-gh-fpawlowski commented Aug 6, 2026

Copy link
Copy Markdown
Author

@codecov-commenter

codecov-commenter commented Aug 17, 2026

Copy link
Copy Markdown

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.23%. Comparing base (eaa97e3) to head (4d5e471).

Additional details and impacted files
@@                              Coverage Diff                               @@
##           SNOW-2912540-inline-pandas-staging-helpers    #4309      +/-   ##
==============================================================================
+ Coverage                                       94.95%   95.23%   +0.28%     
==============================================================================
  Files                                             171      171              
  Lines                                           44758    44758              
  Branches                                         7678     7678              
==============================================================================
+ Hits                                            42501    42627     +126     
+ Misses                                           1438     1344      -94     
+ Partials                                          819      787      -32     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@sfc-gh-fpawlowski
sfc-gh-fpawlowski changed the base branch from SNOW-2912540-inline-pandas-staging-helpers to graphite-base/4309 August 20, 2026 16:27
@sfc-gh-fpawlowski
sfc-gh-fpawlowski force-pushed the SNOW-2912540-decouple-oob-telemetry branch from c202a6b to 069c7ff Compare August 20, 2026 16:28
@sfc-gh-fpawlowski
sfc-gh-fpawlowski changed the base branch from graphite-base/4309 to SNOW-2912540-inline-pandas-staging-helpers August 20, 2026 16:28
@sfc-gh-fpawlowski
sfc-gh-fpawlowski changed the base branch from SNOW-2912540-inline-pandas-staging-helpers to graphite-base/4309 August 21, 2026 08:24
sfc-gh-fpawlowski and others added 2 commits August 21, 2026 08:29
…lemetry_oob

The UD's telemetry_oob.TelemetryService is a no-op stub (BD#45) that lacks
batch_size, causing an AttributeError at runtime. Make LocalTestOOBTelemetryService
standalone: add its own singleton, queue, batch_size, _enabled, and close().
Also fixes the pre-existing _enable typo (should have been _enabled).

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
The decoupling from connector.telemetry_oob ported add()/flush()/enable()/
disable() but missed size() (connector/telemetry_oob.py:542,
`return self.queue.qsize()`), which several tests
(tests/mock/test_oob_telemetry.py, tests/mock/test_multithreading.py) call
directly on the service instance.
@sfc-gh-fpawlowski
sfc-gh-fpawlowski force-pushed the SNOW-2912540-decouple-oob-telemetry branch from 4d5e471 to c921b91 Compare August 21, 2026 08:30
@sfc-gh-fpawlowski
sfc-gh-fpawlowski changed the base branch from graphite-base/4309 to SNOW-2912540-inline-pandas-staging-helpers August 21, 2026 08:30
mock/_telemetry.py imported snowflake.connector.secret_detector.SecretDetector
unconditionally -- the only reason UD PR #598 ports SecretDetector into the
Universal Driver at all is to satisfy this one Snowpark import (SecretDetector
is not otherwise used or maintained by UD). Per BehaviorDifferences.yaml #45,
OOB telemetry is disabled on the backend anyway, so this masking is
defense-in-depth on the outbound payload, not a response to active
exploitation -- but export_queue_to_string() feeds a real
requests.Session().post() to a live analytics endpoint, so it's not a safe
no-op either.

Ports only what _telemetry.py actually calls (mask_secrets -> masked string),
from UD's actual _common/secret_detector.py source (PR #598) rather than the
older, less complete real v4 connector's version -- verified UD's copy has
3 additional maskers (OAuth tokens, OAuth client secrets, passcodes) plus a
PASSWORD_PATTERN false-positive fix and a wire-format fix to
CONNECTION_TOKEN_PATTERN, all needed for correctness. Drops everything that
exists solely for UD's backward-compat contract: @backward_compatibility,
MaskedMessageData's 3-tuple return (only masked_text is ever read),
logging.Formatter inheritance, and the classmethod-vs-staticmethod workaround.

Fixes the exception-handling branch rather than porting it straight: the
legacy/UD code does masked_text = str(ex) on failure, which can leak
exception-reflected input. Returns a static sentinel instead -- this was
already flagged as a real finding by the security bot on UD #598.

After this, mock/_telemetry.py has zero remaining imports from
snowflake.connector.

Test coverage: ported every behavioral test from UD PR #598's
test_secret_detector.py TestMaskSecrets class (27 cases via pytest count),
adapted from the 3-tuple/class-method API to the plain-function API here,
plus one exception-handling test adapted to assert the static sentinel
instead of the leaked exception text. Dropped TestFormatter and the
logging.Formatter-specific exception tests -- not applicable, since
mask_secrets is a plain function here, not a logging.Formatter subclass.
Corrects the previous commit: this is now an exact copy of UD's
_common/secret_detector.py (drivers#598), not a collapsed rewrite. Keeps the
SecretDetector class, MaskedMessageData, the (is_masked, masked_text, err_str)
3-tuple return, classmethods, logging.Formatter inheritance, format(),
create_formatting_error_log(), and masked_text = str(ex) on failure --
unchanged, even though the str(ex) behavior is a known real finding (flagged
by the security bot on UD #598); fixing it is a separate call for whoever
reviews this, not bundled into a move.

Only removed what's physically impossible to keep: @backward_compatibility,
install_backward_compatibility_getattr and its import, and the trailing
install_backward_compatibility_getattr(__name__) call -- none of that
mechanism exists in snowpark-python. The mask_secrets() docstring explaining
why the maskers are classmethods (to survive @backward_compatibility
stashing the class out of module globals) is left as-is even though that
rationale no longer applies here -- not silently deleted or rewritten as
part of this move.

_telemetry.py's call site goes back to unpacking the 3-tuple unmodified;
test_secret_detector.py is now a full copy of UD's test suite (TestMaskSecrets,
TestMaskSecretsExceptionHandling, TestFormatter), not just the masking-behavior
subset -- format()/logging.Formatter are kept, so their tests are too.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants