Skip to content

fix: sync telemetry logger enable states#17693

Open
suzunn wants to merge 1 commit into
eclipse-theia:masterfrom
suzunn:fix-telemetry-logger-enable-states
Open

fix: sync telemetry logger enable states#17693
suzunn wants to merge 1 commit into
eclipse-theia:masterfrom
suzunn:fix-telemetry-logger-enable-states

Conversation

@suzunn

@suzunn suzunn commented Jun 22, 2026

Copy link
Copy Markdown

Problem

TelemetryLogger initialized isUsageEnabled and isErrorsEnabled to true regardless of the current telemetry setting. That means extensions can see telemetry as enabled even when env.isTelemetryEnabled is false, which can cause telemetry senders to initialize unnecessarily.

Solution

I updated TelemetryLogger so its usage and error enable states mirror the current telemetry-enabled state when a logger is created and whenever telemetry is toggled. I also added focused coverage for the initial disabled state, toggling back and forth, and onDidChangeEnableStates notifications.

Fixes #17592.

Testing

  • npx eslint packages\plugin-ext\src\plugin\telemetry-ext.ts packages\plugin-ext\src\plugin\telemetry-ext.spec.ts
  • git diff --check origin/master...HEAD

I also tried to run the local compile/test path, but this Windows environment cannot complete the repository setup cleanly: npm ci fails while building the native drivelist dependency because no Visual Studio C++ workload is installed, and the fallback install leaves some generated Theia workspace references unavailable for package compilation.

@github-project-automation github-project-automation Bot moved this to Waiting on reviewers in PR Backlog Jun 22, 2026
@ndoschek
ndoschek self-requested a review July 2, 2026 11:13

@ndoschek ndoschek left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Thanks for the fix, @suzunn!

Two quick housekeeping items first:

  1. The ECA check is failing. Could you sign the Eclipse Contributor Agreement and make sure the email on your commits matches. Details here: https://github.com/eclipse-theia/theia/blob/master/CONTRIBUTING.md#eclipse-contributor-agreement
  2. Could you also update the PR description to follow our PR template? We rely on the sections for our automated changelog generation. Thanks!

The approach itself looks good and I can verify the issue is fixed using the reproduction steps from the linked issue. 👍

About the setup issues you ran into on Windows, this page might help: https://github.com/eclipse-theia/theia/blob/master/doc/Developing.md#windows. Heads up though, that section is a bit outdated (it still mentions msvs_version 2017); for Node 22 for example you'll want at least Visual Studio 2022 Build Tools with the C++ workload. I'll take care of updating this part! (edit: see also here for the wip: #17752)

@@ -0,0 +1,61 @@
// *****************************************************************************
// Copyright (C) 2026 EclipseSource and others.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

You can put yourself/your company as copyright holder here.

@github-project-automation github-project-automation Bot moved this from Waiting on reviewers to Waiting on author in PR Backlog Jul 2, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

Status: Waiting on author

Development

Successfully merging this pull request may close these issues.

TelemetryLogger.isUsageEnabled/isErrorsEnabled hardcoded to true, ignoring telemetry settings

2 participants