Codex/feat diagnostics and integrations - #14
Conversation
There was a problem hiding this comment.
Code Review
This pull request introduces a persistence layer for visitor state via a new DataStore protocol and an in-memory implementation, enabling the SDK to maintain visitor attributes, properties, and segments across context recreations. It also adds a privacy-safe diagnostic logging system, support for custom segment evaluation, and config entity lookup utilities. Feedback suggests refining the sensitive key redaction logic to avoid false positives caused by broad substring matching and optimizing variation lookups by indexing them within the ConfigSnapshot to improve performance.
| def _is_sensitive_key(key: str) -> bool: | ||
| if key in {"transport", "transport_config"}: | ||
| return True | ||
| return any(part in key for part in SENSITIVE_KEY_PARTS) |
There was a problem hiding this comment.
The sensitive key detection logic is overly broad because it uses substring matching for very short strings like "raw". This will lead to false positives and unnecessary redaction of non-sensitive keys that happen to contain these substrings (e.g., "strawberry", "drawing", "crawl"). Consider using exact matches for "raw" or checking for specific word boundaries like "raw_" or "_raw".
| def _lookup_variation_by_key( | ||
| snapshot: ConfigSnapshot, | ||
| key: str, | ||
| ) -> Mapping[str, Any] | None: | ||
| for experience in snapshot.experiences_by_id.values(): | ||
| variation = _lookup_variation(experience.get("variations"), "key", key) | ||
| if variation is not None: | ||
| return variation | ||
| return None | ||
|
|
||
|
|
||
| def _lookup_variation_by_id( | ||
| snapshot: ConfigSnapshot, | ||
| entity_id: str, | ||
| ) -> Mapping[str, Any] | None: | ||
| for experience in snapshot.experiences_by_id.values(): | ||
| variation = _lookup_variation(experience.get("variations"), "id", entity_id) | ||
| if variation is not None: | ||
| return variation | ||
| return None |
There was a problem hiding this comment.
The variation lookup implementation iterates through all experiences and their variations, resulting in ConfigSnapshot during initialization, allowing for
All eight open Dependabot alerts on this repo sit in `yarn.lock` — the dev-only semantic-release tooling. None of them reach the published wheel/sdist (httpx is still the only runtime dependency). Three of the four highs were already reachable inside the existing semver ranges; the fourth (sigstore GHSA-52v5-jr5w-gjxr, `certificateOIDs` verification constraints silently dropped) needed sigstore >= 4.1.1, which only arrives through a major bump: semantic-release 24.2.9 -> 25.0.9 @semantic-release/npm 12.0.2 -> 13.1.5 npm 10.9.8 -> 11.19.0 libnpmpublish 10 -> 11.2.0 }-> sigstore ^3 -> ^4 (4.1.1) pacote 19 -> 21.5.1 } @sigstore/core 2.0.0 -> 3.2.1 tar 7.5.16 -> 7.5.22 `@semantic-release/github` moves to ^12 to match what semantic-release 25 depends on — leaving it at ^11 would hoist the older copy to the project root and shadow the one core resolves. Resolved (4 high, 4 medium — no criticals were open): #14 high ip-address 10.2.0 -> 10.4.0 (needs >= 10.3.1) #9 high brace-expansion 2.1.1 -> 5.0.9 (needs >= 2.1.2) #7 high js-yaml 4.2.0 -> 4.3.1 (needs >= 4.3.0) #2 high sigstore 3.1.0 -> 4.1.1 (needs >= 4.1.1) #13 medium ip-address (same bump as #14) #12 medium ip-address (same bump as #14) #4 medium tar 7.5.16 -> 7.5.22 (needs >= 7.5.18) #1 medium @sigstore/core 2.0.0 -> 3.2.1 (needs >= 3.2.1) Node: semantic-release 25 requires ^22.14.0 || >= 24.10.0. release.yml installs `lts/*`, currently Node 24.19.0 — satisfied, and every future LTS line stays above the floor. Verified: `yarn install --immutable` (what release.yml runs) passes against the regenerated lockfile with the lockfile format unchanged (__metadata version 10), and `yarn npm audit --all --recursive` reports no suggestions. A `semantic-release --dry-run` against this branch loads all four configured plugins, passes verifyConditions for both exec and github (GitHub authentication + push permission), and analyzes commits to "no release" — correct, since `chore` is a non-releasing type in release.config.mjs. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Summary
Adds production-safe diagnostic logging for the Python SDK as part of Story 4.1.
This PR introduces an internal diagnostics layer using Python standard-library
logging, with structuredsdk_eventandsdk_detailsfields for support/debugging flows. Diagnostics now cover SDK initialization, config loading, context creation, experience/feature/custom-segment evaluation, conversion tracking, queueing, queue release, and tracking delivery failures.The implementation avoids logging raw sensitive values by redacting or omitting SDK keys, secrets, visitor IDs, visitor attributes, conversion data, raw config payloads, tracking payloads, headers, and transport objects.
Scope
convert_sdk.diagnosticsValidation
uv run pytest -p no:cacheprovider tests/test_diagnostics.py→ 3 passeduv run pytest -p no:cacheprovider→ 62 passeduv build→ source distribution and wheel built successfullyNotes
This branch is stacked on
codex/feat-stateful-contexts. If Epic 3 has been merged intodev-branch, this PR can be retargeted todev-branch.