Skip to content

Commit 128cc7c

Browse files
committed
refactor(telemetry): simplify auth/workspace instrumentation
- workspace.ts: inline single-use transition helpers, unify on the spread idiom for optional `observedDurationMs`, move `performance.now()` below the dedup early-return, drop the no-op `() => fn()` wrapper in `traceUpdateTriggered`, and make `INITIAL_STATE` private. - auth.ts: replace the `T extends LoginPromptResult` constraint with a `LoginPromptTracer` exposing `markAborted()`, matching the `RemoteSetupTracer` pattern. Callers now drive abort signaling, so telemetry no longer depends on `LoginResult`'s shape. - loginCoordinator: call `tracer.markAborted()` on `!result.success`.
1 parent c7d49c2 commit 128cc7c

3 files changed

Lines changed: 42 additions & 87 deletions

File tree

src/instrumentation/auth.ts

Lines changed: 6 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -10,8 +10,9 @@ export type AuthIntercept401Recovery =
1010
| "none";
1111
export type AuthLoginPromptTrigger = "auth_required" | "missing_session";
1212

13-
interface LoginPromptResult {
14-
readonly success: boolean;
13+
/** Helpers scoped to the auth.login_prompt trace's lifetime. */
14+
export interface LoginPromptTracer {
15+
markAborted(): void;
1516
}
1617

1718
export class AuthTelemetry {
@@ -30,19 +31,13 @@ export class AuthTelemetry {
3031
this.telemetry.log("auth.intercept_401", { recovery });
3132
}
3233

33-
public traceLoginPrompt<T extends LoginPromptResult>(
34+
public traceLoginPrompt<T>(
3435
trigger: AuthLoginPromptTrigger,
35-
fn: () => Promise<T>,
36+
fn: (tracer: LoginPromptTracer) => Promise<T>,
3637
): Promise<T> {
3738
return this.telemetry.trace(
3839
"auth.login_prompt",
39-
async (span) => {
40-
const result = await fn();
41-
if (!result.success) {
42-
span.markAborted();
43-
}
44-
return result;
45-
},
40+
(span) => fn({ markAborted: () => span.markAborted() }),
4641
{ trigger },
4742
);
4843
}

src/instrumentation/workspace.ts

Lines changed: 29 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -4,35 +4,14 @@ import {
44
} from "../telemetry/reporter";
55

66
import type {
7-
BuildReason,
87
Workspace,
98
WorkspaceAgent,
109
WorkspaceAgentLifecycle,
1110
WorkspaceAgentStatus,
1211
WorkspaceStatus,
13-
WorkspaceTransition,
1412
} from "coder/site/src/api/typesGenerated";
1513

16-
export const INITIAL_STATE = "unknown";
17-
18-
type InitialState = typeof INITIAL_STATE;
19-
20-
interface WorkspaceStateTransition {
21-
readonly from: WorkspaceStatus | InitialState;
22-
readonly to: WorkspaceStatus;
23-
readonly transition?: WorkspaceTransition;
24-
readonly reason?: BuildReason;
25-
readonly observedDurationMs?: number;
26-
}
27-
28-
interface WorkspaceAgentStateTransition {
29-
readonly agentName: string;
30-
readonly fromStatus: WorkspaceAgentStatus | InitialState;
31-
readonly toStatus: WorkspaceAgentStatus;
32-
readonly fromLifecycleState: WorkspaceAgentLifecycle | InitialState;
33-
readonly toLifecycleState: WorkspaceAgentLifecycle;
34-
readonly observedDurationMs?: number;
35-
}
14+
const INITIAL_STATE = "unknown";
3615

3716
interface ObservedWorkspaceState {
3817
readonly status: WorkspaceStatus;
@@ -55,42 +34,50 @@ export class WorkspaceTelemetry {
5534

5635
public observeWorkspace(workspace: Workspace): void {
5736
const status = workspace.latest_build.status;
58-
const now = performance.now();
5937
const previous = this.observedWorkspaceState;
6038
if (previous?.status === status) {
6139
return;
6240
}
41+
const now = performance.now();
6342

64-
this.workspaceStateTransition({
65-
from: previous?.status ?? INITIAL_STATE,
66-
to: status,
67-
transition: workspace.latest_build.transition,
68-
reason: workspace.latest_build.reason,
69-
...(previous && {
70-
observedDurationMs: now - previous.observedAtMs,
71-
}),
72-
});
43+
this.telemetry.log(
44+
"workspace.state_transitioned",
45+
{
46+
from: previous?.status ?? INITIAL_STATE,
47+
to: status,
48+
...(workspace.latest_build.transition && {
49+
transition: workspace.latest_build.transition,
50+
}),
51+
...(workspace.latest_build.reason && {
52+
reason: workspace.latest_build.reason,
53+
}),
54+
},
55+
previous ? { observedDurationMs: now - previous.observedAtMs } : {},
56+
);
7357
this.observedWorkspaceState = { status, observedAtMs: now };
7458
}
7559

7660
public observeAgent(agent: WorkspaceAgent): void {
77-
const now = performance.now();
7861
const previous = this.observedAgentState;
7962
if (
8063
previous?.status === agent.status &&
8164
previous.lifecycleState === agent.lifecycle_state
8265
) {
8366
return;
8467
}
68+
const now = performance.now();
8569

86-
this.agentStateTransition({
87-
agentName: agent.name,
88-
fromStatus: previous?.status ?? INITIAL_STATE,
89-
toStatus: agent.status,
90-
fromLifecycleState: previous?.lifecycleState ?? INITIAL_STATE,
91-
toLifecycleState: agent.lifecycle_state,
92-
...(previous && { observedDurationMs: now - previous.observedAtMs }),
93-
});
70+
this.telemetry.log(
71+
"workspace.agent.state_transitioned",
72+
{
73+
agentName: agent.name,
74+
fromStatus: previous?.status ?? INITIAL_STATE,
75+
toStatus: agent.status,
76+
fromLifecycleState: previous?.lifecycleState ?? INITIAL_STATE,
77+
toLifecycleState: agent.lifecycle_state,
78+
},
79+
previous ? { observedDurationMs: now - previous.observedAtMs } : {},
80+
);
9481
this.observedAgentState = {
9582
status: agent.status,
9683
lifecycleState: agent.lifecycle_state,
@@ -102,40 +89,7 @@ export class WorkspaceTelemetry {
10289
this.observedAgentState = undefined;
10390
}
10491

105-
private workspaceStateTransition(transition: WorkspaceStateTransition): void {
106-
this.telemetry.log(
107-
"workspace.state_transitioned",
108-
{
109-
from: transition.from,
110-
to: transition.to,
111-
...(transition.transition && { transition: transition.transition }),
112-
...(transition.reason && { reason: transition.reason }),
113-
},
114-
transition.observedDurationMs === undefined
115-
? {}
116-
: { observedDurationMs: transition.observedDurationMs },
117-
);
118-
}
119-
120-
private agentStateTransition(
121-
transition: WorkspaceAgentStateTransition,
122-
): void {
123-
this.telemetry.log(
124-
"workspace.agent.state_transitioned",
125-
{
126-
agentName: transition.agentName,
127-
fromStatus: transition.fromStatus,
128-
toStatus: transition.toStatus,
129-
fromLifecycleState: transition.fromLifecycleState,
130-
toLifecycleState: transition.toLifecycleState,
131-
},
132-
transition.observedDurationMs === undefined
133-
? {}
134-
: { observedDurationMs: transition.observedDurationMs },
135-
);
136-
}
137-
13892
public traceUpdateTriggered<T>(fn: () => Promise<T>): Promise<T> {
139-
return this.telemetry.trace("workspace.update.triggered", () => fn());
93+
return this.telemetry.trace("workspace.update.triggered", fn);
14094
}
14195
}

src/login/loginCoordinator.ts

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -99,7 +99,13 @@ export class LoginCoordinator implements vscode.Disposable {
9999
): Promise<LoginResult> {
100100
return this.authTelemetry.traceLoginPrompt(
101101
options.trigger ?? "auth_required",
102-
() => this.performLoginDialog(options),
102+
async (tracer) => {
103+
const result = await this.performLoginDialog(options);
104+
if (!result.success) {
105+
tracer.markAborted();
106+
}
107+
return result;
108+
},
103109
);
104110
}
105111

0 commit comments

Comments
 (0)