Skip to content

Commit 1339ef1

Browse files
Autocomplete: Only sample requests that are also suggested (#3139)
Thinking about #3136 again, I don't think it's helpful to sample these traces even. It's not bad per-se, but it's easier to aggregate if we don't have to filter based on a root span attribute everywhere. E.g. I want to see the "median" completion time so I get the median timings of specific spans but if I have to exclude some values in the root span, this is getting more complex then necessary. Let's keep it simple and don't even sample these (which also saves some bandwidth) ## Test plan - Added a debugger breakpoint to see if the sampled attribute is set
1 parent 9fe310e commit 1339ef1

File tree

3 files changed

+21
-24
lines changed

3 files changed

+21
-24
lines changed

vscode/src/completions/inline-completion-item-provider.ts

Lines changed: 2 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -441,15 +441,7 @@ export class InlineCompletionItemProvider
441441
this.unstable_handleDidShowCompletionItem(autocompleteItems[0])
442442
}
443443

444-
// If the completion callbacks makes it this far, it is going to be displayed on the
445-
// client (for VS Code we have already run the did show handler). However, since we
446-
// are still inside the callback, we can add some final data to the span and decide
447-
// wether to sample it or not.
448-
markSpanAsSampled(
449-
span,
450-
result.source,
451-
completionProviderConfig.getPrefetchedFlag(FeatureFlag.CodyAutocompleteTracing)
452-
)
444+
addExposedExperimentsToSpan(span, result.source)
453445

454446
return autocompleteResult
455447
} catch (error) {
@@ -793,18 +785,9 @@ function onlyCompletionWidgetSelectionChanged(
793785
return prevSelectedCompletionInfo.text !== nextSelectedCompletionInfo.text
794786
}
795787

796-
function markSpanAsSampled(
797-
span: Span,
798-
source: InlineCompletionsResultSource,
799-
tracingFlagEnabled: boolean
800-
): void {
788+
function addExposedExperimentsToSpan(span: Span, source: InlineCompletionsResultSource): void {
801789
// Add exposed experiments at the very end to make sure we include experiments that the user is
802790
// being exposed to while the completion was generated
803791
span.setAttributes(featureFlagProvider.getExposedExperiments())
804792
span.setAttributes(getExtensionDetails(getConfiguration(vscode.workspace.getConfiguration())) as any)
805-
806-
const shouldSample = tracingFlagEnabled && source !== InlineCompletionsResultSource.HotStreak
807-
if (shouldSample) {
808-
span.setAttribute('sampled', true)
809-
}
810793
}

vscode/src/completions/logger.test.ts

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { InlineCompletionsResultSource, TriggerKind } from './get-inline-complet
1010
import * as CompletionLogger from './logger'
1111
import type { RequestParams } from './request-manager'
1212
import { documentAndPosition } from './test-helpers'
13+
import { initCompletionProviderConfig } from './get-inline-completions-tests/helpers'
1314

1415
const defaultArgs = {
1516
multiline: false,
@@ -46,7 +47,8 @@ const completionItemId = 'completion-item-id' as CompletionLogger.CompletionItem
4647
describe('logger', () => {
4748
let logSpy: MockInstance
4849
let recordSpy: MockInstance
49-
beforeEach(() => {
50+
beforeEach(async () => {
51+
await initCompletionProviderConfig({})
5052
logSpy = vi.spyOn(telemetryService, 'log')
5153
recordSpy = vi.spyOn(telemetryRecorder, 'recordEvent')
5254
})

vscode/src/completions/logger.ts

Lines changed: 16 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,12 @@ import { LRUCache } from 'lru-cache'
22
import * as uuid from 'uuid'
33
import * as vscode from 'vscode'
44

5-
import { isNetworkError, type BillingCategory, type BillingProduct } from '@sourcegraph/cody-shared'
5+
import {
6+
isNetworkError,
7+
type BillingCategory,
8+
type BillingProduct,
9+
FeatureFlag,
10+
} from '@sourcegraph/cody-shared'
611
import type { KnownString, TelemetryEventParameters } from '@sourcegraph/telemetry'
712

813
import { getConfiguration } from '../configuration'
@@ -20,6 +25,7 @@ import type { InlineCompletionItemWithAnalytics } from './text-processing/proces
2025
import { lines } from './text-processing/utils'
2126
import type { InlineCompletionItem } from './types'
2227
import type { Span } from '@opentelemetry/api'
28+
import { completionProviderConfig } from './completion-provider-config'
2329

2430
// A completion ID is a unique identifier for a specific completion text displayed at a specific
2531
// point in the document. A single completion can be suggested multiple times.
@@ -523,6 +529,15 @@ export function suggested(id: CompletionLogID, span?: Span): void {
523529
span?.setAttributes(getSharedParams(event) as any)
524530
span?.addEvent('suggested')
525531

532+
// Mark the completion as sampled if tracing is enable for this user
533+
const shouldSample = completionProviderConfig.getPrefetchedFlag(
534+
FeatureFlag.CodyAutocompleteTracing
535+
)
536+
537+
if (shouldSample && span) {
538+
span.setAttribute('sampled', true)
539+
}
540+
526541
setTimeout(() => {
527542
const event = activeSuggestionRequests.get(id)
528543
if (!event) {
@@ -540,9 +555,6 @@ export function suggested(id: CompletionLogID, span?: Span): void {
540555
event.suggestionAnalyticsLoggedAt = performance.now()
541556
}
542557
}, READ_TIMEOUT_MS)
543-
} else {
544-
span?.setAttributes(getSharedParams(event) as any)
545-
span?.addEvent('suggested-again')
546558
}
547559
}
548560

0 commit comments

Comments
 (0)