Add anonymous usage telemetry to Email SDK and CLI#80
Conversation
…otice Co-authored-by: null <>
|
|
Requesting review from @leoisadev1 who has experience with the following files modified in this PR:
|
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
Greptile SummaryThis PR adds anonymous usage telemetry to the Email SDK and CLI via a custom PostHog HTTP integration, capturing
Confidence Score: 5/5Safe to merge — telemetry is purely additive, all opt-out paths work, and the CLI refactor correctly flushes events before process.exit. The core email sending path is unchanged and all telemetry calls are fire-and-forget or properly awaited before exit. No blocking or correctness issues were found in the changed code paths. packages/email-sdk/src/telemetry.ts (notice persistence in read-only environments) and packages/email-sdk/src/core.ts (sendBatch N-event fan-out). Important Files Changed
Sequence DiagramsequenceDiagram
participant CLI as cli.ts (top-level await)
participant Main as main()
participant Core as createEmailClient / send()
participant Tel as telemetry.ts (singleton)
participant PH as PostHog /capture/
CLI->>Main: main(command, flags)
Main->>Core: createEmailClient(options)
Core->>Tel: getTelemetry()
Tel-->>Core: Telemetry instance
Core->>Tel: void capture("client created")
Tel--)PH: POST /capture/ (fire-and-forget)
Main->>Core: client.send(message)
Core->>Tel: void capture("email sent")
Tel--)PH: POST /capture/ (fire-and-forget)
Core-->>Main: response
Main-->>CLI: resolved
CLI->>Tel: captureCliRun → capture("cli command run")
Tel->>PH: POST /capture/ (awaited)
PH-->>Tel: 200 OK
CLI->>Tel: flush() → Promise.all(pending)
Tel->>PH: await remaining in-flight requests
PH-->>Tel: settled
Tel-->>CLI: flushed
alt error path
CLI->>Tel: captureCliRun (success: false)
Tel->>PH: POST /capture/
CLI->>Tel: flush()
CLI->>CLI: process.exit(1)
end
Reviews (2): Last reviewed commit: "fix(telemetry): address review feedback ..." | Re-trigger Greptile |
| const messageFacts = { | ||
| recipients: arrayify(message.to).length, | ||
| has_attachments: (message.attachments?.length ?? 0) > 0, |
There was a problem hiding this comment.
has_attachments collected but not disclosed in README
has_attachments is sent with every email sent event but is not mentioned anywhere in either README under "What is collected". Both READMEs list "recipient counts, SDK version, OS, and Node.js version" but omit this field entirely. Users who read the documentation to understand what is tracked will not see this field. Given this is opt-in-by-default telemetry, every collected property should be explicitly listed. Score: 4/5
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
| const doNotTrack = env.DO_NOT_TRACK?.toLowerCase(); | ||
|
|
||
| if (doNotTrack === "1" || doNotTrack === "true") { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
DO_NOT_TRACK only honors "1" and "true", misses the plain "1" standard
The existing behaviour is reasonable, but a comment explaining the deliberate choice would prevent future confusion. Score: 2/5
| const doNotTrack = env.DO_NOT_TRACK?.toLowerCase(); | |
| if (doNotTrack === "1" || doNotTrack === "true") { | |
| return true; | |
| } | |
| // Honour the standard DNT value "1" as well as the common "true" alias. | |
| // Any other value (including "0") is treated as not opting out. | |
| const doNotTrack = env.DO_NOT_TRACK?.toLowerCase(); | |
| if (doNotTrack === "1" || doNotTrack === "true") { | |
| return true; | |
| } |
Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!
…CLI parsing - count cc and bcc recipients in the email sent event and disclose has_attachments plus the full recipient definition in both READMEs - track in-flight captures and add Telemetry.flush() so the CLI settles fire-and-forget events from core.ts before process.exit(1) - add resetTelemetry() to clear the cached shared instance - document the deliberate DO_NOT_TRACK value handling - parse process.argv once in the CLI entrypoint and pass command/flags to main() and captureCliRun() Co-authored-by: null <>
addressed all 6 greptile review issues on pr #80 in commit 44008da:
validated with bun test (89 pass, including a new flush test), check-types, oxlint/oxfmt, and a cli smoke test of the help, dry-run send, and error paths. pr: #80 |
|
these inline review comments are from greptile's review of the older commit (7bbf9b9) — all six were already fixed in commit 44008da, which is the current head of the pr branch. i verified each one is in place there:
re-verified on the branch head: bun test passes (89 tests) and check-types is clean. nothing new to push — re-triggering greptile on the latest commit should resolve these. pr: #80 |
Summary
Add anonymous usage telemetry to the Email SDK and CLI using PostHog. Captures events:
client created,email sent, andcli command runwith adapter names, command names, success/failure, error codes, duration, and recipient counts. No sensitive data (email content, addresses, headers, credentials) is collected. Includes opt-out via environment variables (EMAIL_SDK_TELEMETRY=0,DO_NOT_TRACK=1) or client config (telemetry: false). Telemetry is disabled automatically in test environments. Adds a one-time opt-out notice on first use.