Skip to content

Commit 302e590

Browse files
authored
Refine posthog telemetry for execute sandbox script (#3471)
1 parent 1cd3d99 commit 302e590

4 files changed

Lines changed: 104 additions & 13 deletions

File tree

src/__tests__/posthogTelemetry.test.ts

Lines changed: 69 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ import { describe, expect, it } from "vitest";
22
import {
33
createExceptionFromTelemetry,
44
getExceptionTelemetryContext,
5+
shouldBypassNonProTelemetrySampling,
56
} from "@/lib/posthogTelemetry";
67

78
describe("createExceptionFromTelemetry", () => {
@@ -26,6 +27,74 @@ describe("createExceptionFromTelemetry", () => {
2627
});
2728
});
2829

30+
describe("shouldBypassNonProTelemetrySampling", () => {
31+
it("always sends sandbox.script.* events for non-Pro sampling", () => {
32+
expect(
33+
shouldBypassNonProTelemetrySampling({
34+
event: "sandbox.script.completed",
35+
properties: { chatId: 1, appId: 2 },
36+
}),
37+
).toBe(true);
38+
expect(
39+
shouldBypassNonProTelemetrySampling({
40+
event: "sandbox.script.truncated",
41+
properties: { chatId: 1 },
42+
}),
43+
).toBe(true);
44+
expect(
45+
shouldBypassNonProTelemetrySampling({
46+
event: "sandbox.script.failed",
47+
properties: { error: "Unexpected token" },
48+
}),
49+
).toBe(true);
50+
expect(
51+
shouldBypassNonProTelemetrySampling({
52+
event: "sandbox.script.timeout",
53+
properties: { error: "Script timed out" },
54+
}),
55+
).toBe(true);
56+
});
57+
58+
it("does not bypass unrelated sandbox telemetry", () => {
59+
expect(
60+
shouldBypassNonProTelemetrySampling({
61+
event: "sandbox.tool.unused_with_attachment",
62+
properties: { chatId: 1 },
63+
}),
64+
).toBe(false);
65+
});
66+
67+
it("still bypasses sampling for error-shaped events", () => {
68+
expect(
69+
shouldBypassNonProTelemetrySampling({
70+
event: "$exception",
71+
properties: { exception_message: "boom" },
72+
}),
73+
).toBe(true);
74+
expect(
75+
shouldBypassNonProTelemetrySampling({
76+
event: "extra-files:error",
77+
properties: {},
78+
}),
79+
).toBe(true);
80+
expect(
81+
shouldBypassNonProTelemetrySampling({
82+
event: "app:crash_detected",
83+
properties: { error: true },
84+
}),
85+
).toBe(true);
86+
});
87+
88+
it("allows routine events to be sampled", () => {
89+
expect(
90+
shouldBypassNonProTelemetrySampling({
91+
event: "chat:submit",
92+
properties: { chatMode: "build" },
93+
}),
94+
).toBe(false);
95+
});
96+
});
97+
2998
describe("getExceptionTelemetryContext", () => {
3099
it("removes exception payload fields before passing custom context to PostHog", () => {
31100
expect(

src/lib/posthogTelemetry.ts

Lines changed: 28 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,33 @@
11
type TelemetryProperties = Record<string, unknown> | undefined;
22

3+
/** PostHog event shape used by renderer `before_send` sampling. */
4+
export type PostHogTelemetryEvent = {
5+
event?: string;
6+
properties?: TelemetryProperties;
7+
};
8+
9+
/**
10+
* Non-Pro telemetry sends only ~10% of events. These events are always sent.
11+
* Keep `sandbox.script.*` here so script instrumentation is never sampled out.
12+
*/
13+
export function shouldBypassNonProTelemetrySampling(
14+
event: PostHogTelemetryEvent | null | undefined,
15+
): boolean {
16+
const eventName = event?.event;
17+
const properties = event?.properties;
18+
19+
if (eventName?.startsWith("sandbox.script.")) {
20+
return true;
21+
}
22+
23+
return (
24+
eventName === "$exception" ||
25+
eventName?.toLowerCase().includes("error") === true ||
26+
!!properties?.$exception_type ||
27+
!!properties?.error
28+
);
29+
}
30+
331
export function createExceptionFromTelemetry(properties: TelemetryProperties) {
432
const exception = new Error(
533
typeof properties?.exception_message === "string"

src/pro/main/ipc/handlers/local_agent/tools/execute_sandbox_script.ts

Lines changed: 0 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -168,11 +168,6 @@ Paths are app-relative (including \`.dyad/media/<stored-name>\`), or attachment
168168
args.description?.trim() || "Run a read-only script",
169169

170170
execute: async (args: ExecuteSandboxScriptArgs, ctx: AgentContext) => {
171-
sendTelemetryEvent("sandbox.script.run", {
172-
chatId: ctx.chatId,
173-
appId: ctx.appId,
174-
});
175-
176171
try {
177172
const result = await runSandboxScript({
178173
appPath: ctx.appPath,

src/renderer.tsx

Lines changed: 7 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ import { queryKeys } from "./lib/queryKeys";
3232
import {
3333
createExceptionFromTelemetry,
3434
getExceptionTelemetryContext,
35+
shouldBypassNonProTelemetrySampling,
3536
} from "./lib/posthogTelemetry";
3637

3738
// @ts-ignore
@@ -97,15 +98,13 @@ const posthogClient = posthog.init(
9798
event.properties["$ip"] = null;
9899
}
99100

100-
// For non-Pro users, only send 10% of events (but always send errors)
101+
// For non-Pro users, only send 10% of events (but always send errors and
102+
// sandbox.script.* instrumentation — see shouldBypassNonProTelemetrySampling).
101103
if (!isDyadProUser()) {
102-
const isErrorEvent =
103-
event?.event === "$exception" ||
104-
event?.event?.toLowerCase().includes("error") ||
105-
event?.properties?.$exception_type ||
106-
event?.properties?.error;
107-
108-
if (!isErrorEvent && Math.random() > 0.1) {
104+
if (
105+
!shouldBypassNonProTelemetrySampling(event) &&
106+
Math.random() > 0.1
107+
) {
109108
console.debug("Non-Pro user: sampling out event", event?.event);
110109
return null;
111110
}

0 commit comments

Comments
 (0)