-
Notifications
You must be signed in to change notification settings - Fork 210
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Autocomplete: Improve sampling code and prepare for Honeycomb export #3034
Merged
Merged
Changes from 4 commits
Commits
Show all changes
21 commits
Select commit
Hold shift + click to select a range
838cfc7
WIP
philipp-spiess ec04cfa
FINALLY some progress
philipp-spiess f2b29ed
stuff
philipp-spiess 115ee9b
Make this pretty
philipp-spiess fa43d61
Make this even prettier
philipp-spiess fab2921
Merge remote-tracking branch 'origin/main' into ps/improve-tracing
philipp-spiess 3028b69
revert all pkg.json changes
philipp-spiess 77bf6b5
remove unuse import
philipp-spiess 91d3655
Fix imports
philipp-spiess 83719de
Fix imports 2
philipp-spiess 61fe9a2
More granular http tracing and sampling on the client
philipp-spiess 7b7f67e
Merge remote-tracking branch 'origin/main' into ps/improve-tracing
philipp-spiess de91915
Biome do your thing
philipp-spiess 09eb001
changelog
philipp-spiess 3ee52dd
Add tracing for feature flag stuff
philipp-spiess 59e0c8d
Merge remote-tracking branch 'origin/main' into ps/improve-tracing
philipp-spiess ed14333
Fixes
philipp-spiess 228e762
Unify logging for request handlers
philipp-spiess 52fb677
Fix bugs
philipp-spiess 1ba453c
Fix bugs
philipp-spiess b164517
Fixes
philipp-spiess File filter
Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,7 +2,12 @@ import { LRUCache } from 'lru-cache' | |
import * as uuid from 'uuid' | ||
import * as vscode from 'vscode' | ||
|
||
import { isNetworkError, type BillingCategory, type BillingProduct } from '@sourcegraph/cody-shared' | ||
import { | ||
isNetworkError, | ||
type BillingCategory, | ||
type BillingProduct, | ||
featureFlagProvider, | ||
} from '@sourcegraph/cody-shared' | ||
import type { KnownString, TelemetryEventParameters } from '@sourcegraph/telemetry' | ||
|
||
import { getConfiguration } from '../configuration' | ||
|
@@ -19,6 +24,7 @@ import * as statistics from './statistics' | |
import type { InlineCompletionItemWithAnalytics } from './text-processing/process-inline-completions' | ||
import { lines } from './text-processing/utils' | ||
import type { InlineCompletionItem } from './types' | ||
import { Span, trace } from '@opentelemetry/api' | ||
|
||
// A completion ID is a unique identifier for a specific completion text displayed at a specific | ||
// point in the document. A single completion can be suggested multiple times. | ||
|
@@ -505,7 +511,7 @@ export function loaded( | |
// | ||
// For statistics logging we start a timeout matching the READ_TIMEOUT_MS so we can increment the | ||
// suggested completion count as soon as we count it as such. | ||
export function suggested(id: CompletionLogID): void { | ||
export function suggested(id: CompletionLogID, span?: Span): void { | ||
const event = activeSuggestionRequests.get(id) | ||
if (!event) { | ||
return | ||
|
@@ -519,6 +525,12 @@ export function suggested(id: CompletionLogID): void { | |
if (!event.suggestedAt) { | ||
event.suggestedAt = performance.now() | ||
|
||
// Add exposed experiments at the very end to make sure we include experiments that the user | ||
// is being exposed to while the completion was generated | ||
span?.setAttributes(featureFlagProvider.getExposedExperiments()) | ||
span?.setAttributes(getSharedParams(event) as any) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why is There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Same as #3034 (comment) |
||
span?.addEvent('suggested') | ||
|
||
setTimeout(() => { | ||
const event = activeSuggestionRequests.get(id) | ||
if (!event) { | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This was causing the issue we saw with traces because for async responses, span.end would be called synchronously after creating the promise but before it resolved which left all spans be very short 😬
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TY for catching this! Makes sense.