Skip to content

feat: add auth telemetry lifecycle traces #994

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
EhabY merged 6 commits into from
Jun 10, 2026
Merged

feat: add auth telemetry lifecycle traces #994

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
51c068e
feat: add auth telemetry lifecycle traces
EhabY Jun 8, 2026
20ab3f7
refactor: isolate auth telemetry wrappers
EhabY Jun 8, 2026
23f7ba4
refactor: align auth telemetry conventions
EhabY Jun 9, 2026
408abd6
refactor: simplify auth telemetry flow
EhabY Jun 9, 2026
23a5367
refactor: simplify auth telemetry per review
EhabY Jun 9, 2026
9f32eec
refactor: apply telemetry review conventions
EhabY Jun 10, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
67 changes: 47 additions & 20 deletions src/commands.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,11 @@ import * as cliExec from "./core/cliExec";
import { CertificateError } from "./error/certificateError";
import { toError } from "./error/errorUtils";
import { type FeatureSet, featureSetForVersion } from "./featureSet";
import {
AuthTelemetry,
type AuthLoginOutcome,
type AuthLogoutOutcome,
} from "./instrumentation/auth";
import {
reportElapsedProgress,
withCancellableProgress,
Expand Down Expand Up @@ -49,7 +54,7 @@ import type { PathResolver } from "./core/pathResolver";
import type { SecretsManager } from "./core/secretsManager";
import type { DeploymentManager } from "./deployment/deploymentManager";
import type { Logger } from "./logging/logger";
import type { LoginCoordinator } from "./login/loginCoordinator";
import type { LoginCoordinator, LoginMethod } from "./login/loginCoordinator";
import type { TelemetryService } from "./telemetry/service";
import type { SpeedtestPanelFactory } from "./webviews/speedtest/speedtestPanelFactory";
import type {
Expand All @@ -68,6 +73,11 @@ interface OpenOptions {
useDefaultDirectory?: boolean;
}

interface LoginArgs {
readonly url?: string;
readonly autoLogin?: boolean;
}

const openDefaults = {
openRecent: false,
useDefaultDirectory: true,
Expand All @@ -83,6 +93,7 @@ export class Commands {
private readonly duplicateWorkspaceIpc: DuplicateWorkspaceIpc;
private readonly speedtestPanelFactory: SpeedtestPanelFactory;
private readonly telemetryService: TelemetryService;
private readonly authTelemetry: AuthTelemetry;

// These will only be populated when actively connected to a workspace and are
// used in commands. Because commands can be executed by the user, it is not
Expand All @@ -109,6 +120,7 @@ export class Commands {
this.loginCoordinator = serviceContainer.getLoginCoordinator();
this.duplicateWorkspaceIpc = serviceContainer.getDuplicateWorkspaceIpc();
this.speedtestPanelFactory = serviceContainer.getSpeedtestPanelFactory();
this.authTelemetry = new AuthTelemetry(this.telemetryService);
}

/**
Expand Down Expand Up @@ -137,20 +149,20 @@ export class Commands {
* Log into a deployment. If already authenticated, this is a no-op.
* If no URL is provided, shows a menu of recent URLs plus defaults.
*/
public async login(args?: {
url?: string;
autoLogin?: boolean;
}): Promise<void> {
public async login(args?: LoginArgs): Promise<void> {
if (this.deploymentManager.isAuthenticated()) {
return;
}
await this.performLogin(args);
await this.authTelemetry.traceLogin(
args?.autoLogin ? "auto_login" : "command",
(trace) => this.performLogin(args, trace.setMethod),
);
}

private async performLogin(args?: {
url?: string;
autoLogin?: boolean;
}): Promise<void> {
private async performLogin(
args: LoginArgs | undefined,
setMethod: (method: LoginMethod) => void,
): Promise<AuthLoginOutcome> {
this.logger.debug("Logging in");

const currentDeployment = await this.secretsManager.getCurrentDeployment();
Expand All @@ -160,7 +172,7 @@ export class Commands {
currentDeployment?.url,
);
if (!url) {
return; // The user aborted.
return { success: false, reason: "no_url_provided" };
}

const safeHostname = toSafeHost(url);
Expand All @@ -173,19 +185,26 @@ export class Commands {
});

if (!result.success) {
return;
return result;
}

// Record the method eagerly so it is captured even if persistence throws.
setMethod(result.method);
await this.deploymentManager.setDeployment({
url,
safeHostname,
token: result.token,
user: result.user,
});
this.showWelcomeMessage(result.user.username);
this.logger.debug("Login complete to deployment:", url);
return { success: true, method: result.method };
}

private showWelcomeMessage(username: string): void {
vscode.window
.showInformationMessage(
`Welcome to Coder, ${result.user.username}!`,
`Welcome to Coder, ${username}!`,
{
detail:
"You can now use the Coder extension to manage your Coder instance.",
Expand All @@ -197,7 +216,6 @@ export class Commands {
vscode.commands.executeCommand("coder.open");
}
});
this.logger.debug("Login complete to deployment:", url);
}

/**
Expand Down Expand Up @@ -418,21 +436,30 @@ export class Commands {
* Log out and clear stored credentials, requiring re-authentication on next login.
*/
public async logout(): Promise<void> {
await this.authTelemetry.traceLogout(() => this.performLogout());
}

private async performLogout(): Promise<AuthLogoutOutcome> {
if (!this.deploymentManager.isAuthenticated()) {
return;
return { success: false, reason: "not_authenticated" };
}

this.logger.debug("Logging out");

const deployment = this.deploymentManager.getCurrentDeployment();

await this.deploymentManager.clearDeployment();
await this.deploymentManager.clearDeployment("logout");

if (deployment) {
await this.cliManager.clearCredentials(deployment.url);
await this.secretsManager.clearAllAuthData(deployment.safeHostname);
}

this.showLogoutMessage();
this.logger.debug("Logout complete");
return { success: true };
}

private showLogoutMessage(): void {
vscode.window
.showInformationMessage("You've been logged out of Coder!", "Login")
.then((action) => {
Expand All @@ -442,8 +469,6 @@ export class Commands {
});
}
});

this.logger.debug("Logout complete");
}

/**
Expand All @@ -452,7 +477,9 @@ export class Commands {
*/
public async switchDeployment(): Promise<void> {
this.logger.debug("Switching deployment");
await this.performLogin();
await this.authTelemetry.traceLogin("switch_deployment", (trace) =>
this.performLogin(undefined, trace.setMethod),
);
}

/**
Expand Down
97 changes: 70 additions & 27 deletions src/core/cliCredentialManager.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,8 +7,14 @@ import * as semver from "semver";

import { isAbortError } from "../error/errorUtils";
import { featureSetForVersion } from "../featureSet";
import {
CredentialCliError,
CredentialFileError,
CredentialTelemetry,
} from "../instrumentation/credentials";
import { isKeyringEnabled } from "../settings/cli";
import { getHeaderArgs } from "../settings/headers";
import { type TelemetryReporter } from "../telemetry/reporter";
import { toSafeHost } from "../util";
import { writeAtomically } from "../util/fs";

Expand All @@ -17,6 +23,7 @@ import { version } from "./cliExec";
import type { WorkspaceConfiguration } from "vscode";

import type { Logger } from "../logging/logger";
import type { Span } from "../telemetry/span";

import type { PathResolver } from "./pathResolver";

Expand Down Expand Up @@ -46,11 +53,16 @@ export function isKeyringSupported(): boolean {
* persistence: keyring-backed (via CLI) and file-based (plaintext).
*/
export class CliCredentialManager {
private readonly credentialTelemetry: CredentialTelemetry;

constructor(
private readonly logger: Logger,
private readonly resolveBinary: BinaryResolver,
private readonly pathResolver: PathResolver,
) {}
telemetry: TelemetryReporter,
) {
this.credentialTelemetry = new CredentialTelemetry(telemetry);
}

/**
* Store credentials for a deployment URL. Uses the OS keyring when the
Expand All @@ -59,22 +71,35 @@ export class CliCredentialManager {
*
* Keyring and files are mutually exclusive, never both.
*/
public async storeToken(
public storeToken(
Comment on lines -62 to +74

@DanielleMaywood DanielleMaywood Jun 10, 2026

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

How come we've dropped the async? I might just have a knowledge gap with JS practices but I'd have assumed we'd keep this function as async and call await on our credentialTelemetry.traceStore.

@EhabY EhabY Jun 10, 2026 •
edited
Loading

Copy link
Copy Markdown
Collaborator Author

Choose a reason for hiding this comment

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

We have lint rules that remove the async when there are no await, it might still be an async function that returns a promise but in this case we just have to return said promise without waiting

There is no advantage to just calling doing return promise over return await promise unless we want to catch errors, then the latter makes sense. Otherwise it's just noise

url: string,
token: string,
configs: Pick<WorkspaceConfiguration, "get">,
options?: { signal?: AbortSignal },
): Promise<void> {
const binPath = await this.resolveKeyringBinary(
url,
configs,
"keyringAuth",
);
if (!binPath) {
await this.writeCredentialFiles(url, token);
return;
}
return this.credentialTelemetry.traceStore(configs, async (span) => {
const binPath = await this.resolveKeyringBinary(
url,
configs,
"keyringAuth",
);
if (!binPath) {
span.setProperty("category", "file");
await this.writeCredentialFiles(url, token);
return;
}
span.setProperty("category", "keyring");
await this.storeKeyringToken(binPath, url, token, configs, options);
});
}

private async storeKeyringToken(
binPath: string,
url: string,
token: string,
configs: Pick<WorkspaceConfiguration, "get">,
options?: { signal?: AbortSignal },
): Promise<void> {
const args = [
...getHeaderArgs(configs),
"login",
Expand All @@ -89,7 +114,10 @@ export class CliCredentialManager {
this.logger.info("Stored token via CLI for", url);
} catch (error) {
this.logger.warn("Failed to store token via CLI:", error);
throw error;
if (isAbortError(error)) {
throw error;
}
throw new CredentialCliError(error);
}
}

Expand Down Expand Up @@ -139,15 +167,20 @@ export class CliCredentialManager {
* deletion in parallel, both best-effort. Throws AbortError when the
* signal is aborted.
*/
public async deleteToken(
public deleteToken(
Comment thread
EhabY marked this conversation as resolved.
url: string,
configs: Pick<WorkspaceConfiguration, "get">,
options?: { signal?: AbortSignal },
): Promise<void> {
await Promise.all([
this.deleteCredentialFiles(url),
this.deleteKeyringToken(url, configs, options?.signal),
]);
return this.credentialTelemetry.traceClear(configs, async (span) => {
await Promise.all([
this.deleteCredentialFiles(url),
this.deleteKeyringToken(url, configs, {
signal: options?.signal,
span,
}),
]);
});
}

/**
Expand Down Expand Up @@ -201,14 +234,18 @@ export class CliCredentialManager {
url: string,
token: string,
): Promise<void> {
const safeHostname = toSafeHost(url);
await Promise.all([
this.atomicWriteFile(this.pathResolver.getUrlPath(safeHostname), url),
this.atomicWriteFile(
this.pathResolver.getSessionTokenPath(safeHostname),
token,
),
]);
try {
const safeHostname = toSafeHost(url);
await Promise.all([
this.atomicWriteFile(this.pathResolver.getUrlPath(safeHostname), url),
this.atomicWriteFile(
this.pathResolver.getSessionTokenPath(safeHostname),
token,
),
]);
} catch (error) {
throw new CredentialFileError(error);
}
}

/**
Expand All @@ -230,18 +267,22 @@ export class CliCredentialManager {
}

/**
* Delete keyring token via `coder logout`. Best-effort: never throws.
* Delete keyring token via `coder logout`. Best-effort: records the failure
* on the span instead of throwing (except on abort), so it is tagged where
* it occurs.
*/
private async deleteKeyringToken(
url: string,
configs: Pick<WorkspaceConfiguration, "get">,
signal?: AbortSignal,
{ signal, span }: { signal?: AbortSignal; span: Span },
): Promise<void> {
let binPath: string | undefined;
try {
binPath = await this.resolveKeyringBinary(url, configs, "keyringAuth");
} catch (error) {
this.logger.warn("Could not resolve keyring binary for delete:", error);
span.setProperty("failure_category", "binary");
span.markFailure();
return;
}
if (!binPath) {
Expand All @@ -257,6 +298,8 @@ export class CliCredentialManager {
throw error;
}
this.logger.warn("Failed to delete token via CLI:", error);
span.setProperty("failure_category", "cli");
span.markFailure();
}
}

Expand Down
1 change: 1 addition & 0 deletions src/core/container.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ export class ServiceContainer implements vscode.Disposable {
}
},
this.pathResolver,
this.telemetryService,
);
this.cliManager = new CliManager(
this.logger,
Expand Down
Loading