Adding Distributed Tracing and Smart Apply to cody #6178

arafatkatze · 2024-11-22T11:11:24Z

This PR introduces distributed tracing to connect traces originating in the UX with those starting in the extension host. This ensures unified and all-encompassing traces for operations spanning both the web view and the extension host.

NOTE: this PR has been rebased to the main and is ready for review

Changes Made

Distributed Tracing for Chat Interaction: Traces starting in the UX are now connected to corresponding traces in the extension host for chat interactions.
Distributed Tracing for Smart Apply: Similarly, traces for Smart Apply now span both the UX and the extension host.

These changes are in alignment with the milestone goals and aim to deliver foundational support for distributed tracing in these areas.

Next Steps

Immediate Goal: Ensure distributed tracing is functional and provides value for chat interaction and Smart Apply use cases.
I will rebase the PR and resolve merge conflicts AFTER the proper merge of Integrate OpenTelemetry tracing in Cody Webview #6100 it was reverted last time because of some issues
Follow-Up Work for next PR:
Refactor naming conventions for better consistency.
Eliminate redundant metric names.

Test plan

Run sourcegraph instance locally
Run sg start otel
Run the debugger for vscode cody locally on this branch
Perform some chat operations
Go to http://localhost:16686 to see if Jaegar is running
Select Cody-Client as the service
See a trace with the title chat-interaction this is a collection of spans that show a single trace for spans from both webview and extension host

Changelog

vscode/webviews/utils/webviewOpenTelemetryService.ts

vscode/webviews/chat/Transcript.tsx

vscode/webviews/utils/telemetry.ts

vscode/webviews/Chat.tsx

arafatkatze · 2024-11-25T10:12:29Z

vscode/src/chat/chat-view/ChatController.ts

-
-            await this.saveSession()
-            signal.throwIfAborted()
+        return context.with(extractedContext, () => {


Adding distributed tracing in chat so now the trace looks like

This has both UX and extension host stuff together. See related comment

@dominiccooney

…ation in chat

github-actions · 2024-12-02T11:25:48Z

‼️ Hey @sourcegraph/cody-security, please review this PR carefully as it introduces the usage of an unsafe_ function or abuses PromptString.

arafatkatze · 2024-12-02T13:22:08Z

Initially, I encountered a strange race condition where the traces didn’t always “link up” as they should, which meant some traces weren’t visible in the expected order. You can see the traces with Handleusermessage as the title and they are from the Cody extension host alone whereas the traces with chat-interaction as the title are the "full traces".

However, the issue resolved itself over time—I just had to wait.

The underlying problem stemmed from the way buffering happens differently on the Cody client side versus the webview side. Occasionally, the trace from the webview would arrive first, while other times the trace from the extension host would arrive first. This discrepancy in buffering cadence caused the traces to appear mismatched initially.

Eventually, Jaeger’s trace-linking mechanism kicks in and aligns all the traces correctly in the user interface. After some time, you end up with a properly linked view, like this:

arafatkatze · 2024-12-03T07:47:17Z

vscode/src/services/open-telemetry/CodyTraceExport.ts

            if (rootSpan === null) {
+                // the child of the root is sampled but root is not and the span is  continued
+                const rootChildSpan = getRootChildSpan(spanMap, span)
+                if (rootChildSpan && isSampled(rootChildSpan) && isContinued(rootChildSpan)) {


The naming conventions in this part could use some improvement because the logic I’m applying here is essential for linking the traces where the root is actually located in the webview. The key idea is that if the root isn’t present in the traces here, I want to measure the child of the root (the trace located in the extension host) and then send that one, as it effectively serves as the ‘root’ in this context. Hopefully, that clarifies things.

@dominiccooney

dominiccooney

Some feedback inline.

It is GREAT that we're joining traces across these domains.

vscode/src/chat/chat-view/ChatController.ts

vscode/src/edit/manager.ts

vscode/src/edit/provider.ts

vscode/src/services/open-telemetry/CodyTraceExport.ts

vscode/webviews/Chat.tsx

dominiccooney

Looks good, nice cleanup.

Can you remove the questions and concerns from the PR description? (If you still have questions and concerns, let's go through those.)

lib/shared/src/prompt/prompt-string.ts

arafatkatze · 2024-12-05T02:01:02Z

Can you remove the questions and concerns from the PR description? (If you still have questions and concerns, let's go through those.)

Removed

This reverts commit 5686699.

This reverts commits 5686699 and 6bf801b. They cause stdout logspam. ## Test plan CI ## Changelog - fix/edits: Distributed tracing of webview and Smart Apply, recently added, is reverted.

This reverts commit 1c6bc95.

arafatkatze changed the title ~~Dist tracing~~ Adding Distributed Tracing and Smart Apply to cody Nov 25, 2024

arafatkatze force-pushed the dist-tracing branch from 9d23bde to 5b3c62f Compare November 25, 2024 09:45