From ffb7f161553003ec7b0f7e574165053751979ffd Mon Sep 17 00:00:00 2001 From: Eric Fritz Date: Thu, 13 Feb 2020 18:02:30 -0600 Subject: [PATCH 1/5] Add telemetry events. --- shared/badges.ts | 4 ++- shared/providers.ts | 73 +++++++++++++++++++++++++++++++++++---------- shared/telemetry.ts | 49 ++++++++++++++++++++++++++++++ shared/util/util.ts | 2 +- 4 files changed, 110 insertions(+), 18 deletions(-) create mode 100644 shared/telemetry.ts diff --git a/shared/badges.ts b/shared/badges.ts index 8e3174280..049126fa9 100644 --- a/shared/badges.ts +++ b/shared/badges.ts @@ -1,3 +1,5 @@ +import * as sourcegraph from 'sourcegraph' + const g: any = global g.btoa = g.btoa || ((s: string) => Buffer.from(s, 'binary').toString('base64')) @@ -58,7 +60,7 @@ function makeInfoIcon(color: string): string { /** * The badge to send back on all results that come from searched-based data. */ -export const impreciseBadge = { +export const impreciseBadge: sourcegraph.BadgeAttachmentRenderOptions = { icon: makeInfoIcon('#ffffff'), light: { icon: makeInfoIcon('#000000') }, hoverMessage: diff --git a/shared/providers.ts b/shared/providers.ts index cabb1d2bb..2e1915ec3 100644 --- a/shared/providers.ts +++ b/shared/providers.ts @@ -5,6 +5,7 @@ import { impreciseBadge } from './badges' import { LanguageSpec } from './language-specs/spec' import { createProviders as createLSIFProviders } from './lsif/providers' import { createProviders as createSearchProviders } from './search/providers' +import { TelemetryEmitter } from './telemetry' import { noopAsyncGenerator, observableFromAsyncIterator } from './util/ix' import { asArray, mapArrayish } from './util/util' @@ -143,9 +144,12 @@ export function createDefinitionProvider( doc: sourcegraph.TextDocument, pos: sourcegraph.Position ): AsyncGenerator { + const emitter = new TelemetryEmitter() + let lastLsifResult: sourcegraph.Definition | undefined for await (const lsifResult of lsifProvider(doc, pos)) { if (lsifResult) { + await emitter.emitOnce('lsifDefinitions') yield lsifResult lastLsifResult = lsifResult } @@ -156,19 +160,24 @@ export function createDefinitionProvider( } if (lspProvider) { - // Delegate to LSP if it's available. Do not try to supplement + for await (const lspResult of lspProvider(doc, pos)) { + await emitter.emitOnce('lspDefinitions') + yield lspResult + } + + // Do not try to supplement // with additional search results as we have all the context we // need for complete and precise results here. - yield* lspProvider(doc, pos) return } for await (const searchResult of searchProvider(doc, pos)) { - // No results so far, fall back to search. Mark result as imprecise. - yield mapArrayish(searchResult, location => ({ - ...location, - badge: impreciseBadge, - })) + // No results so far, fall back to search. Mark the result as + // imprecise. + if (searchResult) { + await emitter.emitOnce('searchDefinitions') + yield badgeValues(searchResult, impreciseBadge) + } } }), } @@ -197,9 +206,12 @@ export function createReferencesProvider( pos: sourcegraph.Position, ctx: sourcegraph.ReferenceContext ): AsyncGenerator { + const emitter = new TelemetryEmitter() + let lsifResults: sourcegraph.Location[] = [] for await (const lsifResult of lsifProvider(doc, pos, ctx)) { if (lsifResult) { + await emitter.emitOnce('lsifReferences') yield lsifResult lsifResults = lsifResult } @@ -215,6 +227,7 @@ export function createReferencesProvider( // Re-emit the last results from the previous provider // so we do not overwrite what was emitted previously. + await emitter.emitOnce('lspReferences') yield lsifResults.concat(filteredResults) } @@ -241,11 +254,9 @@ export function createReferencesProvider( // Re-emit the last results from the previous provider so we // do not overwrite what was emitted previously. Mark new results // as imprecise. + await emitter.emitOnce('searchReferences') yield lsifResults.concat( - filteredResults.map(location => ({ - ...location, - badge: impreciseBadge, - })) + asArray(badgeValues(filteredResults, impreciseBadge)) ) } }), @@ -273,9 +284,12 @@ export function createHoverProvider( void, undefined > { + const emitter = new TelemetryEmitter() + let lastLsifResult: sourcegraph.Hover | null | undefined for await (const lsifResult of lsifProvider(doc, pos)) { if (lsifResult) { + await emitter.emitOnce('lsifHover') yield lsifResult lastLsifResult = lsifResult } @@ -286,16 +300,25 @@ export function createHoverProvider( } if (lspProvider) { - // Delegate to LSP if it's available. Do not try to supplement - // with additional search results as we have all the context we - // need for complete and precise results here. - yield* lspProvider(doc, pos) + // Delegate to LSP if it's available. + for await (const lspResult of lspProvider(doc, pos)) { + if (lspResult) { + await emitter.emitOnce('lspHover') + yield lspResult + } + } + + // Do not try to supplement with additional search results + // as we have all the context we need for complete and precise + // results here. return } for await (const searchResult of searchProvider(doc, pos)) { - // No results so far, fall back to search. Mark result as imprecise. + // No results so far, fall back to search. Mark the result as + // imprecise. if (searchResult) { + await emitter.emitOnce('searchHover') yield { ...searchResult, badge: impreciseBadge } } } @@ -303,6 +326,24 @@ export function createHoverProvider( } } +/** + * Add a badge property to a single value or to a list of values. Returns the + * modified result in the same shape as the input. + * + * @param value The list of values, a single value, or null. + * @param badge The badge attachment. + */ +export function badgeValues( + value: T | T[] | null, + badge: sourcegraph.BadgeAttachmentRenderOptions +): sourcegraph.Badged | sourcegraph.Badged[] | null { + return Array.isArray(value) + ? value.map(v => ({ ...v, badge })) + : value + ? { ...value, badge } + : null +} + /** * Converts an async generator provider into an observable provider. This also * memoizes the previous result as a workaround for #1321 (below). diff --git a/shared/telemetry.ts b/shared/telemetry.ts new file mode 100644 index 000000000..5d38f545b --- /dev/null +++ b/shared/telemetry.ts @@ -0,0 +1,49 @@ +import * as sourcegraph from 'sourcegraph' + +/** + * A wrapper around telemetry events. A new instance of this class + * should be instantiated at the start of each action as it handles + * latency tracking. + */ +export class TelemetryEmitter { + private started: number + private emitted = new Set() + + constructor() { + this.started = performance.now() + } + + /** + * Emit a telemetry event with a durationMs attribute only if the + * same action has not yet emitted for this instance. + */ + public emitOnce(action: string, args: object = {}): Promise { + if (!this.emitted.has(action)) { + return Promise.resolve() + } + + this.emitted.add(action) + return this.emit(action, args) + } + + /** + * Emit a telemetry event with a durationMs attribute. + */ + public async emit(action: string, args: object = {}): Promise { + try { + await sourcegraph.commands.executeCommand( + 'logTelemetryEvent', + `codeintel.${action}`, + { ...args, durationMs: this.elapsed() } + ) + } catch { + // Older version of Sourcegraph may have not registered this + // command, causing the promise to reject. We can safely ignore + // this condition. + } + } + + private elapsed(): number { + return performance.now() - this.started + } +} diff --git a/shared/util/util.ts b/shared/util/util.ts index 3f8c22bd4..ca361bd52 100644 --- a/shared/util/util.ts +++ b/shared/util/util.ts @@ -17,7 +17,7 @@ export function asArray(value: T | T[] | null): T[] { } /** - * Apply a map function on a singel value or over a list of values. Returns the + * Apply a map function on a single value or over a list of values. Returns the * modified result in the same shape as the input. * * @param value The list of values, a single value, or null. From 87a71614c870286f8c82f622593d2f6d571d6a95 Mon Sep 17 00:00:00 2001 From: Eric Fritz Date: Thu, 13 Feb 2020 18:31:20 -0600 Subject: [PATCH 2/5] Add polyfill for performance. --- shared/badges.ts | 1 + shared/telemetry.ts | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/shared/badges.ts b/shared/badges.ts index 049126fa9..2361a2722 100644 --- a/shared/badges.ts +++ b/shared/badges.ts @@ -1,5 +1,6 @@ import * as sourcegraph from 'sourcegraph' +// Polyfill const g: any = global g.btoa = g.btoa || ((s: string) => Buffer.from(s, 'binary').toString('base64')) diff --git a/shared/telemetry.ts b/shared/telemetry.ts index 5d38f545b..97f003179 100644 --- a/shared/telemetry.ts +++ b/shared/telemetry.ts @@ -1,5 +1,9 @@ import * as sourcegraph from 'sourcegraph' +// Polyfill +const g: any = global +g.performance = g.performance || { now: () => 0 } + /** * A wrapper around telemetry events. A new instance of this class * should be instantiated at the start of each action as it handles From b06baf2f0938b63116042443b85753e70c243d56 Mon Sep 17 00:00:00 2001 From: Eric Fritz Date: Thu, 13 Feb 2020 18:31:33 -0600 Subject: [PATCH 3/5] Fix bad bool check. --- shared/telemetry.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/shared/telemetry.ts b/shared/telemetry.ts index 97f003179..584e40849 100644 --- a/shared/telemetry.ts +++ b/shared/telemetry.ts @@ -22,7 +22,7 @@ export class TelemetryEmitter { * same action has not yet emitted for this instance. */ public emitOnce(action: string, args: object = {}): Promise { - if (!this.emitted.has(action)) { + if (this.emitted.has(action)) { return Promise.resolve() } From 0304bbdda135a5e120b85e2533bff71eaed0ade2 Mon Sep 17 00:00:00 2001 From: Eric Fritz Date: Thu, 13 Feb 2020 18:31:40 -0600 Subject: [PATCH 4/5] Use mapArrayish where applicable. --- shared/providers.ts | 6 +----- 1 file changed, 1 insertion(+), 5 deletions(-) diff --git a/shared/providers.ts b/shared/providers.ts index 2e1915ec3..d66e84c4e 100644 --- a/shared/providers.ts +++ b/shared/providers.ts @@ -337,11 +337,7 @@ export function badgeValues( value: T | T[] | null, badge: sourcegraph.BadgeAttachmentRenderOptions ): sourcegraph.Badged | sourcegraph.Badged[] | null { - return Array.isArray(value) - ? value.map(v => ({ ...v, badge })) - : value - ? { ...value, badge } - : null + return mapArrayish(value, v => ({ ...v, badge })) } /** From ee07d6367ea0ab526dad100b1566cb944fec33e5 Mon Sep 17 00:00:00 2001 From: Eric Fritz Date: Mon, 24 Feb 2020 13:32:53 -0600 Subject: [PATCH 5/5] Use date over performance.now. --- shared/telemetry.ts | 8 ++------ 1 file changed, 2 insertions(+), 6 deletions(-) diff --git a/shared/telemetry.ts b/shared/telemetry.ts index 584e40849..143466350 100644 --- a/shared/telemetry.ts +++ b/shared/telemetry.ts @@ -1,9 +1,5 @@ import * as sourcegraph from 'sourcegraph' -// Polyfill -const g: any = global -g.performance = g.performance || { now: () => 0 } - /** * A wrapper around telemetry events. A new instance of this class * should be instantiated at the start of each action as it handles @@ -14,7 +10,7 @@ export class TelemetryEmitter { private emitted = new Set() constructor() { - this.started = performance.now() + this.started = Date.now() } /** @@ -48,6 +44,6 @@ export class TelemetryEmitter { } private elapsed(): number { - return performance.now() - this.started + return Date.now() - this.started } }