From a988f8b04b78fe16444ccca711cd987c011eec4d Mon Sep 17 00:00:00 2001 From: Quinn Slack Date: Sun, 15 Sep 2024 17:44:28 -0700 Subject: [PATCH] make some services more reactive to config (#5550) Partial application of https://github.com/sourcegraph/cody/pull/5221 to some services: FeatureFlagProvider, ClientStateBroadcaster, UpstreamHealthProvider, OpenTelemetryService, SentryService, etc. The idea is to make services listen to config instead of checking it once and not updating themselves when config changes, which is bad because users need to reload the editor sometimes when they change config. ## Test plan CI --------- Co-authored-by: Valery Bugakov --- agent/src/TestClient.ts | 3 +- agent/src/cli/command-chat.test.ts | 1 - .../cody-ignore/context-filters-provider.ts | 1 - .../FeatureFlagProvider.test.ts | 230 ++++++------ .../experimentation/FeatureFlagProvider.ts | 333 +++++++----------- lib/shared/src/guardrails/client.ts | 21 +- lib/shared/src/index.ts | 2 +- lib/shared/src/misc/observable.test.ts | 51 +++ lib/shared/src/misc/observable.ts | 212 ++++++++++- .../src/sourcegraph-api/graphql/client.ts | 15 +- vscode/src/chat/clientStateBroadcaster.ts | 5 +- .../get-inline-completions-tests/helpers.ts | 1 + .../inline-completion-item-provider.test.ts | 13 +- .../providers/create-provider.test.ts | 8 +- .../openctx/common/get-repository-mentions.ts | 11 +- .../context/openctx/remoteDirectorySearch.ts | 7 +- .../src/context/openctx/remoteFileSearch.ts | 7 +- .../context/openctx/remoteRepositorySearch.ts | 10 +- vscode/src/editor/utils/editor-context.ts | 4 +- vscode/src/extension.common.ts | 12 +- vscode/src/external-services.ts | 9 +- vscode/src/local-context/symf.ts | 2 +- vscode/src/main.ts | 22 +- vscode/src/models/sync.test.ts | 3 + .../notifications/cody-pro-expiration.test.ts | 20 +- .../src/notifications/setup-notification.ts | 8 +- vscode/src/services/AuthProvider.ts | 3 +- vscode/src/services/UpstreamHealthProvider.ts | 75 ++-- .../OpenTelemetryService.node.ts | 65 ++-- vscode/src/services/open-telemetry/utils.ts | 4 +- vscode/src/services/sentry/sentry.ts | 113 +++--- vscode/src/services/telemetry-v2.ts | 136 +++---- vscode/src/test-support.ts | 11 +- vscode/src/testutils/mocks.ts | 18 - vscode/test/integration/helpers.ts | 26 +- 35 files changed, 797 insertions(+), 665 deletions(-) diff --git a/agent/src/TestClient.ts b/agent/src/TestClient.ts index 27911d32cd34..905047115388 100644 --- a/agent/src/TestClient.ts +++ b/agent/src/TestClient.ts @@ -85,7 +85,6 @@ interface TestClientParams { readonly credentials: TestingCredentials bin?: string telemetryExporter?: 'testing' | 'graphql' // defaults to testing, which doesn't send telemetry - areFeatureFlagsEnabled?: boolean // do not evaluate feature flags by default logEventMode?: 'connected-instance-only' | 'all' | 'dotcom-only' onWindowRequest?: (params: ShowWindowMessageParams) => Promise extraConfiguration?: Record @@ -165,7 +164,7 @@ export class TestClient extends MessageHandler { SRC_ACCESS_TOKEN: params.credentials.token, REDACTED_SRC_ACCESS_TOKEN: params.credentials.redactedToken, CODY_TELEMETRY_EXPORTER: params.telemetryExporter ?? 'testing', - DISABLE_FEATURE_FLAGS: params.areFeatureFlagsEnabled ? undefined : 'true', + DISABLE_FEATURE_FLAGS: 'true', DISABLE_UPSTREAM_HEALTH_PINGS: 'true', CODY_LOG_EVENT_MODE: params.logEventMode, ...process.env, diff --git a/agent/src/cli/command-chat.test.ts b/agent/src/cli/command-chat.test.ts index 701c6791954a..5d89f66af427 100644 --- a/agent/src/cli/command-chat.test.ts +++ b/agent/src/cli/command-chat.test.ts @@ -30,7 +30,6 @@ describe('cody chat', () => { args: string[] expectedExitCode?: number }): Promise { - process.env.DISABLE_FEATURE_FLAGS = 'true' process.env.CODY_TELEMETRY_EXPORTER = 'testing' const args = [ ...params.args, diff --git a/lib/shared/src/cody-ignore/context-filters-provider.ts b/lib/shared/src/cody-ignore/context-filters-provider.ts index 9c850d08bef1..dbf92a34971f 100644 --- a/lib/shared/src/cody-ignore/context-filters-provider.ts +++ b/lib/shared/src/cody-ignore/context-filters-provider.ts @@ -327,6 +327,5 @@ function parseContextFilterItem(item: CodyContextFilterItem): ParsedContextFilte /** * A singleton instance of the `ContextFiltersProvider` class. - * `contextFiltersProvider.init` should be called and awaited on extension activation. */ export const contextFiltersProvider = new ContextFiltersProvider() diff --git a/lib/shared/src/experimentation/FeatureFlagProvider.test.ts b/lib/shared/src/experimentation/FeatureFlagProvider.test.ts index 6cfc3d4bb513..cf10019320c7 100644 --- a/lib/shared/src/experimentation/FeatureFlagProvider.test.ts +++ b/lib/shared/src/experimentation/FeatureFlagProvider.test.ts @@ -1,124 +1,70 @@ -import { beforeEach, describe, expect, it, vi, vitest } from 'vitest' +import { afterEach, beforeAll, beforeEach, describe, expect, it, vi, vitest } from 'vitest' import { graphqlClient } from '../sourcegraph-api/graphql' +import { mockAuthStatus } from '../auth/authStatus' +import { AUTH_STATUS_FIXTURE_AUTHED } from '../auth/types' +import { mockResolvedConfig } from '../configuration/resolver' import { readValuesFrom } from '../misc/observable' -import type { GraphQLAPIClientConfig } from '../sourcegraph-api/graphql/client' -import { nextTick } from '../utils' -import { FeatureFlag, FeatureFlagProvider } from './FeatureFlagProvider' +import { FeatureFlag, FeatureFlagProviderImpl } from './FeatureFlagProvider' vi.mock('../sourcegraph-api/graphql/client') describe('FeatureFlagProvider', () => { - beforeEach(() => { - // @ts-ignore - graphqlClient._config = { - serverEndpoint: 'https://example.com', - } as Partial as GraphQLAPIClientConfig + beforeAll(() => { + vi.useFakeTimers() + mockResolvedConfig({ + auth: { accessToken: null, serverEndpoint: 'https://example.com' }, + }) + mockAuthStatus(AUTH_STATUS_FIXTURE_AUTHED) }) - it('evaluates the feature flag on dotcom', async () => { - vi.spyOn(graphqlClient, 'getEvaluatedFeatureFlags').mockResolvedValue({}) - vi.spyOn(graphqlClient, 'evaluateFeatureFlag').mockResolvedValue(true) - - const provider = new FeatureFlagProvider() + let featureFlagProvider: FeatureFlagProviderImpl + beforeEach(() => { + featureFlagProvider = new FeatureFlagProviderImpl() + }) - expect(await provider.evaluateFeatureFlag(FeatureFlag.TestFlagDoNotUse)).toBe(true) + let unsubscribeAfter: (() => void) | undefined = undefined + afterEach(() => { + vi.clearAllMocks() + unsubscribeAfter?.() + unsubscribeAfter = undefined }) - it('loads all evaluated feature flag on `syncAuthStatus`', async () => { + it('evaluates a single feature flag', async () => { const getEvaluatedFeatureFlagsMock = vi .spyOn(graphqlClient, 'getEvaluatedFeatureFlags') - .mockResolvedValue({ - [FeatureFlag.TestFlagDoNotUse]: true, - }) - const evaluateFeatureFlagMock = vi.spyOn(graphqlClient, 'evaluateFeatureFlag') - - const provider = new FeatureFlagProvider() - await provider.refresh() - - // Wait for the async initialization - await nextTick() + .mockResolvedValue({}) + const evaluateFeatureFlagMock = vi + .spyOn(graphqlClient, 'evaluateFeatureFlag') + .mockResolvedValue(true) + + expect(await featureFlagProvider.evaluateFeatureFlag(FeatureFlag.TestFlagDoNotUse)).toBe(true) + expect(getEvaluatedFeatureFlagsMock).toHaveBeenCalledTimes(0) + expect(evaluateFeatureFlagMock).toHaveBeenCalledOnce() + }) - expect(await provider.evaluateFeatureFlag(FeatureFlag.TestFlagDoNotUse)).toBe(true) - expect(getEvaluatedFeatureFlagsMock).toHaveBeenCalled() - expect(evaluateFeatureFlagMock).not.toHaveBeenCalled() + it('reports exposed experiments', async () => { + vi.spyOn(graphqlClient, 'getEvaluatedFeatureFlags').mockResolvedValue({ + [FeatureFlag.TestFlagDoNotUse]: true, + }) + vi.spyOn(graphqlClient, 'evaluateFeatureFlag').mockResolvedValue(true) + const { unsubscribe } = readValuesFrom( + featureFlagProvider.evaluatedFeatureFlag(FeatureFlag.TestFlagDoNotUse) + ) + unsubscribeAfter = unsubscribe + await vi.runOnlyPendingTimersAsync() + expect(featureFlagProvider.getExposedExperiments('https://example.com')).toStrictEqual({ + [FeatureFlag.TestFlagDoNotUse]: true, + }) + expect(featureFlagProvider.getExposedExperiments('https://other.example.com')).toStrictEqual({}) }) it('should handle API errors', async () => { vi.spyOn(graphqlClient, 'getEvaluatedFeatureFlags').mockResolvedValue(new Error('API error')) vi.spyOn(graphqlClient, 'evaluateFeatureFlag').mockResolvedValue(new Error('API error')) - const provider = new FeatureFlagProvider() - - expect(await provider.evaluateFeatureFlag(FeatureFlag.TestFlagDoNotUse)).toBe(false) - }) - - it('should refresh flags', async () => { - const getEvaluatedFeatureFlagsMock = vi - .spyOn(graphqlClient, 'getEvaluatedFeatureFlags') - .mockResolvedValue({ - [FeatureFlag.TestFlagDoNotUse]: true, - }) - vi.spyOn(graphqlClient, 'evaluateFeatureFlag') - - const provider = new FeatureFlagProvider() - - // Wait for the async initialization - await nextTick() - - getEvaluatedFeatureFlagsMock.mockResolvedValue({ - [FeatureFlag.TestFlagDoNotUse]: false, - }) - - await provider.refresh() - - // Wait for the async reload - await nextTick() - - expect(await provider.evaluateFeatureFlag(FeatureFlag.TestFlagDoNotUse)).toBe(false) - }) - - it('should refresh flags after one hour', async () => { - const originalNow = Date.now - try { - Date.now = () => 0 - const getEvaluatedFeatureFlagsMock = vi - .spyOn(graphqlClient, 'getEvaluatedFeatureFlags') - .mockResolvedValue({ - [FeatureFlag.TestFlagDoNotUse]: true, - }) - const evaluateFeatureFlagMock = vi.spyOn(graphqlClient, 'evaluateFeatureFlag') - - const provider = new FeatureFlagProvider() - await provider.refresh() - - // Wait for the async initialization - await nextTick() - - expect(await provider.evaluateFeatureFlag(FeatureFlag.TestFlagDoNotUse)).toBe(true) - expect(getEvaluatedFeatureFlagsMock).toHaveBeenCalled() - expect(evaluateFeatureFlagMock).not.toHaveBeenCalled() - - getEvaluatedFeatureFlagsMock.mockResolvedValue({ - [FeatureFlag.TestFlagDoNotUse]: false, - }) - - Date.now = () => 61 * 60 * 1000 - - // We have a stale-while-revalidate cache so this will return the previous value while it - // is reloading - expect(await provider.evaluateFeatureFlag(FeatureFlag.TestFlagDoNotUse)).toBe(true) - expect(getEvaluatedFeatureFlagsMock).toHaveBeenCalled() - expect(evaluateFeatureFlagMock).not.toHaveBeenCalled() - - // Wait for the async reload - await nextTick() - - expect(await provider.evaluateFeatureFlag(FeatureFlag.TestFlagDoNotUse)).toBe(false) - } finally { - Date.now = originalNow - } + expect(await featureFlagProvider.evaluateFeatureFlag(FeatureFlag.TestFlagDoNotUse)).toBe(false) }) describe('evaluatedFeatureFlag', () => { @@ -127,20 +73,19 @@ describe('FeatureFlagProvider', () => { updateMocks, expectFinalValues, }: { - expectInitialValues: (boolean | undefined)[] + expectInitialValues: boolean[] updateMocks?: () => void - expectFinalValues?: (boolean | undefined)[] + expectFinalValues?: boolean[] }): Promise { vitest.useFakeTimers() - const provider = new FeatureFlagProvider() - - const flag$ = provider.evaluatedFeatureFlag(FeatureFlag.TestFlagDoNotUse) - const { values, done, unsubscribe } = readValuesFrom(flag$) - vitest.runAllTimers() + const { values, done, unsubscribe } = readValuesFrom( + featureFlagProvider.evaluatedFeatureFlag(FeatureFlag.TestFlagDoNotUse) + ) + unsubscribeAfter = unsubscribe // Test the initial emissions. - await nextTick() + await vi.runOnlyPendingTimersAsync() expect(values).toEqual(expectInitialValues) values.length = 0 @@ -150,8 +95,8 @@ describe('FeatureFlagProvider', () => { // Test that the observable emits updated values when flags change. updateMocks() - provider.refresh() - await nextTick() + featureFlagProvider.refresh() + await vi.runOnlyPendingTimersAsync() expect(values).toEqual(expectFinalValues!) values.length = 0 @@ -161,10 +106,10 @@ describe('FeatureFlagProvider', () => { expect(values).toEqual([]) } - it('should emit when a new flag is evaluated', async () => { + it('should emit when a new flag is evaluated', { timeout: 500 }, async () => { vi.spyOn(graphqlClient, 'getEvaluatedFeatureFlags').mockResolvedValue({}) vi.spyOn(graphqlClient, 'evaluateFeatureFlag').mockResolvedValue(false) - await testEvaluatedFeatureFlag({ expectInitialValues: [undefined, false] }) + await testEvaluatedFeatureFlag({ expectInitialValues: [false] }) }) it('should emit when value changes from true to false', async () => { @@ -189,7 +134,6 @@ describe('FeatureFlagProvider', () => { [FeatureFlag.TestFlagDoNotUse]: false, }) vi.spyOn(graphqlClient, 'evaluateFeatureFlag').mockResolvedValue(false) - await testEvaluatedFeatureFlag({ expectInitialValues: [false], updateMocks: () => { @@ -202,7 +146,7 @@ describe('FeatureFlagProvider', () => { }) }) - it('should emit undefined when a previously false flag is no longer in the exposed list', async () => { + it('should not emit false when a previously false flag is no longer in the exposed list', async () => { vi.spyOn(graphqlClient, 'getEvaluatedFeatureFlags').mockResolvedValue({ [FeatureFlag.TestFlagDoNotUse]: false, }) @@ -213,8 +157,66 @@ describe('FeatureFlagProvider', () => { vi.spyOn(graphqlClient, 'getEvaluatedFeatureFlags').mockResolvedValue({}) vi.spyOn(graphqlClient, 'evaluateFeatureFlag').mockResolvedValue(null) }, - expectFinalValues: [undefined], + expectFinalValues: [], + }) + }) + + it('should refresh flags when the endpoint changes', async () => { + const getEvaluatedFeatureFlagsMock = vi + .spyOn(graphqlClient, 'getEvaluatedFeatureFlags') + .mockResolvedValue({ + [FeatureFlag.TestFlagDoNotUse]: true, + }) + const evaluateFeatureFlagMock = vi + .spyOn(graphqlClient, 'evaluateFeatureFlag') + .mockResolvedValue(true) + mockAuthStatus({ ...AUTH_STATUS_FIXTURE_AUTHED, endpoint: 'https://example.com' }) + + expect(await featureFlagProvider.evaluateFeatureFlag(FeatureFlag.TestFlagDoNotUse)).toBe( + true + ) + + getEvaluatedFeatureFlagsMock.mockResolvedValue({ + [FeatureFlag.TestFlagDoNotUse]: false, + }) + evaluateFeatureFlagMock.mockResolvedValue(false) + mockAuthStatus({ ...AUTH_STATUS_FIXTURE_AUTHED, endpoint: 'https://other.example.com' }) + await vi.runOnlyPendingTimersAsync() + expect(await featureFlagProvider.evaluateFeatureFlag(FeatureFlag.TestFlagDoNotUse)).toBe( + false + ) + }) + + it('refresh()', async () => { + vi.clearAllMocks() + const getEvaluatedFeatureFlagsMock = vi + .spyOn(graphqlClient, 'getEvaluatedFeatureFlags') + .mockResolvedValue({ + [FeatureFlag.TestFlagDoNotUse]: true, + }) + const evaluateFeatureFlagMock = vi + .spyOn(graphqlClient, 'evaluateFeatureFlag') + .mockResolvedValue(true) + + const { values, unsubscribe } = readValuesFrom( + featureFlagProvider.evaluatedFeatureFlag(FeatureFlag.TestFlagDoNotUse) + ) + unsubscribeAfter = unsubscribe + + await vi.runOnlyPendingTimersAsync() + expect(values).toStrictEqual([true]) + values.length = 0 + expect(getEvaluatedFeatureFlagsMock).toHaveBeenCalledTimes(1) + expect(evaluateFeatureFlagMock).toHaveBeenCalledTimes(1) + + getEvaluatedFeatureFlagsMock.mockResolvedValue({ + [FeatureFlag.TestFlagDoNotUse]: false, }) + featureFlagProvider.refresh() + await vi.runOnlyPendingTimersAsync() + expect(values).toStrictEqual([false]) + expect(getEvaluatedFeatureFlagsMock).toHaveBeenCalledTimes(2) + expect(evaluateFeatureFlagMock).toHaveBeenCalledTimes(1) }) }) }) diff --git a/lib/shared/src/experimentation/FeatureFlagProvider.ts b/lib/shared/src/experimentation/FeatureFlagProvider.ts index 8e6a80708569..209328dfa9ee 100644 --- a/lib/shared/src/experimentation/FeatureFlagProvider.ts +++ b/lib/shared/src/experimentation/FeatureFlagProvider.ts @@ -1,8 +1,19 @@ -import { Observable } from 'observable-fns' -import type { Event } from 'vscode' -import { logDebug } from '../logger' -import { distinctUntilChanged, fromVSCodeEvent } from '../misc/observable' -import { graphqlClient } from '../sourcegraph-api/graphql/client' +import { Observable, Subject, interval, map } from 'observable-fns' +import { authStatus } from '../auth/authStatus' +import type { AuthStatus, AuthenticatedAuthStatus } from '../auth/types' +import { logError } from '../logger' +import { + combineLatest, + concat, + debounceTime, + distinctUntilChanged, + firstValueFrom, + mergeMap, + promiseFactoryToObservable, + shareReplay, + startWith, +} from '../misc/observable' +import { graphqlClient } from '../sourcegraph-api/graphql' import { wrapInActiveSpan } from '../tracing' import { isError } from '../utils' @@ -81,223 +92,129 @@ export enum FeatureFlag { const ONE_HOUR = 60 * 60 * 1000 -export class FeatureFlagProvider { - // The exposed feature flags are one where the backend returns a non-null value and thus we know - // the user is in either the test or control group. - // - // The first key maps to the endpoint so that we do never cache the wrong flag for different - // endpoints - private exposedFeatureFlags: Record> = {} - private lastRefreshTimestamp = 0 - // Unexposed feature flags are cached differently since they don't usually mean that the backend - // won't have access to this feature flag. Those will not automatically update when feature - // flags are updated in the background. - private unexposedFeatureFlags: Record> = {} - - private subscriptionsForEndpoint: Map< - string, // ${endpoint}#${prefix filter} - { lastSnapshot: Record; callbacks: Set<() => void> } - > = new Map() - // When we have at least one subscription, ensure that we also periodically refresh the flags - private nextRefreshTimeout: NodeJS.Timeout | number | undefined = undefined +export interface FeatureFlagProvider { + evaluateFeatureFlag(flag: FeatureFlag): Promise + evaluatedFeatureFlag(flag: FeatureFlag): Observable + getExposedExperiments(serverEndpoint: string): Record + refresh(): void +} +export class FeatureFlagProviderImpl implements FeatureFlagProvider { /** - * Get a flag's value from the cache. The returned value could be stale. You must have - * previously called {@link FeatureFlagProvider.evaluateFeatureFlag} or - * {@link FeatureFlagProvider.evaluatedFeatureFlag} to ensure that this feature flag's value is - * present in the cache. For that reason, this method is private because it is easy for external - * callers to mess that up when calling it. + * The cached exposed feature flags are ones where the backend returns a non-null value and thus + * we know the user is in either the test or control group. + * + * The first key maps to the endpoint so that we never cache the wrong flag for different + * endpoints. */ - private getFromCache(flagName: FeatureFlag): boolean | undefined { - void this.refreshIfStale() - - const endpoint = graphqlClient.endpoint - - const exposedValue = this.exposedFeatureFlags[endpoint]?.[flagName] - if (exposedValue !== undefined) { - return exposedValue - } - - if (this.unexposedFeatureFlags[endpoint]?.has(flagName)) { - return false - } - - return undefined - } - - public getExposedExperiments(): Record { - const endpoint = graphqlClient.endpoint - return this.exposedFeatureFlags[endpoint] || {} + private cache: Record> = {} + + private refreshRequests = new Subject() + private refreshes: Observable = combineLatest([ + this.refreshRequests.pipe(startWith(undefined)), + interval(ONE_HOUR).pipe(startWith(undefined)), + ]).pipe(map(() => undefined)) + + private relevantAuthStatusChanges: Observable< + Pick & + Partial> + > = authStatus.pipe( + map(authStatus => ({ + authenticated: authStatus.authenticated, + endpoint: authStatus.endpoint, + username: authStatus.authenticated ? authStatus.username : undefined, + })), + distinctUntilChanged() + ) + + private evaluatedFeatureFlags: Observable> = combineLatest([ + this.relevantAuthStatusChanges, + this.refreshes, + ]).pipe( + debounceTime(0), + mergeMap(([authStatus]) => + promiseFactoryToObservable(signal => + process.env.DISABLE_FEATURE_FLAGS + ? Promise.resolve({}) + : graphqlClient.getEvaluatedFeatureFlags(signal) + ).pipe( + map(resultOrError => { + if (isError(resultOrError)) { + logError( + 'FeatureFlagProvider', + 'Failed to get all evaluated feature flags', + resultOrError + ) + } + + // Cache so that FeatureFlagProvider.getExposedExperiments can return these synchronously. + const result = isError(resultOrError) ? {} : resultOrError + this.cache[authStatus.endpoint] = result + return result + }) + ) + ), + distinctUntilChanged(), + shareReplay() + ) + + public getExposedExperiments(serverEndpoint: string): Record { + return this.cache[serverEndpoint] || {} } public async evaluateFeatureFlag(flagName: FeatureFlag): Promise { - const endpoint = graphqlClient.endpoint - return wrapInActiveSpan(`FeatureFlagProvider.evaluateFeatureFlag.${flagName}`, async () => { - if (process.env.DISABLE_FEATURE_FLAGS) { - return false - } - - const cachedValue = this.getFromCache(flagName) - if (cachedValue !== undefined) { - return cachedValue - } - - const value = await graphqlClient.evaluateFeatureFlag(flagName) - - if (value === null || typeof value === 'undefined' || isError(value)) { - // The backend does not know about this feature flag, so we can't know if the user - // is in the test or control group. - if (!this.unexposedFeatureFlags[endpoint]) { - this.unexposedFeatureFlags[endpoint] = new Set() - } - this.unexposedFeatureFlags[endpoint].add(flagName) - return false - } - - if (!this.exposedFeatureFlags[endpoint]) { - this.exposedFeatureFlags[endpoint] = {} - } - this.exposedFeatureFlags[endpoint][flagName] = value - return value - }) - } - - /** - * Observe the evaluated value of a feature flag. - */ - public evaluatedFeatureFlag(flagName: FeatureFlag): Observable { - if (process.env.DISABLE_FEATURE_FLAGS) { - return Observable.of(undefined) - } - - const onChangeEvent: Event = ( - listener: (value: boolean | undefined) => void - ) => { - const dispose = this.onFeatureFlagChanged(() => listener(this.getFromCache(flagName))) - return { dispose } - } - return fromVSCodeEvent(onChangeEvent, () => this.evaluateFeatureFlag(flagName)).pipe( - distinctUntilChanged() + return wrapInActiveSpan(`FeatureFlagProvider.evaluateFeatureFlag.${flagName}`, () => + firstValueFrom(this.evaluatedFeatureFlag(flagName)) ) } - public async refresh(): Promise { - this.exposedFeatureFlags = {} - this.unexposedFeatureFlags = {} - await this.refreshFeatureFlags() - } - - private async refreshIfStale(): Promise { - const now = Date.now() - if (now - this.lastRefreshTimestamp > ONE_HOUR) { - // Cache expired, refresh - await this.refreshFeatureFlags() - } - } - - private async refreshFeatureFlags(): Promise { - return wrapInActiveSpan('FeatureFlagProvider.refreshFeatureFlags', async () => { - const endpoint = graphqlClient.endpoint - const data = process.env.DISABLE_FEATURE_FLAGS - ? {} - : await graphqlClient.getEvaluatedFeatureFlags() - - this.exposedFeatureFlags[endpoint] = isError(data) ? {} : data - - this.lastRefreshTimestamp = Date.now() - this.notifyFeatureFlagChanged() - - if (this.nextRefreshTimeout) { - clearTimeout(this.nextRefreshTimeout) - this.nextRefreshTimeout = undefined - } - if (this.subscriptionsForEndpoint.size > 0) { - this.nextRefreshTimeout = setTimeout(() => this.refreshFeatureFlags(), ONE_HOUR) - } - }) - } - /** - * Allows you to subscribe to a change event that is triggered when feature flags change that - * the user is currently exposed to. - * - * Note this will only update feature flags that a user is currently exposed to. For feature - * flags not defined upstream, the changes will require a new call to - * {@link FeatureFlagProvider.evaluateFeatureFlag} or - * {@link FeatureFlagProvider.evaluatedFeatureFlag} to be picked up. + * Observe the evaluated value of a feature flag. */ - private onFeatureFlagChanged(callback: () => void): () => void { - const endpoint = graphqlClient.endpoint - const subscription = this.subscriptionsForEndpoint.get(endpoint) - if (subscription) { - subscription.callbacks.add(callback) - return () => subscription.callbacks.delete(callback) - } - this.subscriptionsForEndpoint.set(endpoint, { - lastSnapshot: this.computeFeatureFlagSnapshot(endpoint), - callbacks: new Set([callback]), - }) - - if (!this.nextRefreshTimeout) { - this.nextRefreshTimeout = setTimeout(() => { - this.nextRefreshTimeout = undefined - void this.refreshFeatureFlags() - }, ONE_HOUR) - } - - return () => { - const subscription = this.subscriptionsForEndpoint.get(endpoint) - if (subscription) { - subscription.callbacks.delete(callback) - if (subscription.callbacks.size === 0) { - this.subscriptionsForEndpoint.delete(endpoint) - } - - if (this.subscriptionsForEndpoint.size === 0 && this.nextRefreshTimeout) { - clearTimeout(this.nextRefreshTimeout) - this.nextRefreshTimeout = undefined - } - } - } + public evaluatedFeatureFlag(flagName: FeatureFlag): Observable { + // Whenever the auth status changes, we need to call `evaluateFeatureFlag` on the GraphQL + // endpoint, because our endpoint or authentication may have changed, and + // `getEvaluatedFeatureFlags` only returns the set of recently evaluated feature flags. + return combineLatest([this.relevantAuthStatusChanges, this.refreshes]) + .pipe( + mergeMap(([authStatus]) => + concat( + promiseFactoryToObservable(async signal => { + if (process.env.DISABLE_FEATURE_FLAGS) { + return false + } + + const cachedValue = this.cache[authStatus.endpoint]?.[flagName.toString()] + if (cachedValue !== undefined) { + // We'll immediately return the cached value and then start observing + // for updates. + return cachedValue + } + + const result = await graphqlClient.evaluateFeatureFlag(flagName, signal) + return isError(result) ? false : result ?? false + }), + this.evaluatedFeatureFlags.pipe( + map(featureFlags => Boolean(featureFlags[flagName.toString()])) + ) + ) + ) + ) + .pipe(distinctUntilChanged()) } - private notifyFeatureFlagChanged(): void { - const callbacksToTrigger: (() => void)[] = [] - for (const [endpoint, subs] of this.subscriptionsForEndpoint) { - const currentSnapshot = this.computeFeatureFlagSnapshot(endpoint) - // We only care about flags being changed that we previously already captured. A new - // evaluation should not trigger a change event unless that new value is later changed. - if ( - subs.lastSnapshot === NO_FLAGS || - computeIfExistingFlagChanged(subs.lastSnapshot, currentSnapshot) - ) { - for (const callback of subs.callbacks) { - callbacksToTrigger.push(callback) - } - } - subs.lastSnapshot = currentSnapshot - } - // Disable on CI to unclutter the output. - if (!process.env.VITEST) { - logDebug('featureflag', 'refreshed') - } - for (const callback of callbacksToTrigger) { - callback() - } - } - - private computeFeatureFlagSnapshot(endpoint: string): Record { - return this.exposedFeatureFlags[endpoint] ?? NO_FLAGS + public refresh(): void { + this.refreshRequests.next() } } -const NO_FLAGS: Record = {} - -export const featureFlagProvider = new FeatureFlagProvider() - -function computeIfExistingFlagChanged( - oldFlags: Record, - newFlags: Record -): boolean { - return Object.keys(oldFlags).some(key => oldFlags[key] !== newFlags[key]) +const noopFeatureFlagProvider: FeatureFlagProvider = { + evaluateFeatureFlag: async () => false, + evaluatedFeatureFlag: () => Observable.of(false), + getExposedExperiments: () => ({}), + refresh: () => {}, } + +export const featureFlagProvider = process.env.DISABLE_FEATURE_FLAGS + ? noopFeatureFlagProvider + : new FeatureFlagProviderImpl() diff --git a/lib/shared/src/guardrails/client.ts b/lib/shared/src/guardrails/client.ts index faf79b8c3961..26d3ff0730fa 100644 --- a/lib/shared/src/guardrails/client.ts +++ b/lib/shared/src/guardrails/client.ts @@ -1,8 +1,7 @@ -import type { SourcegraphGraphQLAPIClient } from '../sourcegraph-api/graphql' -import { isError } from '../utils' - import type { Attribution, Guardrails } from '.' -import { ClientConfigSingleton } from '../sourcegraph-api/graphql/client' +import { currentResolvedConfig } from '../configuration/resolver' +import { ClientConfigSingleton, graphqlClient } from '../sourcegraph-api/graphql/client' +import { isError } from '../utils' // 10s timeout is enough to serve most attribution requests. // It's a better user experience for chat attribution to wait @@ -18,15 +17,6 @@ export interface GuardrailsClientConfig { } export class SourcegraphGuardrailsClient implements Guardrails { - constructor( - private client: SourcegraphGraphQLAPIClient, - private config: GuardrailsClientConfig - ) {} - - public onConfigurationChange(newConfig: GuardrailsClientConfig): void { - this.config = newConfig - } - public async searchAttribution(snippet: string): Promise { // Short-circuit attribution search if turned off in site config. const clientConfig = await ClientConfigSingleton.getInstance().getConfig() @@ -35,9 +25,10 @@ export class SourcegraphGuardrailsClient implements Guardrails { } const timeout = - (this.config.experimentalGuardrailsTimeoutSeconds ?? defaultTimeoutSeconds) * 1000 + ((await currentResolvedConfig()).configuration.experimentalGuardrailsTimeoutSeconds ?? + defaultTimeoutSeconds) * 1000 - const result = await this.client.searchAttribution(snippet, AbortSignal.timeout(timeout)) + const result = await graphqlClient.searchAttribution(snippet, AbortSignal.timeout(timeout)) if (isError(result)) { return result diff --git a/lib/shared/src/index.ts b/lib/shared/src/index.ts index 184536b7cd1a..589d4a6d347d 100644 --- a/lib/shared/src/index.ts +++ b/lib/shared/src/index.ts @@ -141,7 +141,7 @@ export { hydrateAfterPostMessage } from './editor/hydrateAfterPostMessage' export * from './editor/utils' export { FeatureFlag, - FeatureFlagProvider, + type FeatureFlagProvider, featureFlagProvider, } from './experimentation/FeatureFlagProvider' export { GuardrailsPost } from './guardrails' diff --git a/lib/shared/src/misc/observable.test.ts b/lib/shared/src/misc/observable.test.ts index 67446aaf9906..10f16e8863d4 100644 --- a/lib/shared/src/misc/observable.test.ts +++ b/lib/shared/src/misc/observable.test.ts @@ -6,6 +6,7 @@ import { type ObservableValue, allValuesFrom, combineLatest, + concat, createDisposables, distinctUntilChanged, firstValueFrom, @@ -529,3 +530,53 @@ describe('storeLastValue', () => { subscription.unsubscribe() }) }) + +describe('concat', () => { + afterEach(() => { + vi.useRealTimers() + }) + + test('concatenates observables in order', async () => { + const observable = concat( + observableOfSequence(1, 2), + observableOfSequence(3, 4), + observableOfSequence(5, 6) + ) + expect(await allValuesFrom(observable)).toStrictEqual([1, 2, 3, 4, 5, 6]) + }) + + test('handles empty observables', async () => { + const observable = concat( + observableOfSequence(), + observableOfSequence(1, 2), + observableOfSequence(), + observableOfSequence(3, 4) + ) + expect(await allValuesFrom(observable)).toStrictEqual([1, 2, 3, 4]) + }) + + test('propagates errors', async () => { + const error = new Error('Test error') + const observable = concat( + observableOfSequence(1, 2), + new Observable(observer => observer.error(error)), + observableOfSequence(3, 4) + ) + await expect(allValuesFrom(observable)).rejects.toThrow('Test error') + }) + + test('unsubscribes correctly', async () => { + vi.useFakeTimers() + const observable = concat( + observableOfTimedSequence(10, 'a', 10, 'b'), + observableOfTimedSequence(10, 'c', 10, 'd') + ) + const { values, done, unsubscribe } = readValuesFrom(observable) + + await vi.advanceTimersByTimeAsync(15) + unsubscribe() + await done + + expect(values).toStrictEqual(['a']) + }) +}) diff --git a/lib/shared/src/misc/observable.ts b/lib/shared/src/misc/observable.ts index 0d22ca7665bb..869739ad44fc 100644 --- a/lib/shared/src/misc/observable.ts +++ b/lib/shared/src/misc/observable.ts @@ -590,13 +590,52 @@ export function distinctUntilChanged( } } -export function tap(fn: (value: T) => void): (input: ObservableLike) => Observable { - return map(value => { - fn(value) - return value - }) -} +export function tap( + observerOrNext: Partial> | ((value: T) => void) +): (input: ObservableLike) => Observable { + return (input: ObservableLike) => + new Observable(observer => { + const tapObserver: Partial> = + typeof observerOrNext === 'function' ? { next: observerOrNext } : observerOrNext + const subscription = input.subscribe({ + next(value) { + if (tapObserver.next) { + try { + tapObserver.next(value) + } catch (err) { + observer.error(err) + return + } + } + observer.next(value) + }, + error(err) { + if (tapObserver.error) { + try { + tapObserver.error(err) + } catch (err) { + observer.error(err) + return + } + } + observer.error(err) + }, + complete() { + if (tapObserver.complete) { + try { + tapObserver.complete() + } catch (err) { + observer.error(err) + return + } + } + observer.complete() + }, + }) + return () => unsubscribe(subscription) + }) +} export function printDiff(): (input: ObservableLike) => Observable { let lastValue: T | typeof NO_VALUES_YET = NO_VALUES_YET return map(value => { @@ -734,17 +773,170 @@ export function mergeMap( } } +export interface StoredLastValue { + value: { last: undefined; isSet: false } | { last: T; isSet: true } + subscription: Unsubscribable +} + /** * Store the last value emitted by an {@link Observable} so that it can be accessed synchronously. * Callers must take care to not create a race condition when using this function. */ -export function storeLastValue(observable: Observable): { - value: { last: undefined; isSet: false } | { last: T; isSet: true } - subscription: Unsubscribable -} { +export function storeLastValue(observable: Observable): StoredLastValue { const value: ReturnType['value'] = { last: undefined, isSet: false } const subscription = observable.subscribe(v => { Object.assign(value, { last: v, isSet: true }) }) return { value, subscription } } + +export function debounceTime(duration: number): (source: ObservableLike) => Observable { + return (source: ObservableLike) => + new Observable(observer => { + let timeoutId: ReturnType | null = null + let latestValue: T | null = null + let hasValue = false + + const subscription = source.subscribe({ + next: value => { + latestValue = value + hasValue = true + + if (timeoutId === null) { + timeoutId = setTimeout(() => { + if (hasValue) { + observer.next(latestValue!) + hasValue = false + } + timeoutId = null + }, duration) + } + }, + error: err => observer.error(err), + complete: () => { + if (timeoutId !== null) { + clearTimeout(timeoutId) + } + if (hasValue) { + observer.next(latestValue!) + } + observer.complete() + }, + }) + + return () => { + unsubscribe(subscription) + if (timeoutId !== null) { + clearTimeout(timeoutId) + } + } + }) +} + +export type ObservableInputTuple = { + [K in keyof T]: ObservableLike +} + +export function concat( + ...inputs: [...ObservableInputTuple] +): Observable { + return new Observable(observer => { + let currentIndex = 0 + let currentSubscription: UnsubscribableLike = null + + function subscribeToNext() { + if (currentIndex >= inputs.length) { + observer.complete() + return + } + + const input = inputs[currentIndex] + currentSubscription = input.subscribe({ + next: value => observer.next(value), + error: err => observer.error(err), + complete: () => { + currentIndex++ + subscribeToNext() + }, + }) + } + + subscribeToNext() + + return () => { + if (currentSubscription) { + unsubscribe(currentSubscription) + } + } + }) +} + +export function concatMap( + project: (value: T, index: number) => ObservableLike +): (source: ObservableLike) => Observable { + return (source: ObservableLike) => + new Observable(observer => { + let index = 0 + let isOuterCompleted = false + let innerSubscription: UnsubscribableLike | null = null + const outerSubscription = source.subscribe({ + next(value) { + try { + const innerObservable = project(value, index++) + if (innerSubscription) { + unsubscribe(innerSubscription) + } + innerSubscription = innerObservable.subscribe({ + next(innerValue) { + observer.next(innerValue) + }, + error(err) { + observer.error(err) + }, + complete() { + innerSubscription = null + if (isOuterCompleted && !innerSubscription) { + observer.complete() + } + }, + }) + } catch (err) { + observer.error(err) + } + }, + error(err) { + observer.error(err) + }, + complete() { + isOuterCompleted = true + if (!innerSubscription) { + observer.complete() + } + }, + }) + + return () => { + unsubscribe(outerSubscription) + if (innerSubscription) { + unsubscribe(innerSubscription) + } + } + }) +} + +export function lifecycle({ + onSubscribe, + onUnsubscribe, +}: { onSubscribe?: () => void; onUnsubscribe?: () => void }): ( + source: ObservableLike +) => Observable { + return (source: ObservableLike) => + new Observable(observer => { + onSubscribe?.() + const subscription = source.subscribe(observer) + return () => { + unsubscribe(subscription) + onUnsubscribe?.() + } + }) +} diff --git a/lib/shared/src/sourcegraph-api/graphql/client.ts b/lib/shared/src/sourcegraph-api/graphql/client.ts index f9d17d6171d0..ef2186bc3045 100644 --- a/lib/shared/src/sourcegraph-api/graphql/client.ts +++ b/lib/shared/src/sourcegraph-api/graphql/client.ts @@ -1394,10 +1394,13 @@ export class SourcegraphGraphQLAPIClient { ).then(response => extractDataOrError(response, data => data.snippetAttribution)) } - public async getEvaluatedFeatureFlags(): Promise | Error> { + public async getEvaluatedFeatureFlags( + signal?: AbortSignal + ): Promise | Error> { return this.fetchSourcegraphAPI>( GET_FEATURE_FLAGS_QUERY, - {} + {}, + signal ).then(response => { return extractDataOrError(response, data => data.evaluatedFeatureFlags.reduce((acc: Record, { name, value }) => { @@ -1408,12 +1411,16 @@ export class SourcegraphGraphQLAPIClient { }) } - public async evaluateFeatureFlag(flagName: string): Promise { + public async evaluateFeatureFlag( + flagName: string, + signal?: AbortSignal + ): Promise { return this.fetchSourcegraphAPI>( EVALUATE_FEATURE_FLAG_QUERY, { flagName, - } + }, + signal ).then(response => extractDataOrError(response, data => data.evaluateFeatureFlag)) } diff --git a/vscode/src/chat/clientStateBroadcaster.ts b/vscode/src/chat/clientStateBroadcaster.ts index 5c37fe55b981..ef0119332361 100644 --- a/vscode/src/chat/clientStateBroadcaster.ts +++ b/vscode/src/chat/clientStateBroadcaster.ts @@ -7,6 +7,7 @@ import { authStatus, combineLatest, contextFiltersProvider, + currentResolvedConfig, displayLineRange, displayPathBasename, expandToLineRange, @@ -134,6 +135,7 @@ export async function getCorpusContextItemsForEditorState(useRemote: boolean): P // remote search). There should be a single internal thing in Cody that lets you monitor the // user's current codebase. if (useRemote && workspaceReposMonitor) { + const { auth } = await currentResolvedConfig() const repoMetadata = await workspaceReposMonitor.getRepoMetadata() for (const repo of repoMetadata) { if (await contextFiltersProvider.isRepoNameIgnored(repo.repoName)) { @@ -150,7 +152,8 @@ export async function getCorpusContextItemsForEditorState(useRemote: boolean): P name: repo.repoName, url: repo.repoName, }, - REMOTE_REPOSITORY_PROVIDER_URI + REMOTE_REPOSITORY_PROVIDER_URI, + auth ) ), title: 'Current Repository', diff --git a/vscode/src/completions/get-inline-completions-tests/helpers.ts b/vscode/src/completions/get-inline-completions-tests/helpers.ts index 8a9c4565116b..353ba9d1d321 100644 --- a/vscode/src/completions/get-inline-completions-tests/helpers.ts +++ b/vscode/src/completions/get-inline-completions-tests/helpers.ts @@ -425,6 +425,7 @@ export function initCompletionProviderConfig({ }: Partial>): Promise { graphqlClient.setConfig({} as unknown as GraphQLAPIClientConfig) vi.spyOn(featureFlagProvider, 'evaluateFeatureFlag').mockResolvedValue(false) + vi.spyOn(featureFlagProvider, 'evaluatedFeatureFlag').mockReturnValue(Observable.of(false)) mockAuthStatus(authStatus) return completionProviderConfig.init((configuration ?? {}) as ClientConfiguration) } diff --git a/vscode/src/completions/inline-completion-item-provider.test.ts b/vscode/src/completions/inline-completion-item-provider.test.ts index 44fdc460b1d4..5385730d4083 100644 --- a/vscode/src/completions/inline-completion-item-provider.test.ts +++ b/vscode/src/completions/inline-completion-item-provider.test.ts @@ -1,13 +1,11 @@ import dedent from 'dedent' -import { afterEach, beforeAll, beforeEach, describe, expect, it, vi } from 'vitest' +import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest' import * as vscode from 'vscode' import { AUTH_STATUS_FIXTURE_AUTHED, - type GraphQLAPIClientConfig, RateLimitError, contextFiltersProvider, - graphqlClient, } from '@sourcegraph/cody-shared' import { telemetryRecorder } from '@sourcegraph/cody-shared' @@ -33,8 +31,6 @@ const DUMMY_CONTEXT: vscode.InlineCompletionContext = { triggerKind: vscode.InlineCompletionTriggerKind.Automatic, } -graphqlClient.setConfig({} as unknown as GraphQLAPIClientConfig) - class MockableInlineCompletionItemProvider extends InlineCompletionItemProvider { constructor( mockGetInlineCompletions: typeof getInlineCompletions, @@ -61,13 +57,10 @@ class MockableInlineCompletionItemProvider extends InlineCompletionItemProvider public declare lastCandidate } -describe('InlineCompletionItemProvider', () => { - beforeAll(async () => { +describe('InlineCompletionItemProvider', async () => { + beforeEach(async () => { await initCompletionProviderConfig({}) mockLocalStorage() - }) - - beforeEach(() => { vi.spyOn(contextFiltersProvider, 'isUriIgnored').mockResolvedValue(false) CompletionLogger.reset_testOnly() }) diff --git a/vscode/src/completions/providers/create-provider.test.ts b/vscode/src/completions/providers/create-provider.test.ts index e7acd2155e30..17c0c9fd5bec 100644 --- a/vscode/src/completions/providers/create-provider.test.ts +++ b/vscode/src/completions/providers/create-provider.test.ts @@ -2,24 +2,24 @@ import { AUTH_STATUS_FIXTURE_AUTHED_DOTCOM, type ClientConfiguration, type CodyLLMSiteConfiguration, - type GraphQLAPIClientConfig, - graphqlClient, + featureFlagProvider, mockAuthStatus, toFirstValueGetter, } from '@sourcegraph/cody-shared' -import { beforeAll, describe, expect, it } from 'vitest' +import { beforeAll, describe, expect, it, vi } from 'vitest' import { mockLocalStorage } from '../../services/LocalStorageProvider' import { getVSCodeConfigurationWithAccessToken } from '../../testutils/mocks' +import { Observable } from 'observable-fns' import { createProvider } from './create-provider' -graphqlClient.setConfig({} as unknown as GraphQLAPIClientConfig) const createProviderFirstValue = toFirstValueGetter(createProvider) describe('createProvider', () => { beforeAll(async () => { mockAuthStatus() mockLocalStorage() + vi.spyOn(featureFlagProvider, 'evaluatedFeatureFlag').mockReturnValue(Observable.of(false)) }) describe('local settings', () => { diff --git a/vscode/src/context/openctx/common/get-repository-mentions.ts b/vscode/src/context/openctx/common/get-repository-mentions.ts index 1d76da598e4d..1a08f70c8652 100644 --- a/vscode/src/context/openctx/common/get-repository-mentions.ts +++ b/vscode/src/context/openctx/common/get-repository-mentions.ts @@ -1,7 +1,9 @@ import type { Mention } from '@openctx/client' import { + type AuthCredentials, type SuggestionsRepo, contextFiltersProvider, + currentResolvedConfig, graphqlClient, isError, } from '@sourcegraph/cody-shared' @@ -33,6 +35,7 @@ export async function getRepositoryMentions( query: string, providerId: string ): Promise { + const { auth } = await currentResolvedConfig() const dataOrError = await graphqlClient.searchRepoSuggestions(query) if (isError(dataOrError) || dataOrError === null || dataOrError.search === null) { @@ -50,7 +53,8 @@ export async function getRepositoryMentions( ...repository.item, current: !!localRepos.find(({ repoName }) => repoName === repository.item.name), }, - providerId + providerId, + auth ) ) ) @@ -60,12 +64,13 @@ type MinimalRepoMention = Pick & { curre export async function createRepositoryMention( repo: MinimalRepoMention, - providerId: string + providerId: string, + { serverEndpoint }: Pick ): Promise { return { title: repo.name, providerUri: providerId, - uri: graphqlClient.endpoint + repo.url, + uri: serverEndpoint + repo.url, // By default, we show <uri> in the mentions' menu. // As repo.url and repo.name are almost same, we do not want to show the uri. diff --git a/vscode/src/context/openctx/remoteDirectorySearch.ts b/vscode/src/context/openctx/remoteDirectorySearch.ts index f0afb4c588ba..3fd17c1e694b 100644 --- a/vscode/src/context/openctx/remoteDirectorySearch.ts +++ b/vscode/src/context/openctx/remoteDirectorySearch.ts @@ -1,4 +1,4 @@ -import { REMOTE_DIRECTORY_PROVIDER_URI } from '@sourcegraph/cody-shared' +import { REMOTE_DIRECTORY_PROVIDER_URI, currentResolvedConfig } from '@sourcegraph/cody-shared' import type { Item, Mention } from '@openctx/client' import { graphqlClient, isDefined, isError } from '@sourcegraph/cody-shared' @@ -52,6 +52,9 @@ export async function getDirectoryMentions( const directoryRe = directoryPath ? escapeRegExp(directoryPath) : '' const query = `repo:${repoRe} file:${directoryRe}.*\/.* select:file.directory count:10` + const { + auth: { serverEndpoint }, + } = await currentResolvedConfig() const dataOrError = await graphqlClient.searchFileMatches(query) if (isError(dataOrError) || dataOrError === null) { @@ -64,7 +67,7 @@ export async function getDirectoryMentions( return null } - const url = `${graphqlClient.endpoint.replace(/\/$/, '')}${result.file.url}` + const url = `${serverEndpoint.replace(/\/$/, '')}${result.file.url}` return { uri: url, diff --git a/vscode/src/context/openctx/remoteFileSearch.ts b/vscode/src/context/openctx/remoteFileSearch.ts index f5ece8547c79..18b9fbe73d11 100644 --- a/vscode/src/context/openctx/remoteFileSearch.ts +++ b/vscode/src/context/openctx/remoteFileSearch.ts @@ -1,6 +1,7 @@ import type { Item, Mention } from '@openctx/client' import { REMOTE_FILE_PROVIDER_URI, + currentResolvedConfig, displayPathBasename, graphqlClient, isDefined, @@ -53,6 +54,7 @@ async function getFileMentions(repoName: string, filePath?: string): Promise<Men const fileRe = filePath ? escapeRegExp(filePath) : '^.*$' const query = `repo:${repoRe} file:${fileRe} type:file count:10` + const { auth } = await currentResolvedConfig() const dataOrError = await graphqlClient.searchFileMatches(query) if (isError(dataOrError) || dataOrError === null) { @@ -65,7 +67,7 @@ async function getFileMentions(repoName: string, filePath?: string): Promise<Men return null } - const url = `${graphqlClient.endpoint.replace(/\/$/, '')}${result.file.url}` + const url = `${auth.serverEndpoint.replace(/\/$/, '')}${result.file.url}` const basename = displayPathBasename(URI.parse(result.file.path)) @@ -84,6 +86,7 @@ async function getFileMentions(repoName: string, filePath?: string): Promise<Men } async function getFileItem(repoName: string, filePath: string, rev = 'HEAD'): Promise<Item[]> { + const { auth } = await currentResolvedConfig() const dataOrError = await graphqlClient.getFileContents(repoName, filePath, rev) if (isError(dataOrError)) { @@ -95,7 +98,7 @@ async function getFileItem(repoName: string, filePath: string, rev = 'HEAD'): Pr return [] } - const url = `${graphqlClient.endpoint.replace(/\/$/, '')}${file.url}` + const url = `${auth.serverEndpoint.replace(/\/$/, '')}${file.url}` return [ { diff --git a/vscode/src/context/openctx/remoteRepositorySearch.ts b/vscode/src/context/openctx/remoteRepositorySearch.ts index a4b704738b58..8da3fdd2536d 100644 --- a/vscode/src/context/openctx/remoteRepositorySearch.ts +++ b/vscode/src/context/openctx/remoteRepositorySearch.ts @@ -1,5 +1,10 @@ import type { Item } from '@openctx/client' -import { REMOTE_REPOSITORY_PROVIDER_URI, graphqlClient, isError } from '@sourcegraph/cody-shared' +import { + REMOTE_REPOSITORY_PROVIDER_URI, + currentResolvedConfig, + graphqlClient, + isError, +} from '@sourcegraph/cody-shared' import { getRepositoryMentions } from './common/get-repository-mentions' import type { OpenCtxProvider } from './types' @@ -27,6 +32,7 @@ export function createRemoteRepositoryProvider(customTitle?: string): OpenCtxPro return [] } + const { auth } = await currentResolvedConfig() const dataOrError = await graphqlClient.contextSearch({ repoIDs: [mention?.data?.repoId as string], query: message, @@ -38,7 +44,7 @@ export function createRemoteRepositoryProvider(customTitle?: string): OpenCtxPro return dataOrError.map( node => ({ - url: graphqlClient.endpoint + node.uri.toString(), + url: auth.serverEndpoint + node.uri.toString(), title: node.path, ai: { content: node.content, diff --git a/vscode/src/editor/utils/editor-context.ts b/vscode/src/editor/utils/editor-context.ts index f95d016a9604..ac1caa04533f 100644 --- a/vscode/src/editor/utils/editor-context.ts +++ b/vscode/src/editor/utils/editor-context.ts @@ -15,6 +15,7 @@ import { type SymbolKind, TokenCounterUtils, contextFiltersProvider, + currentResolvedConfig, displayPath, graphqlClient, isAbortError, @@ -472,13 +473,14 @@ async function resolveFileOrSymbolContextItem( ? { startLine: contextItem.range.start.line, endLine: contextItem.range.end.line + 1 } : undefined + const { auth } = await currentResolvedConfig() const resultOrError = await graphqlClient.getFileContent(repository, path, ranges, signal) if (!isErrorLike(resultOrError)) { return { ...contextItem, title: path, - uri: URI.parse(`${graphqlClient.endpoint}${repository}/-/blob${path}`), + uri: URI.parse(`${auth.serverEndpoint}${repository}/-/blob${path}`), content: resultOrError, repoName: repository, source: ContextItemSource.Unified, diff --git a/vscode/src/extension.common.ts b/vscode/src/extension.common.ts index 80f67da2c22b..bfcad55c8855 100644 --- a/vscode/src/extension.common.ts +++ b/vscode/src/extension.common.ts @@ -2,7 +2,6 @@ import * as vscode from 'vscode' import type { ClientConfiguration, - ClientConfigurationWithEndpoint, CompletionLogger, CompletionsClientConfig, SourcegraphCompletionsClient, @@ -21,10 +20,7 @@ import type { ExtensionClient } from './extension-client' import type { LocalEmbeddingsConfig, LocalEmbeddingsController } from './local-context/local-embeddings' import type { SymfRunner } from './local-context/symf' import { start } from './main' -import type { - OpenTelemetryService, - OpenTelemetryServiceConfig, -} from './services/open-telemetry/OpenTelemetryService.node' +import type { OpenTelemetryService } from './services/open-telemetry/OpenTelemetryService.node' import { type SentryService, captureException } from './services/sentry/sentry' type Constructor<T extends new (...args: any) => any> = T extends new ( @@ -46,10 +42,8 @@ export interface PlatformContext { config: CompletionsClientConfig, logger?: CompletionLogger ) => SourcegraphCompletionsClient - createSentryService?: ( - config: Pick<ClientConfigurationWithEndpoint, 'serverEndpoint'> - ) => SentryService - createOpenTelemetryService?: (config: OpenTelemetryServiceConfig) => OpenTelemetryService + createSentryService?: () => SentryService + createOpenTelemetryService?: () => OpenTelemetryService startTokenReceiver?: typeof startTokenReceiver onConfigurationChange?: (configuration: ClientConfiguration) => void extensionClient: ExtensionClient diff --git a/vscode/src/external-services.ts b/vscode/src/external-services.ts index 033dfbe62d4d..cfbe264dbb21 100644 --- a/vscode/src/external-services.ts +++ b/vscode/src/external-services.ts @@ -59,8 +59,8 @@ export async function configureExternalServices( > ): Promise<ExternalServices> { const initialConfig = await firstValueFrom(resolvedConfigWithAccessToken) - const sentryService = platform.createSentryService?.(initialConfig) - const openTelemetryService = platform.createOpenTelemetryService?.(initialConfig) + platform.createSentryService?.() + platform.createOpenTelemetryService?.() const completionsClient = platform.createCompletionsClient(initialConfig, logger) const symfRunner = platform.createSymfRunner?.(context, completionsClient) @@ -80,7 +80,7 @@ export async function configureExternalServices( const chatClient = new ChatClient(completionsClient, () => currentAuthStatusAuthed()) - const guardrails = new SourcegraphGuardrailsClient(graphqlClient, initialConfig) + const guardrails = new SourcegraphGuardrailsClient() const contextAPIClient = new ContextAPIClient(graphqlClient) @@ -92,10 +92,7 @@ export async function configureExternalServices( symfRunner, contextAPIClient, onConfigurationChange: newConfig => { - sentryService?.onConfigurationChange(newConfig) - openTelemetryService?.onConfigurationChange(newConfig) completionsClient.onConfigurationChange(newConfig) - guardrails.onConfigurationChange(newConfig) void localEmbeddings?.setAccessToken(newConfig.serverEndpoint, newConfig.accessToken) }, } diff --git a/vscode/src/local-context/symf.ts b/vscode/src/local-context/symf.ts index fb923b654be1..91cabb265763 100644 --- a/vscode/src/local-context/symf.ts +++ b/vscode/src/local-context/symf.ts @@ -363,7 +363,7 @@ export class SymfRunner implements vscode.Disposable { } await rename(indexDir.fsPath, trashDir.fsPath) - void rm(trashDir.fsPath, { recursive: true, force: true }) // delete in background + rm(trashDir.fsPath, { recursive: true, force: true }).catch(() => {}) // delete in background } private async unsafeIndexExists(scopeDir: FileURI): Promise<boolean> { diff --git a/vscode/src/main.ts b/vscode/src/main.ts index e1af0651ddc0..b3474ce7c7d9 100644 --- a/vscode/src/main.ts +++ b/vscode/src/main.ts @@ -7,14 +7,12 @@ import { type DefaultCodyCommands, type Guardrails, PromptString, - type ResolvedConfiguration, authStatus, combineLatest, contextFiltersProvider, currentAuthStatus, currentAuthStatusOrNotReadyYet, distinctUntilChanged, - featureFlagProvider, firstValueFrom, fromVSCodeEvent, graphqlClient, @@ -153,13 +151,7 @@ export async function start( ) ) - disposables.push( - subscriptionDisposable( - resolvedConfig.subscribe({ - next: config => configureEventsInfra(config, isExtensionModeDevOrTest), - }) - ) - ) + disposables.push(createOrUpdateTelemetryRecorderProvider(isExtensionModeDevOrTest)) disposables.push( subscriptionDisposable( @@ -313,8 +305,6 @@ async function initializeSingletons( next: config => { void localStorage.setConfig(config) graphqlClient.setConfig(config) - void featureFlagProvider.refresh() - upstreamHealthProvider.instance!.onConfigurationChange(config) defaultCodeCompletionsClient.instance!.onConfigurationChange(config) }, }) @@ -794,13 +784,3 @@ function registerChat( return { chatsController } } - -/** - * Create or update events infrastructure, using the new telemetryRecorder. - */ -async function configureEventsInfra( - config: ResolvedConfiguration, - isExtensionModeDevOrTest: boolean -): Promise<void> { - await createOrUpdateTelemetryRecorderProvider(config, isExtensionModeDevOrTest) -} diff --git a/vscode/src/models/sync.test.ts b/vscode/src/models/sync.test.ts index 42ee903666ed..a94b9f0d28b7 100644 --- a/vscode/src/models/sync.test.ts +++ b/vscode/src/models/sync.test.ts @@ -11,6 +11,7 @@ import { RestClient, type ServerModel, type ServerModelConfiguration, + featureFlagProvider, getDotComDefaultModels, graphqlClient, mockAuthStatus, @@ -32,6 +33,8 @@ describe('syncModels', () => { setModelsSpy.mockClear() mockAuthStatus(AUTH_STATUS_FIXTURE_AUTHED) + vi.spyOn(featureFlagProvider, 'evaluateFeatureFlag').mockResolvedValue(false) + // Mock the /.api/client-config for these tests so that modelsAPIEnabled == false vi.spyOn(ClientConfigSingleton.prototype, 'getConfig').mockResolvedValue({ chatEnabled: true, diff --git a/vscode/src/notifications/cody-pro-expiration.test.ts b/vscode/src/notifications/cody-pro-expiration.test.ts index 0d3845d683d4..88e22ef07fbc 100644 --- a/vscode/src/notifications/cody-pro-expiration.test.ts +++ b/vscode/src/notifications/cody-pro-expiration.test.ts @@ -6,8 +6,6 @@ import { type AuthStatus, DOTCOM_URL, FeatureFlag, - type GraphQLAPIClientConfig, - type SourcegraphGraphQLAPIClient, authStatus, featureFlagProvider, graphqlClient, @@ -20,7 +18,6 @@ vi.mock('../services/AuthProvider') describe('Cody Pro expiration notifications', () => { let notifier: CodyProExpirationNotifications - let apiClient: SourcegraphGraphQLAPIClient let authStatus_: AuthStatus let authChangeListener = () => {} let codyPlan: string @@ -51,14 +48,13 @@ describe('Cody Pro expiration notifications', () => { vi.spyOn(featureFlagProvider, 'evaluateFeatureFlag').mockImplementation((flag: FeatureFlag) => Promise.resolve(enabledFeatureFlags.has(flag)) ) - vi.spyOn(featureFlagProvider, 'refresh').mockImplementation(() => Promise.resolve()) - graphqlClient.setConfig({} as unknown as GraphQLAPIClientConfig) - apiClient = { - getCurrentUserCodySubscription: () => ({ - status: codyStatus, - plan: codyPlan, - }), - } as unknown as SourcegraphGraphQLAPIClient + vi.spyOn(graphqlClient, 'getCurrentUserCodySubscription').mockImplementation(async () => ({ + status: codyStatus, + plan: codyPlan, + applyProRateLimits: false, + currentPeriodEndAt: new Date(2022, 1, 1), + currentPeriodStartAt: new Date(2021, 1, 1), + })) vi.spyOn(authStatus, 'subscribe').mockImplementation((f: any): any => { authChangeListener = f // (return an object that simulates the unsubscribe @@ -80,7 +76,7 @@ describe('Cody Pro expiration notifications', () => { function createNotifier() { return new CodyProExpirationNotifications( - apiClient, + graphqlClient, showInformationMessage, openExternal, 10, diff --git a/vscode/src/notifications/setup-notification.ts b/vscode/src/notifications/setup-notification.ts index 91d720c82c81..0c0812aea15a 100644 --- a/vscode/src/notifications/setup-notification.ts +++ b/vscode/src/notifications/setup-notification.ts @@ -1,16 +1,14 @@ import * as vscode from 'vscode' -import type { ClientConfigurationWithAccessToken } from '@sourcegraph/cody-shared' +import type { AuthCredentials } from '@sourcegraph/cody-shared' import { localStorage } from '../services/LocalStorageProvider' import { telemetryRecorder } from '@sourcegraph/cody-shared' import { showActionNotification } from '.' -export const showSetupNotification = async ( - config: ClientConfigurationWithAccessToken -): Promise<void> => { - if (config.serverEndpoint && config.accessToken) { +export const showSetupNotification = async (auth: AuthCredentials): Promise<void> => { + if (auth.serverEndpoint && auth.accessToken) { // User has already attempted to configure Cody. // Regardless of if they are authenticated or not, we don't want to prompt them. return diff --git a/vscode/src/services/AuthProvider.ts b/vscode/src/services/AuthProvider.ts index a171b548b1f8..3e8e579750ba 100644 --- a/vscode/src/services/AuthProvider.ts +++ b/vscode/src/services/AuthProvider.ts @@ -227,6 +227,8 @@ export class AuthProvider implements vscode.Disposable { private async updateAuthStatus(authStatus: AuthStatus): Promise<void> { try { + this.status.next(authStatus) + // We update the graphqlClient and ModelsService first // because many listeners rely on these graphqlClient.setConfig(await getFullConfig()) @@ -234,7 +236,6 @@ export class AuthProvider implements vscode.Disposable { } catch (error) { logDebug('AuthProvider', 'updateAuthStatus error', error) } finally { - this.status.next(authStatus) let eventValue: 'disconnected' | 'connected' | 'failed' if (authStatus.authenticated) { eventValue = 'connected' diff --git a/vscode/src/services/UpstreamHealthProvider.ts b/vscode/src/services/UpstreamHealthProvider.ts index 4ddc79ff8338..bfc8ba04e290 100644 --- a/vscode/src/services/UpstreamHealthProvider.ts +++ b/vscode/src/services/UpstreamHealthProvider.ts @@ -1,12 +1,15 @@ import { type BrowserOrNodeResponse, - type ClientConfigurationWithAccessToken, addCustomUserAgent, addTraceparent, + currentResolvedConfig, + distinctUntilChanged, isDotCom, logDebug, + resolvedConfig, setSingleton, singletonNotYetSet, + subscriptionDisposable, wrapInActiveSpan, } from '@sourcegraph/cody-shared' import { fetch } from '@sourcegraph/cody-shared' @@ -23,20 +26,25 @@ const INITIAL_PING_DELAY_MS = 10 * 1000 // 10 seconds * * You can query it to get aggregates of the most recent pings. */ -class UpstreamHealthProvider implements vscode.Disposable { +export class UpstreamHealthProvider implements vscode.Disposable { private lastUpstreamLatency?: number private lastGatewayLatency?: number private disposables: vscode.Disposable[] = [] - private config: Pick< - ClientConfigurationWithAccessToken, - 'serverEndpoint' | 'customHeaders' | 'accessToken' - > | null = null private nextTimeoutId: NodeJS.Timeout | null = null constructor() { + // Refresh when auth (endpoint or token) changes. this.disposables.push( + subscriptionDisposable( + resolvedConfig.pipe(distinctUntilChanged()).subscribe(() => { + this.lastUpstreamLatency = undefined + this.lastGatewayLatency = undefined + + this.enqueue(INITIAL_PING_DELAY_MS) + }) + ), vscode.window.onDidChangeWindowState(state => { if (state.focused && this.lastMeasurementSkippedBecauseNotFocused) { this.lastMeasurementSkippedBecauseNotFocused = false @@ -47,35 +55,13 @@ class UpstreamHealthProvider implements vscode.Disposable { } public getUpstreamLatency(): number | undefined { - if (!this.config) { - return undefined - } return this.lastUpstreamLatency } public getGatewayLatency(): number | undefined { - if (!this.config) { - return undefined - } return this.lastGatewayLatency } - public onConfigurationChange( - newConfig: Pick< - ClientConfigurationWithAccessToken, - 'serverEndpoint' | 'customHeaders' | 'accessToken' - > - ) { - this.config = newConfig - this.lastUpstreamLatency = undefined - this.lastGatewayLatency = undefined - - // Enqueue the initial ping after a config change in 10 seconds. This - // avoids running the test while the extension is still initializing and - // competing with many other network requests. - this.enqueue(INITIAL_PING_DELAY_MS) - } - private enqueue(delay: number): void { if (this.nextTimeoutId) { clearTimeout(this.nextTimeoutId) @@ -90,41 +76,38 @@ class UpstreamHealthProvider implements vscode.Disposable { clearTimeout(this.nextTimeoutId) } + if (!vscode.window.state.focused) { + // Skip if the window is not focused, and try again when the window becomes focused + // again. Some users have OS firewalls that make periodic background network access + // annoying for users, and this eliminates that annoyance. See + // https://linear.app/sourcegraph/issue/CODY-3745/codys-background-periodic-network-access-causes-2fa. + this.lastMeasurementSkippedBecauseNotFocused = true + return + } + try { if (process.env.DISABLE_UPSTREAM_HEALTH_PINGS === 'true') { return } - if (!vscode.window.state.focused) { - // Skip if the window is not focused, and try again when the window becomes focused - // again. Some users have OS firewalls that make periodic background network access - // annoying for users, and this eliminates that annoyance. See - // https://linear.app/sourcegraph/issue/CODY-3745/codys-background-periodic-network-access-causes-2fa. - this.lastMeasurementSkippedBecauseNotFocused = true - return - } - - if (!this.config) { - throw new Error('UpstreamHealthProvider not initialized') - } - - const sharedHeaders = new Headers(this.config.customHeaders as HeadersInit) + const { auth, configuration } = await currentResolvedConfig() + const sharedHeaders = new Headers(configuration.customHeaders as HeadersInit | undefined) sharedHeaders.set('Content-Type', 'application/json; charset=utf-8') addTraceparent(sharedHeaders) addCustomUserAgent(sharedHeaders) const upstreamHeaders = new Headers(sharedHeaders) - if (this.config.accessToken) { - upstreamHeaders.set('Authorization', `token ${this.config.accessToken}`) + if (auth.accessToken) { + upstreamHeaders.set('Authorization', `token ${auth.accessToken}`) } - const url = new URL('/healthz', this.config.serverEndpoint) + const url = new URL('/healthz', auth.serverEndpoint) const upstreamResult = await wrapInActiveSpan('upstream-latency.upstream', span => { span.setAttribute('sampled', true) return measureLatencyToUri(upstreamHeaders, url.toString()) }) // We don't want to congest the network so we run the test serially - if (isDotCom(this.config.serverEndpoint)) { + if (isDotCom(auth.serverEndpoint)) { const gatewayHeaders = new Headers(sharedHeaders) const uri = 'https://cody-gateway.sourcegraph.com/-/__version' const gatewayResult = await wrapInActiveSpan('upstream-latency.gateway', span => { diff --git a/vscode/src/services/open-telemetry/OpenTelemetryService.node.ts b/vscode/src/services/open-telemetry/OpenTelemetryService.node.ts index 4c2a392a763d..fd25148d6c6a 100644 --- a/vscode/src/services/open-telemetry/OpenTelemetryService.node.ts +++ b/vscode/src/services/open-telemetry/OpenTelemetryService.node.ts @@ -5,9 +5,11 @@ import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node' import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions' import { - type ClientConfigurationWithAccessToken, FeatureFlag, + type ResolvedConfiguration, + type Unsubscribable, featureFlagProvider, + resolvedConfig, } from '@sourcegraph/cody-shared' import { DiagConsoleLogger, DiagLogLevel, diag } from '@opentelemetry/api' @@ -16,11 +18,6 @@ import { version } from '../../version' import { CodyTraceExporter } from './CodyTraceExport' import { ConsoleBatchSpanExporter } from './console-batch-span-exporter' -export type OpenTelemetryServiceConfig = Pick< - ClientConfigurationWithAccessToken, - 'serverEndpoint' | 'experimentalTracing' | 'debugVerbose' | 'accessToken' -> - export class OpenTelemetryService { private tracerProvider?: NodeTracerProvider private unloadInstrumentations?: () => void @@ -31,38 +28,42 @@ export class OpenTelemetryService { // be run in parallel private reconfigurePromiseMutex: Promise<void> = Promise.resolve() - constructor(protected config: OpenTelemetryServiceConfig) { - this.reconfigurePromiseMutex = this.reconfigurePromiseMutex.then(() => this.reconfigure()) - } - - public onConfigurationChange(newConfig: OpenTelemetryServiceConfig): void { - this.config = newConfig - this.reconfigurePromiseMutex = this.reconfigurePromiseMutex.then(() => this.reconfigure()) - } + private configSubscription: Unsubscribable - private async reconfigure(): Promise<void> { - this.isTracingEnabled = - this.config.experimentalTracing || - (await featureFlagProvider.evaluateFeatureFlag(FeatureFlag.CodyAutocompleteTracing)) + constructor() { + this.configSubscription = resolvedConfig.subscribe(({ configuration, auth }) => { + this.reconfigurePromiseMutex = this.reconfigurePromiseMutex.then(async () => { + this.isTracingEnabled = + configuration.experimentalTracing || + (await featureFlagProvider.evaluateFeatureFlag(FeatureFlag.CodyAutocompleteTracing)) - const traceUrl = new URL('/-/debug/otlp/v1/traces', this.config.serverEndpoint).toString() - if (this.lastTraceUrl === traceUrl) { - return - } - this.lastTraceUrl = traceUrl + const traceUrl = new URL('/-/debug/otlp/v1/traces', auth.serverEndpoint).toString() + if (this.lastTraceUrl === traceUrl) { + return + } + this.lastTraceUrl = traceUrl - const logLevel = this.config.debugVerbose ? DiagLogLevel.INFO : DiagLogLevel.ERROR - diag.setLogger(new DiagConsoleLogger(), logLevel) + const logLevel = configuration.debugVerbose ? DiagLogLevel.INFO : DiagLogLevel.ERROR + diag.setLogger(new DiagConsoleLogger(), logLevel) - await this.reset() + await this.reset() - this.unloadInstrumentations = registerInstrumentations({ - instrumentations: [new HttpInstrumentation()], + this.unloadInstrumentations = registerInstrumentations({ + instrumentations: [new HttpInstrumentation()], + }) + this.configureTracerProvider(traceUrl, { configuration, auth }) + }) }) - this.configureTracerProvider(traceUrl) } - public configureTracerProvider(traceUrl: string): void { + public dispose(): void { + this.configSubscription.unsubscribe() + } + + private configureTracerProvider( + traceUrl: string, + { configuration, auth }: Pick<ResolvedConfiguration, 'configuration' | 'auth'> + ): void { this.tracerProvider = new NodeTracerProvider({ resource: new Resource({ [SemanticResourceAttributes.SERVICE_NAME]: 'cody-client', @@ -76,13 +77,13 @@ export class OpenTelemetryService { new CodyTraceExporter({ traceUrl, isTracingEnabled: this.isTracingEnabled, - accessToken: this.config.accessToken, + accessToken: auth.accessToken, }) ) ) // Add the console exporter used in development for verbose logging and debugging. - if (process.env.NODE_ENV === 'development' || this.config.debugVerbose) { + if (process.env.NODE_ENV === 'development' || configuration.debugVerbose) { this.tracerProvider.addSpanProcessor(new BatchSpanProcessor(new ConsoleBatchSpanExporter())) } diff --git a/vscode/src/services/open-telemetry/utils.ts b/vscode/src/services/open-telemetry/utils.ts index d0bf9d8c6bc7..630d98085f3d 100644 --- a/vscode/src/services/open-telemetry/utils.ts +++ b/vscode/src/services/open-telemetry/utils.ts @@ -1,5 +1,5 @@ import type { Span } from '@opentelemetry/api' -import { featureFlagProvider } from '@sourcegraph/cody-shared' +import { currentAuthStatus, featureFlagProvider } from '@sourcegraph/cody-shared' import * as vscode from 'vscode' import { getConfiguration } from '../../configuration' import type { ExtensionApi } from '../../extension-api' @@ -8,7 +8,7 @@ import { getExtensionDetails } from '../telemetry-v2' // Ensure to ad exposed experiments at the very end to make sure we include experiments that the // user is being exposed to while the span was generated export function recordExposedExperimentsToSpan(span: Span): void { - span.setAttributes(featureFlagProvider.getExposedExperiments()) + span.setAttributes(featureFlagProvider.getExposedExperiments(currentAuthStatus().endpoint)) const extensionDetails = getExtensionDetails(getConfiguration(vscode.workspace.getConfiguration())) span.setAttributes(extensionDetails as any) diff --git a/vscode/src/services/sentry/sentry.ts b/vscode/src/services/sentry/sentry.ts index f56f63d27c74..ffb269c937f5 100644 --- a/vscode/src/services/sentry/sentry.ts +++ b/vscode/src/services/sentry/sentry.ts @@ -2,15 +2,15 @@ import type { init as browserInit } from '@sentry/browser' import type { init as nodeInit } from '@sentry/node' import { - type ClientConfigurationWithEndpoint, NetworkError, + type Unsubscribable, isAbortError, isAuthError, isDotCom, isError, isRateLimitError, + resolvedConfig, } from '@sourcegraph/cody-shared' - import { version } from '../../version' export * from '@sentry/core' @@ -19,67 +19,62 @@ const SENTRY_DSN = 'https://f565373301c9c7ef18448a1c60dfde8d@o19358.ingest.sentr export type SentryOptions = NonNullable<Parameters<typeof nodeInit | typeof browserInit>[0]> export abstract class SentryService { - constructor( - protected config: Pick< - ClientConfigurationWithEndpoint, - 'serverEndpoint' | 'isRunningInsideAgent' | 'agentIDE' - > - ) { - this.prepareReconfigure() - } - - public onConfigurationChange( - newConfig: Pick<ClientConfigurationWithEndpoint, 'serverEndpoint'> - ): void { - this.config = newConfig - this.prepareReconfigure() - } - - private prepareReconfigure(): void { - try { - const isProd = process.env.NODE_ENV === 'production' - - // Used to enable Sentry reporting in the development environment. - const isSentryEnabled = process.env.ENABLE_SENTRY === 'true' - if (!isProd && !isSentryEnabled) { - return + private configSubscription: Unsubscribable + + constructor() { + this.configSubscription = resolvedConfig.subscribe(({ configuration, auth }) => { + try { + const isProd = process.env.NODE_ENV === 'production' + + // Used to enable Sentry reporting in the development environment. + const isSentryEnabled = process.env.ENABLE_SENTRY === 'true' + if (!isProd && !isSentryEnabled) { + return + } + + const options: SentryOptions = { + dsn: SENTRY_DSN, + release: version, + sampleRate: 0.05, // 5% of errors are sent to Sentry + environment: configuration.isRunningInsideAgent + ? 'agent' + : typeof process === 'undefined' + ? 'vscode-web' + : 'vscode-node', + + // In dev mode, have Sentry log extended debug information to the console. + debug: !isProd, + + // Only send errors when connected to dotcom in the production build. + beforeSend: (event, hint) => { + if ( + isProd && + isDotCom(auth.serverEndpoint) && + shouldErrorBeReported( + hint.originalException, + !!configuration.isRunningInsideAgent + ) + ) { + return event + } + + return null + }, + } + + this.reconfigure(options) + } catch (error) { + // We don't want to crash the extension host or VS Code if Sentry fails to load. + console.error('Failed to initialize Sentry', error) } - - const options: SentryOptions = { - dsn: SENTRY_DSN, - release: version, - sampleRate: 0.05, // 5% of errors are sent to Sentry - environment: this.config.isRunningInsideAgent - ? 'agent' - : typeof process === 'undefined' - ? 'vscode-web' - : 'vscode-node', - - // In dev mode, have Sentry log extended debug information to the console. - debug: !isProd, - - // Only send errors when connected to dotcom in the production build. - beforeSend: (event, hint) => { - if ( - isProd && - isDotCom(this.config.serverEndpoint) && - shouldErrorBeReported(hint.originalException, !!this.config.isRunningInsideAgent) - ) { - return event - } - - return null - }, - } - - this.reconfigure(options) - } catch (error) { - // We don't want to crash the extension host or VS Code if Sentry fails to load. - console.error('Failed to initialize Sentry', error) - } + }) } protected abstract reconfigure(options: Parameters<typeof nodeInit | typeof browserInit>[0]): void + + public dispose(): void { + this.configSubscription.unsubscribe() + } } export function shouldErrorBeReported(error: unknown, insideAgent: boolean): boolean { diff --git a/vscode/src/services/telemetry-v2.ts b/vscode/src/services/telemetry-v2.ts index 53c60d5b9260..311712b16cb1 100644 --- a/vscode/src/services/telemetry-v2.ts +++ b/vscode/src/services/telemetry-v2.ts @@ -5,14 +5,16 @@ import { type LogEventMode, MockServerTelemetryRecorderProvider, NoOpTelemetryRecorderProvider, - type ResolvedConfiguration, TelemetryRecorderProvider, + resolvedConfig, + subscriptionDisposable, telemetryRecorder, telemetryRecorderProvider, updateGlobalTelemetryInstances, } from '@sourcegraph/cody-shared' import { TimestampTelemetryProcessor } from '@sourcegraph/telemetry/dist/processors/timestamp' +import type { Disposable } from 'vscode' import { logDebug } from '../log' import { getOSArch } from '../os' import { version } from '../version' @@ -53,81 +55,85 @@ const debugLogLabel = 'telemetry-v2' * new telemetry framework: * https://sourcegraph.com/docs/dev/background-information/telemetry */ -export async function createOrUpdateTelemetryRecorderProvider( - config: ResolvedConfiguration, +export function createOrUpdateTelemetryRecorderProvider( /** * Hardcode isExtensionModeDevOrTest to false to test real exports - when * true, exports are logged to extension output instead. */ isExtensionModeDevOrTest: boolean -): Promise<void> { - const extensionDetails = getExtensionDetails(config.configuration) - - // Add timestamp processor for realistic data in output for dev or no-op scenarios - const defaultNoOpProvider = new NoOpTelemetryRecorderProvider([new TimestampTelemetryProcessor()]) - - if (config.configuration.telemetryLevel === 'off' || !extensionDetails.ide) { - updateGlobalTelemetryInstances(defaultNoOpProvider) - return - } - - const anonymousUserID = localStorage.anonymousUserID() - const initialize = telemetryRecorderProvider === undefined - - /** - * In testing, send events to the mock server. - */ - if (process.env.CODY_TESTING === 'true') { - logDebug(debugLogLabel, 'using mock exporter') - updateGlobalTelemetryInstances( - new MockServerTelemetryRecorderProvider( - extensionDetails, - config.configuration, - anonymousUserID - ) - ) - } else if (isExtensionModeDevOrTest) { - logDebug(debugLogLabel, 'using no-op exports') - updateGlobalTelemetryInstances(defaultNoOpProvider) - } else { - updateGlobalTelemetryInstances( - new TelemetryRecorderProvider( - extensionDetails, - { ...config.configuration, ...config.auth }, - anonymousUserID, - legacyBackcompatLogEventMode - ) - ) - } +): Disposable { + return subscriptionDisposable( + resolvedConfig.subscribe(({ configuration, auth, clientState }) => { + const extensionDetails = getExtensionDetails(configuration) + + // Add timestamp processor for realistic data in output for dev or no-op scenarios + const defaultNoOpProvider = new NoOpTelemetryRecorderProvider([ + new TimestampTelemetryProcessor(), + ]) + + if (configuration.telemetryLevel === 'off' || !extensionDetails.ide) { + updateGlobalTelemetryInstances(defaultNoOpProvider) + return + } - const isCodyWeb = config.configuration.agentIDE === CodyIDE.Web + const initialize = telemetryRecorderProvider === undefined - /** - * On first initialization, also record some initial events. - * Skip any init events for Cody Web use case. - */ - const newAnonymousUser = localStorage.checkIfCreatedAnonymousUserID() - if (initialize && !isCodyWeb) { - if (newAnonymousUser) { /** - * New user + * In testing, send events to the mock server. */ - telemetryRecorder.recordEvent('cody.extension', 'installed', { - billingMetadata: { - product: 'cody', - category: 'billable', - }, - }) - } else if ( - !config.configuration.isRunningInsideAgent || - config.configuration.agentHasPersistentStorage - ) { + if (process.env.CODY_TESTING === 'true') { + logDebug(debugLogLabel, 'using mock exporter') + updateGlobalTelemetryInstances( + new MockServerTelemetryRecorderProvider( + extensionDetails, + configuration, + clientState.anonymousUserID + ) + ) + } else if (isExtensionModeDevOrTest) { + logDebug(debugLogLabel, 'using no-op exports') + updateGlobalTelemetryInstances(defaultNoOpProvider) + } else { + updateGlobalTelemetryInstances( + new TelemetryRecorderProvider( + extensionDetails, + { ...configuration, ...auth }, + clientState.anonymousUserID, + legacyBackcompatLogEventMode + ) + ) + } + + const isCodyWeb = configuration.agentIDE === CodyIDE.Web + /** - * Repeat user + * On first initialization, also record some initial events. + * Skip any init events for Cody Web use case. */ - telemetryRecorder.recordEvent('cody.extension', 'savedLogin') - } - } + const newAnonymousUser = localStorage.checkIfCreatedAnonymousUserID() + if (initialize && !isCodyWeb) { + if (newAnonymousUser) { + /** + * New user + */ + telemetryRecorder.recordEvent('cody.extension', 'installed', { + billingMetadata: { + product: 'cody', + category: 'billable', + }, + }) + } else if ( + !configuration.isRunningInsideAgent || + configuration.agentHasPersistentStorage + ) { + /** + * Repeat user + */ + telemetryRecorder.recordEvent('cody.extension', 'savedLogin') + } + } + }) + ) } /** diff --git a/vscode/src/test-support.ts b/vscode/src/test-support.ts index 65355b58460e..6fb86cc0ba05 100644 --- a/vscode/src/test-support.ts +++ b/vscode/src/test-support.ts @@ -1,4 +1,9 @@ -import { type ChatMessage, ps } from '@sourcegraph/cody-shared' +import { + type AuthStatus, + type ChatMessage, + currentAuthStatusOrNotReadyYet, + ps, +} from '@sourcegraph/cody-shared' import type { ChatController } from './chat/chat-view/ChatController' // A one-slot channel which lets readers block on a value being @@ -41,4 +46,8 @@ export class TestSupport { public async chatMessages(): Promise<readonly ChatMessage[]> { return (await this.chatPanelProvider.get()).getViewTranscript() } + + public authStatus(): AuthStatus | undefined { + return currentAuthStatusOrNotReadyYet() + } } diff --git a/vscode/src/testutils/mocks.ts b/vscode/src/testutils/mocks.ts index 57d276706c74..9ddd91498ae3 100644 --- a/vscode/src/testutils/mocks.ts +++ b/vscode/src/testutils/mocks.ts @@ -12,8 +12,6 @@ import type { import { type ClientConfiguration, type ClientConfigurationWithAccessToken, - type FeatureFlag, - FeatureFlagProvider, OLLAMA_DEFAULT_URL, ps, } from '@sourcegraph/cody-shared' @@ -873,22 +871,6 @@ export const vsCodeMocks = { ProgressLocation, } as const -export class MockFeatureFlagProvider extends FeatureFlagProvider { - constructor(private readonly enabledFlags: Set<FeatureFlag>) { - super() - } - - public evaluateFeatureFlag(flag: FeatureFlag): Promise<boolean> { - return Promise.resolve(this.enabledFlags.has(flag)) - } - - public refresh(): Promise<void> { - return Promise.resolve() - } -} - -export const emptyMockFeatureFlagProvider = new MockFeatureFlagProvider(new Set<FeatureFlag>()) - export const DEFAULT_VSCODE_SETTINGS = { proxy: undefined, codebase: '', diff --git a/vscode/test/integration/helpers.ts b/vscode/test/integration/helpers.ts index 23858ea6c0d0..7fb40b7cea00 100644 --- a/vscode/test/integration/helpers.ts +++ b/vscode/test/integration/helpers.ts @@ -12,16 +12,30 @@ import * as mockServer from '../fixtures/mock-server' */ export async function beforeIntegrationTest(): Promise<void> { // Wait for Cody extension to become ready. - const api = vscode.extensions.getExtension<ExtensionApi>('sourcegraph.cody-ai') - assert.ok(api, 'extension not found') + const ext = vscode.extensions.getExtension<ExtensionApi>('sourcegraph.cody-ai') + assert.ok(ext, 'extension not found') - await api?.activate() + const api = await ext?.activate() - // Wait for Cody to become activated. + // Authenticate extension. + await ensureExecuteCommand('cody.test.token', mockServer.SERVER_URL, mockServer.VALID_TOKEN) await new Promise(resolve => setTimeout(resolve, 200)) - // Configure extension. - await ensureExecuteCommand('cody.test.token', mockServer.SERVER_URL, mockServer.VALID_TOKEN) + function isAuthenticated(): boolean { + const authStatus = api.testing?.authStatus() + return !!authStatus?.authenticated && authStatus.endpoint === `${mockServer.SERVER_URL}/` + } + if (!isAuthenticated()) { + // Try waiting a bit longer. + await new Promise(resolve => setTimeout(resolve, 1000)) + if (!isAuthenticated()) { + throw new Error( + `Failed to authenticate for integration test (auth status is ${JSON.stringify( + api.testing?.authStatus() + )})` + ) + } + } } /**