fix/tracing: Add Client Platform and agentVersion to traces #6889
Conversation
arafatkatze
requested review from
dominiccooney
and removed request for
dominiccooney
arafatkatze
changed the title
arafatkatze
changed the title
| @@ -45,7 +48,7 @@ export class OpenTelemetryService { | |||
| this.tracerProvider = new NodeTracerProvider({ | |||
| resource: new Resource({ | |||
| [SemanticResourceAttributes.SERVICE_NAME]: 'cody-client', | |||
| [SemanticResourceAttributes.SERVICE_VERSION]: version, | |||
| [SemanticResourceAttributes.SERVICE_VERSION]: clientCapabilities().agentExtensionVersion, | |||
There was a problem hiding this comment.
Its the same value so I wanted to maintain consistency in the usage of agentExtensionVersion
|
|
@dominiccooney Thanks for the pointers I updated the PR description properly. |
arafatkatze
changed the title
arafatkatze
changed the title
Right, and where does JetBrains, VS, etc. set that? |
dominiccooney
changed the title
|
You should literally just write You should remove scree like |
| @@ -14,8 +14,8 @@ export class WebviewOpenTelemetryService { | |||
| private unloadInstrumentations?: () => void | |||
| private isTracingEnabled = false | |||
| private isInitialized = false | |||
| private agentIDE?: CodyIDE | |||
| private extensionAgentVersion?: string | |||
| private clientPlatform?: CodyIDE | |||
There was a problem hiding this comment.
Why name it this? I guess the IDE is a platform in some sense, but I think if you took a poll of cody core devs about what "clientPlatform" means and most would say macOS/Windows/etc.
There was a problem hiding this comment.
I wanted to keep the word client somewhere because we use server for sourcegraph instance and client for whether its Vscode Cody or jetbrains etc. But I see your point now.
But how about we just keep it clientIde or ideClient ? WE might also editorClient. I am not the best with naming so it would be great if you can pick something from these choices as to me they are all VERY close.
There was a problem hiding this comment.
If it is the IDE, then how about private ide: CodyIDE?
It is not ideal because not everything is an IDE, but until we fix the CodyIDE type name, should be an ok field name.
There was a problem hiding this comment.
Even simpler. Let's just do that.
There was a problem hiding this comment.
...I see you're adding the same name to Telemetry. How about editor? It doesn't quite stretch to the command line but it covers everything else well.
Or maybe application?
There was a problem hiding this comment.
I am ambivalent between editor and ide they both mean the same to me. Application wouldn't be good, I think of something foreign.
| private agentIDE?: CodyIDE | ||
| private extensionAgentVersion?: string | ||
| private clientPlatform?: CodyIDE | ||
| private agentVersion?: string |
There was a problem hiding this comment.
This is unclear to me too. Let's pick a better name and consider adding a comment.
First, unclear whether this refers to the IDE version, like v7.65.0-nightly or v1.66.0, or some version of the src/agent, src/vscode, etc. code. "agent" did imply the latter. These days the IDE version is the most useful thing to have. But above all, unclear to me what this is.
Second, we need to give up the term "Agent" for @olafurpg & crew's latest effort. There's no clear replacement term yet, unfortunately. But just saying "agent" is not great.
There was a problem hiding this comment.
For time being, I think the name "Cody Agent" is distinct enough to minimize confusion while avoiding the need for a large rewrite.
There was a problem hiding this comment.
I used agentversion for the same reasons as you described above but I see how that's a confusing naming.
How about we do something much simpler like codyExtensionVersion or extensionVersion?
There was a problem hiding this comment.
Yep, I'm not asking for a rewrite here. But by the same token I don't want to introduce an undifferentiated "agent". Particularly if the name goes to Telemetry logs and we're going to be living with it for a long time.
Part of CODY-4771. Adds agent version and ClientPlatform in OpenTelemetryService.
NOTE: the version string uses pre-release and release versions properly and @abeatrix confirmed that too. The version string is what is actually the value of clientCapabilities.agentExtensionVersion
Test plan
Tested locally with the debugger
sg startsg start otelhttp://localhost:16686/-/debug/jaegerthis shows you the local tracesLook into your specific chat trace(which will give you Client Platform and agent Version )