Skip to content

fix(core): don't log full event payloads or deep-link tokens at info level - #1326

Open
sunitaprajapati89 wants to merge 2 commits into
masterfrom
optimise-logs
Open

fix(core): don't log full event payloads or deep-link tokens at info level#1326
sunitaprajapati89 wants to merge 2 commits into
masterfrom
optimise-logs

Conversation

@sunitaprajapati89

Copy link
Copy Markdown
Contributor

Track/screen/identify/group/alias/deep-link event logging previously
passed the entire SegmentEvent (userId, traits, properties, and the raw
deep-link URL) to logger.info. The default Logger only disables itself
when NODE_ENV === 'production', which many RN release bundles don't set
for JS, and any custom logger or logger.enable() call re-exposed this
data to logcat/device logs and log-capture SDKs.

  • Log only non-sensitive metadata (type, messageId, name) at info level
    by default.
  • Add an explicit debugPayloads config flag to opt into full-payload
    logging, gated behind a startup warning about the PII/token exposure.
  • Redact the deep-link URL's query string before it is ever logged,
    even when debugPayloads is enabled.
  • Log the processed event (with its real messageId) instead of the
    pre-processed one, so the metadata log is actually useful for
    correlation.

Comment thread packages/core/src/analytics.ts Outdated
Comment thread packages/core/src/analytics.ts Outdated
Comment thread packages/core/src/analytics.ts Outdated
Comment thread packages/core/src/types.ts Outdated

@didiergarcia didiergarcia left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

A few changes to capture edge cases

didiergarcia flagged four issues on #1326:

- redactDeepLinkUrl only stripped the query string; OAuth callback
  tokens in the URL fragment (myapp://callback#access_token=...)
  survived redaction. Split on /[?#]/ instead of '?'.
- The deep-link track call logged the pre-process event because
  `void this.process(event)` discarded its result, so messageId was
  always undefined. Await the result via .then() (keeping the call
  site fire-and-forget) and log the processed event.
- `processedEvent ?? event` collapsed "saved", "disabled", and
  "dropped by a before/consent plugin" into one misleading "event
  saved" log. Added logEventResult() to log "<label> event dropped"
  when processing yields undefined, reused across screen/track/
  identify/group/alias and the deep-link path.
- Fixed the debugPayloads JSDoc: it logs the full payload *in
  addition to* metadata, not instead of it.

Added test coverage for fragment redaction and the dropped-event log
path.

Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
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.

3 participants