Skip to content

Commit

Permalink
agent: add telemetry/recordEvent to replace graphql/logEvent (#1713)
Browse files Browse the repository at this point in the history
1. Add `telemetry/recordEvent`, which sends events to a telemetry
recorder provider backed by a backwards-compatible GraphQL exporter -
the same one we use for VSCode, added in #1192. This replaces the
current `graphql/logEvent`
2. Deprecate `extensionConfiguration.eventProperties`
3. Clients should now provide `{ extensionConfiguration: {
anonymousUserID }` up-front


## Test plan

`pnpm build` and `node dist/index.js` are okay, but might need some help
testing this properly with a real extension
  • Loading branch information
bobheadxi committed Nov 17, 2023
1 parent 2152573 commit 19101ca
Show file tree
Hide file tree
Showing 12 changed files with 241 additions and 43 deletions.
1 change: 1 addition & 0 deletions agent/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
},
"dependencies": {
"@sourcegraph/cody-shared": "workspace:*",
"@sourcegraph/telemetry": "^0.13.0",
"@sourcegraph/scip-typescript": "^0.3.9",
"commander": "^11.1.0",
"env-paths": "^3.0.0",
Expand Down
101 changes: 82 additions & 19 deletions agent/src/agent.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,10 @@ import { convertGitCloneURLToCodebaseName } from '@sourcegraph/cody-shared/dist/
import { Client, createClient } from '@sourcegraph/cody-shared/src/chat/client'
import { registeredRecipes } from '@sourcegraph/cody-shared/src/chat/recipes/agent-recipes'
import { SourcegraphNodeCompletionsClient } from '@sourcegraph/cody-shared/src/sourcegraph-api/completions/nodeClient'
import { setUserAgent } from '@sourcegraph/cody-shared/src/sourcegraph-api/graphql/client'
import { LogEventMode, setUserAgent } from '@sourcegraph/cody-shared/src/sourcegraph-api/graphql/client'
import { BillingCategory, BillingProduct } from '@sourcegraph/cody-shared/src/telemetry-v2'
import { NoOpTelemetryRecorderProvider } from '@sourcegraph/cody-shared/src/telemetry-v2/TelemetryRecorderProvider'
import { TelemetryEventParameters } from '@sourcegraph/telemetry'

import { activate } from '../../vscode/src/extension.node'

Expand All @@ -19,6 +22,7 @@ import { AgentWorkspaceDocuments } from './AgentWorkspaceDocuments'
import { AgentEditor } from './editor'
import { MessageHandler } from './jsonrpc-alias'
import { AutocompleteItem, ClientInfo, ExtensionConfiguration, RecipeInfo } from './protocol-alias'
import { AgentHandlerTelemetryRecorderProvider } from './telemetry'
import * as vscode_shim from './vscode-shim'

const secretStorage = new Map<string, string>()
Expand Down Expand Up @@ -135,6 +139,24 @@ export class Agent extends MessageHandler {

private clientInfo: ClientInfo | null = null

/**
* agentTelemetryRecorderProvider must be used for all events recording
* directly within the agent (i.e. code in agent/src/...) and via the agent's
* 'telemetry/recordEvent' RPC.
*
* Components that use VSCode implementations directly (i.e. code in
* vscode/src/...) will continue to use the shared recorder initialized and
* configured as part of VSCode initialization in vscode/src/services/telemetry-v2.ts.
*/
private agentTelemetryRecorderProvider: AgentHandlerTelemetryRecorderProvider = new NoOpTelemetryRecorderProvider([
{
processEvent: event =>
process.stderr.write(
`Cody Agent: failed to record telemetry event '${event.feature}/${event.action}' before agent initialization\n`
),
},
])

constructor() {
super()
vscode_shim.setWorkspaceDocuments(this.workspace)
Expand All @@ -149,15 +171,14 @@ export class Agent extends MessageHandler {
: vscode.Uri.from({ scheme: 'file', path: clientInfo.workspaceRootPath })
initializeVscodeExtension(this.workspace.workspaceRootUri)

if (clientInfo.extensionConfiguration) {
this.setClient(clientInfo.extensionConfiguration)
}

// Register client info
this.clientInfo = clientInfo
setUserAgent(`${clientInfo?.name} / ${clientInfo?.version}`)

if (clientInfo.extensionConfiguration) {
await this.setClientAndTelemetry(clientInfo.extensionConfiguration)
}
const codyClient = await this.client

if (!codyClient) {
return {
name: 'cody-agent',
Expand Down Expand Up @@ -207,7 +228,11 @@ export class Agent extends MessageHandler {
vscode_shim.onDidCloseTextDocument.fire(this.workspace.agentTextDocument(document))
})

this.registerNotification('extensionConfiguration/didChange', config => this.setClient(config))
this.registerNotification('extensionConfiguration/didChange', config => {
this.setClientAndTelemetry(config).catch(() => {
process.stderr.write('Cody Agent: failed to update configuration\n')
})
})

this.registerRequest('recipes/list', () =>
Promise.resolve(
Expand Down Expand Up @@ -239,7 +264,8 @@ export class Agent extends MessageHandler {
})
}

await this.recordEvent(`recipe:${data.id}`, 'executed')
await this.logEvent(`recipe:${data.id}`, 'executed', 'dotcom-only')
this.agentTelemetryRecorderProvider.getRecorder().recordEvent(`cody.recipe.${data.id}`, 'executed')
await client.executeRecipe(data.id, {
signal: abortController.signal,
humanChatInput: data.humanChatInput,
Expand Down Expand Up @@ -313,6 +339,28 @@ export class Agent extends MessageHandler {

throw id
})

this.registerRequest('telemetry/recordEvent', async event => {
this.agentTelemetryRecorderProvider.getRecorder().recordEvent(
// 👷 HACK: We have no control over what gets sent over JSON RPC,
// so we depend on client implementations to give type guidance
// to ensure that we don't accidentally share arbitrary,
// potentially sensitive string values. In this RPC handler,
// when passing the provided event to the TelemetryRecorder
// implementation, we forcibly cast all the inputs below
// (feature, action, parameters) into known types (strings
// 'feature', 'action', 'key') so that the recorder will accept
// it. DO NOT do this elsewhere!
event.feature as 'feature',
event.action as 'action',
event.parameters as TelemetryEventParameters<{ key: number }, BillingProduct, BillingCategory>
)
return Promise.resolve(null)
})

/**
* @deprecated use 'telemetry/recordEvent' instead.
*/
this.registerRequest('graphql/logEvent', async event => {
const client = await this.client
if (typeof event.argument === 'object') {
Expand All @@ -321,9 +369,6 @@ export class Agent extends MessageHandler {
if (typeof event.publicArgument === 'object') {
event.publicArgument = JSON.stringify(event.publicArgument)
}

// TODO: Add support for new telemetry recorder, e.g.
// https://github.com/sourcegraph/cody/pull/1192
await client?.graphqlClient.logEvent(event, 'all')
return null
})
Expand Down Expand Up @@ -360,8 +405,28 @@ export class Agent extends MessageHandler {
})
}

private setClient(config: ExtensionConfiguration): void {
/**
* Updates this.client immediately and attempts to update
* this.telemetryRecorderProvider as well if prerequisite configuration
* is available.
*/
private async setClientAndTelemetry(config: ExtensionConfiguration): Promise<void> {
this.client = this.createAgentClient(config)

const codyClient = await this.client
if (codyClient && this.clientInfo) {
// Update telemetry
this.agentTelemetryRecorderProvider?.unsubscribe()
this.agentTelemetryRecorderProvider = new AgentHandlerTelemetryRecorderProvider(
codyClient.graphqlClient,
this.clientInfo,
{
// Add tracking metadata if provided
getMarketingTrackingMetadata: () => this.clientInfo?.marketingTracking || null,
}
)
}

return
}

Expand Down Expand Up @@ -396,10 +461,9 @@ export class Agent extends MessageHandler {
}

/**
* TODO: feature, action should require lib/shared/src/telemetry-v2 types,
* i.e. EventFeature and EventAction.
* @deprecated use `this.telemetryRecorderProvider.getRecorder()` instead.
*/
private async recordEvent(feature: string, action: string): Promise<null> {
private async logEvent(feature: string, action: string, mode: LogEventMode): Promise<null> {
const client = await this.client
if (!client) {
return null
Expand All @@ -420,15 +484,14 @@ export class Agent extends MessageHandler {
return null
}

// TODO: Add support for new telemetry recorder, e.g.
// https://github.com/sourcegraph/cody/pull/1192
const event = `${eventProperties.prefix}:${feature}:${action}`
await client.graphqlClient.logEvent(
{
event,
url: '',
client: eventProperties.client,
userCookieID: eventProperties.anonymousUserID,
userCookieID:
this.clientInfo?.extensionConfiguration?.anonymousUserID || eventProperties.anonymousUserID,
source: eventProperties.source,
publicArgument: JSON.stringify({
serverEndpoint: extensionConfiguration.serverEndpoint,
Expand All @@ -439,7 +502,7 @@ export class Agent extends MessageHandler {
},
}),
},
'all'
mode
)

return null
Expand Down
7 changes: 6 additions & 1 deletion agent/src/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ describe.each([
workspaceRootUri: 'file:///path/to/foo',
workspaceRootPath: '/path/to/foo',
extensionConfiguration: {
anonymousUserID: 'abcde1234',
accessToken: process.env.SRC_ACCESS_TOKEN ?? 'invalid',
serverEndpoint: process.env.SRC_ENDPOINT ?? 'invalid',
customHeaders: {},
Expand All @@ -62,6 +63,7 @@ describe.each([
workspaceRootUri: 'file:///path/to/foo',
workspaceRootPath: '/path/to/foo',
extensionConfiguration: {
anonymousUserID: 'abcde1234',
accessToken: process.env.SRC_ACCESS_TOKEN ?? 'invalid',
serverEndpoint: process.env.SRC_ENDPOINT ?? 'invalid',
customHeaders: {},
Expand All @@ -74,8 +76,9 @@ describe.each([
name: 'test-client',
version: 'v1',
workspaceRootUri: 'file:///path/to/foo',
workspaceRootPath: '/path/to/foo',
extensionConfiguration: {
anonymousUserID: 'abcde1234',
workspaceRootPath: '/path/to/foo',
accessToken: '',
serverEndpoint: 'https://sourcegraph.com/',
customHeaders: {},
Expand Down Expand Up @@ -118,11 +121,13 @@ describe.each([
// fine as long as we didn't send the second unauthenticated config
// change.
client.notify('extensionConfiguration/didChange', {
anonymousUserID: 'abcde1234',
accessToken: 'https://sourcegraph.com/',
serverEndpoint: '',
customHeaders: {},
})
client.notify('extensionConfiguration/didChange', {
anonymousUserID: 'abcde1234',
accessToken: process.env.SRC_ACCESS_TOKEN ?? 'invalid',
serverEndpoint: process.env.SRC_ENDPOINT ?? 'invalid',
customHeaders: {},
Expand Down
36 changes: 36 additions & 0 deletions agent/src/telemetry/index.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,36 @@
import { SourcegraphGraphQLAPIClient } from '@sourcegraph/cody-shared/src/sourcegraph-api/graphql'
import { GraphQLTelemetryExporter } from '@sourcegraph/cody-shared/src/sourcegraph-api/telemetry/GraphQLTelemetryExporter'
import { BillingCategory, BillingProduct } from '@sourcegraph/cody-shared/src/telemetry-v2'
import {
defaultEventRecordingOptions,
MarketingTrackingProvider,
MarketingTrackingTelemetryProcessor,
TelemetryRecorderProvider,
} from '@sourcegraph/telemetry'

import { ClientInfo } from '../protocol-alias'

/**
* Default implementation of a TelemetryRecorderProvider for use in the Agent
* handler only.
*/
export class AgentHandlerTelemetryRecorderProvider extends TelemetryRecorderProvider<BillingProduct, BillingCategory> {
constructor(
graphql: SourcegraphGraphQLAPIClient,
clientInfo: ClientInfo,
marketingTrackingProvider: MarketingTrackingProvider
) {
super(
{
client: clientInfo.name,
clientVersion: clientInfo.version,
},
new GraphQLTelemetryExporter(graphql, clientInfo.extensionConfiguration?.anonymousUserID || '', 'all'),
[new MarketingTrackingTelemetryProcessor(marketingTrackingProvider)],
{
...defaultEventRecordingOptions,
bufferTimeMs: 0, // disable buffering for now
}
)
}
}
2 changes: 1 addition & 1 deletion agent/src/vscode-shim.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ const configuration: vscode.WorkspaceConfiguration = {
case 'cody.customHeaders':
return connectionConfig?.customHeaders
case 'cody.telemetry.level':
// Use the dedicated `graphql/logEvent` to send telemetry from
// Use the dedicated `telemetry/recordEvent` to send telemetry from
// agent clients. The reason we disable telemetry via config is
// that we don't want to submit vscode-specific events when
// running inside the agent.
Expand Down
2 changes: 1 addition & 1 deletion lib/shared/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
},
"dependencies": {
"@microsoft/fetch-event-source": "^2.0.1",
"@sourcegraph/telemetry": "^0.11.0",
"@sourcegraph/telemetry": "^0.13.0",
"dompurify": "^3.0.4",
"highlight.js": "^10.7.3",
"isomorphic-fetch": "^3.0.0",
Expand Down
9 changes: 9 additions & 0 deletions lib/shared/src/sourcegraph-api/graphql/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -185,6 +185,9 @@ function extractDataOrError<T, R>(response: APIResponse<T> | Error, extract: (da
return extract(response.data)
}

/**
* @deprecated Use 'TelemetryEvent' instead.
*/
export interface event {
event: string
userCookieID: string
Expand Down Expand Up @@ -423,6 +426,9 @@ export class SourcegraphGraphQLAPIClient {
* gets exported: https://docs.sourcegraph.com/dev/background-information/telemetry
*
* Only available on Sourcegraph 5.2.0 and later.
*
* DO NOT USE THIS DIRECTLY - use an implementation of implementation
* TelemetryRecorder from '@sourcegraph/telemetry' instead.
*/
public async recordTelemetryEvents(events: TelemetryEventInput[]): Promise<{} | Error> {
const initialResponse = await this.fetchSourcegraphAPI<APIResponse<{}>>(RECORD_TELEMETRY_EVENTS_MUTATION, {
Expand All @@ -433,6 +439,9 @@ export class SourcegraphGraphQLAPIClient {

/**
* logEvent is the legacy event-logging mechanism.
*
* @deprecated use an implementation of implementation TelemetryRecorder
* from '@sourcegraph/telemetry' instead.
*/
public async logEvent(event: event, mode: LogEventMode): Promise<LogEventResponse | Error> {
if (process.env.CODY_TESTING === 'true') {
Expand Down
Loading

0 comments on commit 19101ca

Please sign in to comment.