add tsserver version property to every event#37066
Conversation
|
This test was failing locally (Ubuntu 17.04) on some ipc tests, but the same tests are failing in |
| } | ||
| } | ||
|
|
||
| constructor(private client: TypeScriptServiceClient) { |
There was a problem hiding this comment.
This creates a circular dependency between the reporter and the service client. Can we either pass the version info in logTelemetry or make TelemetryReporter take an interface instead of the entire client
There was a problem hiding this comment.
Exposed the client version through a new IClientVersion interface placed in typescriptServiceClient.ts. So a circular dependency remains. This is preferable to passing the version explicitly because other services may need to report telemetry in the future, and it would be inconvenient to force them to have a handle on the tsserver version.
| } | ||
|
|
||
| /* __GDPR__ | ||
| "typingsInstalled" : { |
There was a problem hiding this comment.
The version field will need to be added to the gdpr annotations as well.
@kieferrm Would __GDPR__COMMON__ work for this?
There was a problem hiding this comment.
GDPR__COMMON means that this is a property that is on every single telemetry event we send. The TS server version is only on TS related events, so it is not a common property.
There was a problem hiding this comment.
Maybe this can be done with GDPR__FRAGMENT and a wildcard to match on the name? Or would we need to extend the comment-handling functionality?
|
Thanks. Getting this in for our October release |
Adds a property to all typescript telemetry events indicating the version of tsserver in the currently running instance of tsserver.
There are two sources of the version of tsserver:
--enableTelemetry, which only occurs if we decided the APIVersion was >=2.0.8. If we could recover the version from the package.json, then tsserver's reported version should match the API Version unless tsserver was overwritten.Since the tsserver version is already reported (for recent tsserver versions), we could in principle join the session of an event we are interested in, say when a crash or exception occurs in tsserver, to the corresponding
projectInfoevent and recover the version of tsserver running. However, this has a couple issues:projectInfoevent fires, we get no version info.Open questions
versionfrom projectinfo?versionis a term that could potentially be overloaded. On the other hand,versionis already used by theprojectInfoevent, and we should (and in this implmentation, do) add the same property to every event, includingprojectInfo. In the PR, theversionproperty we append to every event will coincide exactly with theversionfromprojectInfo.TODO