Add tests to AI event logging #1602
Conversation
| @@ -692,7 +692,7 @@ export class Kernel implements Disposable { | |||
| } | |||
|
|
|||
| TelemetryReporter.sendTelemetryEvent('cell.startExecute') | |||
| getEventReporter().reportExecution(cell) | |||
| await getEventReporter().reportExecution(cell) | |||
There was a problem hiding this comment.
@sourishkrout Won't this block execution below? For example, suppose the Foyle service is down. We might reasonably expect the eventReporter to buffer the events and perform some retrying. But I don't want to block actual cell execution. Is there a better pattern to achieve this?
There was a problem hiding this comment.
Yes. I incorrectly assumed the noop event reporter would be unless the "AI experiment" is turned on.
I agree with you about this. I figured there are likely a few things that need to be addressed to move to a smooth "out-of-the-box" experience.
We could rewrite the Reporter to resemble the telemetry one (don't love the impl either actually), where it separates producing (enqueuing) and reporting the events. However, I'd suggest that for the time being, we use the noop logger unless AI is turned on. And track a separate effort to make this default robust.
I can rewrite this reasonably quickly at a later point. Wdyt @jlewi?
There was a problem hiding this comment.
I decided to reply on the other PR
https://github.com/stateful/vscode-runme/pull/1589/files#r1739511628
|
* Start writing the code to log events. * Update foyle buf package to pull in the events protos. * Rough version of the code. * Update the connect package for Foyle. * Latest version; to try to show the problematic line * Bug fix: AIManager doesn't properly get the configuration for baseURL * We weren't actually reading the baseURL from the config because the key should be "aiBaseURL" not "runme.aiBaseURL" * I tried to add logging to ensure we know what the actual value of the endpoint is but that seemed to prevent the extension from loading. * Include context in execution events. * Fix test. * Define a sessionmanager to manager the context ID so we can attach it to requests. * Include the selected index in the events. * Log accepted suggestions. * Log session starts and ends. * Revert examples. * Initialize the logger properly. * Add ULID * Add a ULID to events * Add return * Add todo * Cherry-pick Sebastian's change bc2b0ac * Add tests to AI event logging (#1602) * Add to tests * Add todo * Let vs var --------- Co-authored-by: Sebastian Tiedtke <sebastiantiedtke@gmail.com>
See diff, please, @jlewi.