CLI: Add telemetry - #14304
CLI: Add telemetry#14304
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
OpenAPI ChangesOperations
Schemas
|
Generated OpenAPI Client
The OpenAPI schema has changed but the generated client has not been regenerated. Please run: cd clients && pnpm generateand commit the updated Diff stat |
frankie567
left a comment
There was a problem hiding this comment.
Do we really need to proxy this via our server? Couldn't we directly hit Posthog? Their client has also lot of goodness like batching and such: https://posthog.com/docs/libraries/node#installation
|
@frankie567 yes sorry I was supposed to set this PR as a draft, that is the plan! |
|
Awesome 🙂 |
There was a problem hiding this comment.
1 issue found across 7 files
Confidence score: 3/5
- In
server/polar/cli/schemas.py, unauthenticated telemetry accepts up to 25 arbitrarily large flag strings and forwards them to PostHog, creating an avoidable resource or payload-abuse risk; cap each flag string’s length before forwarding.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="server/polar/cli/schemas.py">
<violation number="1" location="server/polar/cli/schemas.py:13">
P2: An unauthenticated telemetry request can include 25 arbitrarily large flag strings because `max_length=25` limits only the item count, then forwards them to PostHog. Bound each flag string before forwarding.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| description="Random identifier generated on first run of the CLI." | ||
| ) | ||
| command: str = Field(max_length=100, description="Command path, e.g. `auth login`.") | ||
| flags: list[str] = Field( |
There was a problem hiding this comment.
P2: An unauthenticated telemetry request can include 25 arbitrarily large flag strings because max_length=25 limits only the item count, then forwards them to PostHog. Bound each flag string before forwarding.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At server/polar/cli/schemas.py, line 13:
<comment>An unauthenticated telemetry request can include 25 arbitrarily large flag strings because `max_length=25` limits only the item count, then forwards them to PostHog. Bound each flag string before forwarding.</comment>
<file context>
@@ -0,0 +1,26 @@
+ description="Random identifier generated on first run of the CLI."
+ )
+ command: str = Field(max_length=100, description="Command path, e.g. `auth login`.")
+ flags: list[str] = Field(
+ default_factory=list, max_length=25, description="Flag names, never values."
+ )
</file context>
f622013 to
01c9748
Compare
01c9748 to
d50a346
Compare
d50a346 to
e12b149
Compare
e12b149 to
52f2e08
Compare
|
@cubic-dev-ai review |
|
@cubic-dev-ai re review |
@sebastianekstrom I have started the AI code review. It will take a few minutes to complete. |
There was a problem hiding this comment.
1 issue found across 4 files
Confidence score: 4/5
clients/packages/cli/src/services/telemetry.ts: An unreachable or slow PostHog service can add up to 1.5 seconds to every release command viaEffect.onExit, creating a noticeable CLI slowdown; move telemetry fire-and-forget outside the command exit path or otherwise avoid blocking shutdown.
Prompt for AI agents (unresolved issues)
Check if these issues are valid — if so, understand the root cause of each and fix them. If appropriate, use sub-agents to investigate and fix each issue separately.
<file name="clients/packages/cli/src/services/telemetry.ts">
<violation number="1" location="clients/packages/cli/src/services/telemetry.ts:234">
P2: When PostHog is unreachable or slow, every release command waits up to 1.5 seconds for telemetry during `Effect.onExit` before the process exits. Run telemetry fire-and-forget outside the command's exit path, or otherwise avoid making command completion depend on the network timeout.</violation>
</file>
Reply with feedback, questions, or to request a fix.
Re-trigger cubic
| yield* client.execute(request) | ||
| }).pipe( | ||
| Effect.scoped, | ||
| Effect.timeout(SEND_TIMEOUT), |
There was a problem hiding this comment.
P2: When PostHog is unreachable or slow, every release command waits up to 1.5 seconds for telemetry during Effect.onExit before the process exits. Run telemetry fire-and-forget outside the command's exit path, or otherwise avoid making command completion depend on the network timeout.
Prompt for AI agents
Check if this issue is valid — if so, understand the root cause and fix it. At clients/packages/cli/src/services/telemetry.ts, line 234:
<comment>When PostHog is unreachable or slow, every release command waits up to 1.5 seconds for telemetry during `Effect.onExit` before the process exits. Run telemetry fire-and-forget outside the command's exit path, or otherwise avoid making command completion depend on the network timeout.</comment>
<file context>
@@ -0,0 +1,237 @@
+ yield* client.execute(request)
+ }).pipe(
+ Effect.scoped,
+ Effect.timeout(SEND_TIMEOUT),
+ Effect.provide(http),
+ Effect.ignore,
</file context>
There was a problem hiding this comment.
@sebastianekstrom This one might makes sense from a UX perspective. It would be pretty weird if I tried to exit a command and it waited 1.5 seconds. I know it's not going to be very rare though, let me know what you think.
There was a problem hiding this comment.
I checked and stripe is doing the same so maybe I overreacted
There was a problem hiding this comment.
Agreed though, and fixed!
| ['gemini-cli', /^GEMINI_CLI$/], | ||
| ['copilot', /^(GITHUB_)?COPILOT_/], | ||
| ['aider', /^AIDER_/], | ||
| ['windsurf', /^WINDSURF_/], |
There was a problem hiding this comment.
Does windsurf even exist? 😅
I'd add opencode and pi to this list, I'm pretty sure they have more users than windsurf aider, probably even gemini-cli and copilot
| env = process.env, | ||
| home = homedir(), | ||
| http = FetchHttpClient.layer, |
There was a problem hiding this comment.
This might sound nitpick-y but this is not very idiomatic effect to have these as parameters and the agent probably just did it for testing purposes. Ideally we would have a telemetry service with a record method and the implementation of that service would depend on the FileSystem service and Http service from effect, then in tests a mock implementation of those services can be provided.
There was a problem hiding this comment.
Good point, fixed!
52f2e08 to
277f201
Compare
277f201 to
effed8a
Compare
Summary
Add (anonymous) telemetry so we can analyze the usage of the CLI. For each command we now pass:
to PostHog. These can be opted out of with either the
POLAR_CLI_TELEMETRY_OPTOUTorDO_NOT_TRACKenvironment variable.Fixes https://linear.app/polarsh/issue/PLR-109/add-basic-telemetry