From 21dbf25ce2f7ca4b5946b33d33cf0d249466f853 Mon Sep 17 00:00:00 2001 From: amtoney524 Date: Wed, 11 Feb 2026 18:37:50 -0600 Subject: [PATCH] feat: include access token in Clearcut request --- src/auth/auth-provider.ts | 16 +++- src/telemetry/client.ts | 21 +++-- src/telemetry/client.unit.test.ts | 121 ++++++++++++++++----------- src/telemetry/index.ts | 9 +- src/telemetry/telemetry.unit.test.ts | 15 ++-- 5 files changed, 117 insertions(+), 65 deletions(-) diff --git a/src/auth/auth-provider.ts b/src/auth/auth-provider.ts index 657582d9..78187d58 100644 --- a/src/auth/auth-provider.ts +++ b/src/auth/auth-provider.ts @@ -90,7 +90,8 @@ export class GoogleAuthProvider implements AuthenticationProvider, Disposable { } /** - * Retrieves the Google OAuth2 authentication session. + * Retrieves the Google OAuth2 authentication session. Prompts the user to + * sign in if no matching sessions are found. * * @param vs - The VS Code API. * @returns The authentication session. @@ -108,6 +109,19 @@ export class GoogleAuthProvider implements AuthenticationProvider, Disposable { return session; } + /** + * Retrieves the Google OAuth2 authentication session. + * + * @param vs - The VS Code API. + * @returns A promise that resolves with the authentication session if it + * exists and undefined otherwise. + */ + static async getSession( + vs: typeof vscode, + ): Promise { + return vs.authentication.getSession(PROVIDER_ID, REQUIRED_SCOPES); + } + /** * Disposes the provider and cleans up resources. */ diff --git a/src/telemetry/client.ts b/src/telemetry/client.ts index a15deead..4a870c9f 100644 --- a/src/telemetry/client.ts +++ b/src/telemetry/client.ts @@ -4,9 +4,12 @@ * SPDX-License-Identifier: Apache-2.0 */ -import fetch, { Request } from 'node-fetch'; +import fetch, { Headers, Request } from 'node-fetch'; import { Disposable } from 'vscode'; -import { CONTENT_TYPE_JSON_HEADER } from '../colab/headers'; +import { + AUTHORIZATION_HEADER, + CONTENT_TYPE_JSON_HEADER, +} from '../colab/headers'; import { log } from '../common/logging'; import { ColabLogEvent, @@ -36,6 +39,8 @@ export class ClearcutClient implements Disposable { // Queue of events to be flushed to Clearcut. private pendingEvents: LogEvent[] = []; + constructor(private getAccessToken: () => Promise) {} + dispose() { if (this.isDisposed) { return; @@ -99,12 +104,18 @@ export class ClearcutClient implements Disposable { log_source: LOG_SOURCE, log_event: events, }; + + const headers = new Headers(); + headers.set(CONTENT_TYPE_JSON_HEADER.key, CONTENT_TYPE_JSON_HEADER.value); + const token = await this.getAccessToken(); + if (token) { + headers.set(AUTHORIZATION_HEADER.key, `Bearer ${token}`); + } + const request = new Request(LOGS_ENDPOINT, { method: 'POST', body: JSON.stringify(logRequest), - headers: { - [CONTENT_TYPE_JSON_HEADER.key]: CONTENT_TYPE_JSON_HEADER.value, - }, + headers, }); const response = await fetch(request); // TODO: handle 401 once token is included in request diff --git a/src/telemetry/client.unit.test.ts b/src/telemetry/client.unit.test.ts index 6cf58a1c..2782df5a 100644 --- a/src/telemetry/client.unit.test.ts +++ b/src/telemetry/client.unit.test.ts @@ -24,6 +24,8 @@ const DEFAULT_LOG: ColabLogEvent = { ui_kind: 'UI_KIND_DESKTOP', vscode_version: '1.108.1', }; + +const TOKEN = 'token'; const LOG_RESPONSE = { next_request_wait_millis: 15 * 60 * 1000, }; @@ -36,10 +38,19 @@ describe('ClearcutClient', () => { let client: ClearcutClient; let fakeClock: SinonFakeTimers; let fetchStub: sinon.SinonStubbedMember; + let getAccessToken: Deferred; + let getAccessTokenStub: sinon.SinonStub<[], Promise>; beforeEach(() => { fakeClock = sinon.useFakeTimers({ now: NOW, toFake: [] }); - client = new ClearcutClient(); + getAccessToken = new Deferred(); + getAccessTokenStub = sinon + .stub<[], Promise>() + .callsFake(() => { + getAccessToken.resolve(); + return Promise.resolve(TOKEN); + }); + client = new ClearcutClient(getAccessTokenStub); fetchStub = sinon.stub(fetch, 'default'); }); @@ -48,10 +59,10 @@ describe('ClearcutClient', () => { }); describe('log', () => { - it('flushes an event to Clearcut', () => { + it('flushes an event to Clearcut', async () => { client.log(DEFAULT_LOG); - sinon.assert.calledOnceWithExactly(fetchStub, logRequest([DEFAULT_LOG])); + await assertFetchStubCalledOnceWithEvents([DEFAULT_LOG]); }); it('throws an error when Clearcut responds with a non-200 status', async () => { @@ -87,7 +98,7 @@ describe('ClearcutClient', () => { .resolves(FETCH_RESPONSE_OK); client.log(DEFAULT_LOG); - sinon.assert.calledOnceWithExactly(fetchStub, logRequest([DEFAULT_LOG])); + await assertFetchStubCalledOnceWithEvents([DEFAULT_LOG]); fetchStub.reset(); await fakeClock.tickAsync(TEST_ONLY.MIN_WAIT_BETWEEN_FLUSHES_MS); @@ -97,7 +108,7 @@ describe('ClearcutClient', () => { }; client.log(OTHER_LOG); - sinon.assert.calledOnceWithExactly(fetchStub, logRequest([OTHER_LOG])); + await assertFetchStubCalledOnceWithEvents([OTHER_LOG]); }); const requeueStatusTest = [{ status: 500 }, { status: 501 }]; @@ -110,10 +121,7 @@ describe('ClearcutClient', () => { .resolves(FETCH_RESPONSE_OK); client.log(DEFAULT_LOG); - sinon.assert.calledOnceWithExactly( - fetchStub, - logRequest([DEFAULT_LOG]), - ); + await assertFetchStubCalledOnceWithEvents([DEFAULT_LOG]); fetchStub.reset(); await fakeClock.tickAsync(TEST_ONLY.MIN_WAIT_BETWEEN_FLUSHES_MS); @@ -123,18 +131,16 @@ describe('ClearcutClient', () => { }; client.log(OTHER_LOG); - sinon.assert.calledOnceWithExactly( - fetchStub, - logRequest([DEFAULT_LOG, OTHER_LOG]), - ); + await assertFetchStubCalledOnceWithEvents([DEFAULT_LOG, OTHER_LOG]); }); } describe('on requeue', () => { let pendingFlush: Deferred; - beforeEach(() => { + beforeEach(async () => { client.log(DEFAULT_LOG); // Ensure next events are queued. + await getAccessToken.promise; fetchStub.resetHistory(); pendingFlush = new Deferred(); fetchStub.onFirstCall().callsFake(async () => { @@ -165,10 +171,7 @@ describe('ClearcutClient', () => { fetchStub.resetHistory(); client.dispose(); // Trigger immediate flush - sinon.assert.calledOnceWithExactly( - fetchStub, - logRequest([...failedLogs, ...newLogs]), - ); + await assertFetchStubCalledOnceWithEvents([...failedLogs, ...newLogs]); }); it('requeues some events when there is limited capacity', async () => { @@ -193,13 +196,10 @@ describe('ClearcutClient', () => { fetchStub.resetHistory(); client.dispose(); // Trigger immediate flush - sinon.assert.calledOnceWithExactly( - fetchStub, - logRequest([ - ...failedLogs.slice(TEST_ONLY.MAX_PENDING_EVENTS / 2), - ...newLogs, - ]), - ); + await assertFetchStubCalledOnceWithEvents([ + ...failedLogs.slice(TEST_ONLY.MAX_PENDING_EVENTS / 2), + ...newLogs, + ]); }); it('requeues no events when there is no capacity', async () => { @@ -224,7 +224,7 @@ describe('ClearcutClient', () => { fetchStub.resetHistory(); client.dispose(); // Trigger immediate flush - sinon.assert.calledOnceWithExactly(fetchStub, logRequest(newLogs)); + await assertFetchStubCalledOnceWithEvents(newLogs); }); }); @@ -236,7 +236,7 @@ describe('ClearcutClient', () => { // Log an event to trigger the first flush. client.log(firstLog); - sinon.assert.calledOnceWithExactly(fetchStub, logRequest([firstLog])); + await assertFetchStubCalledOnceWithEvents([firstLog]); fetchStub.resetHistory(); // While waiting for the flush interval to pass, log an event. @@ -259,10 +259,7 @@ describe('ClearcutClient', () => { client.log(thirdLog); // Verify that the two queued events were sent in a batch. - sinon.assert.calledOnceWithExactly( - fetchStub, - logRequest([secondLog, thirdLog]), - ); + await assertFetchStubCalledOnceWithEvents([secondLog, thirdLog]); }); it('queues events to send in batch when a flush is already pending', async () => { @@ -274,7 +271,7 @@ describe('ClearcutClient', () => { // Log an event to trigger the first flush. client.log(firstLog); - sinon.assert.calledOnceWithExactly(fetchStub, logRequest([firstLog])); + await assertFetchStubCalledOnceWithEvents([firstLog]); fetchStub.resetHistory(); // While waiting for the previous flush to resolve, log an event. @@ -299,10 +296,7 @@ describe('ClearcutClient', () => { client.log(thirdLog); // Verify that the two queued events were sent in a batch. - sinon.assert.calledOnceWithExactly( - fetchStub, - logRequest([secondLog, thirdLog]), - ); + await assertFetchStubCalledOnceWithEvents([secondLog, thirdLog]); }); it('drops oldest events when max pending events is exceeded', async () => { @@ -310,7 +304,7 @@ describe('ClearcutClient', () => { // Log an event to trigger the first flush. client.log(firstLog); - sinon.assert.calledOnceWithExactly(fetchStub, logRequest([firstLog])); + await assertFetchStubCalledOnceWithEvents([firstLog]); fetchStub.resetHistory(); const oldestLog = { @@ -330,7 +324,7 @@ describe('ClearcutClient', () => { } // Verify that the oldest event was dropped. - sinon.assert.calledOnceWithExactly(fetchStub, logRequest(newLogs)); + await assertFetchStubCalledOnceWithEvents(newLogs); }); }); }); @@ -340,7 +334,7 @@ describe('ClearcutClient', () => { // Log an event to trigger the first flush. client.log(DEFAULT_LOG); - sinon.assert.calledOnceWithExactly(fetchStub, logRequest([DEFAULT_LOG])); + await assertFetchStubCalledOnceWithEvents([DEFAULT_LOG]); fetchStub.resetHistory(); // Advance time to reach the flush interval. @@ -350,6 +344,7 @@ describe('ClearcutClient', () => { // Trigger flush client.log(DEFAULT_LOG); + await getAccessToken.promise; sinon.assert.calledOnce(fetchStub); }); @@ -380,7 +375,7 @@ describe('ClearcutClient', () => { // Log an event to trigger the first flush. client.log(DEFAULT_LOG); - sinon.assert.calledOnceWithExactly(fetchStub, logRequest([DEFAULT_LOG])); + await assertFetchStubCalledOnceWithEvents([DEFAULT_LOG]); fetchStub.resetHistory(); // Advance time to reach the flush interval. @@ -390,10 +385,26 @@ describe('ClearcutClient', () => { // Trigger flush client.log(DEFAULT_LOG); + await getAccessToken.promise; sinon.assert.calledOnce(fetchStub); }); } + it('does not include the access token in the Clearcut request when it is undefined', async () => { + getAccessTokenStub.callsFake(() => { + getAccessToken.resolve(); + return Promise.resolve(undefined); + }); + + client.log(DEFAULT_LOG); + + await getAccessToken.promise; + sinon.assert.calledOnceWithExactly( + fetchStub, + logRequest([DEFAULT_LOG], { 'Content-Type': 'application/json' }), + ); + }); + describe('dispose', () => { it('does nothing when there are no pending events', () => { client.dispose(); @@ -401,12 +412,12 @@ describe('ClearcutClient', () => { sinon.assert.notCalled(fetchStub); }); - it('forces a flush when the flush interval has not passed', () => { + it('forces a flush when the flush interval has not passed', async () => { fetchStub.resolves(FETCH_RESPONSE_OK); // Log an event to trigger the first flush. client.log(DEFAULT_LOG); - sinon.assert.calledOnceWithExactly(fetchStub, logRequest([DEFAULT_LOG])); + await assertFetchStubCalledOnceWithEvents([DEFAULT_LOG]); fetchStub.resetHistory(); // While the flush interval has not passed, log another event. This @@ -422,10 +433,10 @@ describe('ClearcutClient', () => { // Even though the flush interval has not passed, a second flush should // have been triggered by dispose. - sinon.assert.calledOnceWithExactly(fetchStub, logRequest([otherLog])); + await assertFetchStubCalledOnceWithEvents([otherLog]); }); - it('forces a flush when a flush is already pending', () => { + it('forces a flush when a flush is already pending', async () => { const flushPending = new Deferred(); fetchStub.onFirstCall().callsFake(async () => { await flushPending.promise; // Never resolved @@ -434,7 +445,7 @@ describe('ClearcutClient', () => { // Log an event to trigger the first flush. client.log(DEFAULT_LOG); - sinon.assert.calledOnceWithExactly(fetchStub, logRequest([DEFAULT_LOG])); + await assertFetchStubCalledOnceWithEvents([DEFAULT_LOG]); fetchStub.resetHistory(); // While the flush is still pending, log another event. This event @@ -450,9 +461,15 @@ describe('ClearcutClient', () => { // Even though the first flush has not resolved, a second flush should // have been triggered by dispose. - sinon.assert.calledOnceWithExactly(fetchStub, logRequest([otherLog])); + await assertFetchStubCalledOnceWithEvents([otherLog]); }); }); + + /** Helper to consolidate fetch stub assertion behavior. */ + async function assertFetchStubCalledOnceWithEvents(events: ColabLogEvent[]) { + await getAccessToken.promise; + sinon.assert.calledOnceWithExactly(fetchStub, logRequest(events)); + } }); /** @@ -469,7 +486,13 @@ function createLogEvents(numEvents: number): ColabLogEvent[] { /** * Helper to match the expected Clearcut log request structure */ -function logRequest(events: ColabLogEvent[]): Request { +function logRequest( + events: ColabLogEvent[], + headers: Record = { + 'Content-Type': 'application/json', + Authorization: `Bearer ${TOKEN}`, + }, +): Request { const logEvents = events.map((event) => ({ source_extension_json: JSON.stringify(event), })); @@ -479,8 +502,6 @@ function logRequest(events: ColabLogEvent[]): Request { log_source: LOG_SOURCE, log_event: logEvents, }), - headers: { - 'Content-Type': 'application/json', - }, + headers, }); } diff --git a/src/telemetry/index.ts b/src/telemetry/index.ts index 81552843..58686a51 100644 --- a/src/telemetry/index.ts +++ b/src/telemetry/index.ts @@ -22,9 +22,14 @@ let isTelemetryEnabled: () => boolean; * Initializes the telemetry module * @param context - The VS Code extension context * @param vs - The vscode module. + * @param getAccessToken - A function that resolves with the access token, or + * undefined if the user is unauthenticated. * @returns A {@link Disposable} that can be used to clean up the client. */ -export function initializeTelemetry(vs: typeof vscode): Disposable { +export function initializeTelemetry( + vs: typeof vscode, + getAccessToken: () => Promise, +): Disposable { if (client) { throw new Error('Telemetry has already been initialized.'); } @@ -46,7 +51,7 @@ export function initializeTelemetry(vs: typeof vscode): Disposable { }; isTelemetryEnabled = () => vs.env.isTelemetryEnabled; - client = new ClearcutClient(); + client = new ClearcutClient(getAccessToken); return { dispose: () => { diff --git a/src/telemetry/telemetry.unit.test.ts b/src/telemetry/telemetry.unit.test.ts index 3a5c3024..40b9db9b 100644 --- a/src/telemetry/telemetry.unit.test.ts +++ b/src/telemetry/telemetry.unit.test.ts @@ -23,6 +23,7 @@ const VERSION_VSCODE = '1.109.0'; describe('Telemetry Module', () => { let disposeTelemetry: Disposable | undefined; + const getAccessToken = () => Promise.resolve(undefined); let fakeClock: SinonFakeTimers; let vs: VsCodeStub; @@ -50,10 +51,10 @@ describe('Telemetry Module', () => { describe('lifecycle', () => { it('throws if doubly initialized', () => { - disposeTelemetry = initializeTelemetry(vs.asVsCode()); + disposeTelemetry = initializeTelemetry(vs.asVsCode(), getAccessToken); expect(() => { - initializeTelemetry(vs.asVsCode()); + initializeTelemetry(vs.asVsCode(), getAccessToken); }).to.throw(/already been initialized/); }); @@ -65,7 +66,7 @@ describe('Telemetry Module', () => { it('disposes the client when disposed', () => { const disposeSpy = sinon.spy(ClearcutClient.prototype, 'dispose'); - disposeTelemetry = initializeTelemetry(vs.asVsCode()); + disposeTelemetry = initializeTelemetry(vs.asVsCode(), getAccessToken); disposeTelemetry.dispose(); @@ -76,7 +77,7 @@ describe('Telemetry Module', () => { it('does not log to Clearcut when telemetry is disabled', () => { const logStub = sinon.stub(ClearcutClient.prototype, 'log'); vs.env.isTelemetryEnabled = false; - disposeTelemetry = initializeTelemetry(vs.asVsCode()); + disposeTelemetry = initializeTelemetry(vs.asVsCode(), getAccessToken); telemetry.logActivation(); @@ -88,7 +89,7 @@ describe('Telemetry Module', () => { vs.env.isTelemetryEnabled = false; // Maintain a reference to this stub as that's the reference telemetry has. const vscodeStub = vs.asVsCode(); - disposeTelemetry = initializeTelemetry(vscodeStub); + disposeTelemetry = initializeTelemetry(vscodeStub, getAccessToken); telemetry.logActivation(); sinon.assert.notCalled(logStub); @@ -105,7 +106,7 @@ describe('Telemetry Module', () => { const logStub = sinon.stub(ClearcutClient.prototype, 'log'); // Maintain a reference to this stub as that's the reference telemetry has. const vscodeStub = vs.asVsCode(); - disposeTelemetry = initializeTelemetry(vscodeStub); + disposeTelemetry = initializeTelemetry(vscodeStub, getAccessToken); telemetry.logActivation(); sinon.assert.calledOnce(logStub); @@ -136,7 +137,7 @@ describe('Telemetry Module', () => { vscode_version: VERSION_VSCODE, timestamp: new Date(NOW).toISOString(), }; - disposeTelemetry = initializeTelemetry(vs.asVsCode()); + disposeTelemetry = initializeTelemetry(vs.asVsCode(), getAccessToken); }); it('on activation', () => {