Skip to content

Move telemetry initialization out of our UI-thread bound helper#79282

Merged
jasonmalinowski merged 2 commits intodotnet:mainfrom
jasonmalinowski:simplify-telemetry-service-initialization
Jul 8, 2025
Merged

Move telemetry initialization out of our UI-thread bound helper#79282
jasonmalinowski merged 2 commits intodotnet:mainfrom
jasonmalinowski:simplify-telemetry-service-initialization

Conversation

@jasonmalinowski
Copy link
Member

This isn't needed anymore.

telemetryService.InitializeTelemetrySession(TelemetryService.DefaultSession, logDelta);

Logger.Log(FunctionId.Run_Environment, KeyValueLogMessage.Create(
static m => m["Version"] = FileVersionInfo.GetVersionInfo(typeof(VisualStudioWorkspace).Assembly.Location).FileVersion));
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

dumb question. but why do any work in the constructor? wouldn't it be btter to minimize that?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Possibly yes, we might do another change here. Right now I'm focusing on getting stuff explicitly off the UI thread (and if this constructor is running there, fix whatever is doing that...) and if this is still slow during load we can address that further.

var telemetryService = (VisualStudioWorkspaceTelemetryService)Services.GetRequiredService<IWorkspaceTelemetryService>();
telemetryService.InitializeTelemetrySession(TelemetryService.DefaultSession, logDelta);

Logger.Log(FunctionId.Run_Environment, KeyValueLogMessage.Create(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Run_Environment

I have a dumb question too: Is this telemetry valuable? If so, would it be sufficient to log assembly version instead? (I think the current code will do a File.Exists and other file io)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The assembly version is just something like 17.14.0.0, so you won't have the individual version there.

There's two issues here:

1. Some tests of the VS layer didn't have the telemetry service in the
   first place.
2. There was now a circularity between the VisualStudioWorkspace and
   VisualStudioWorkspaceTelemetryService, so a Lazy has been added.
@jasonmalinowski jasonmalinowski merged commit 7e36f8d into dotnet:main Jul 8, 2025
25 checks passed
@jasonmalinowski jasonmalinowski deleted the simplify-telemetry-service-initialization branch July 8, 2025 23:30
@dotnet-policy-service dotnet-policy-service bot added this to the Next milestone Jul 8, 2025
@RikkiGibson RikkiGibson modified the milestones: Next, 18.0 P1 Aug 19, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants