Telemetry support for SPFx Toolkit. Closes #570#643
Telemetry support for SPFx Toolkit. Closes #570#643Saurabh7019 wants to merge 21 commits intopnp:devfrom
Conversation
|
uuuu looks interesting.... I will try to give it a check/review next week unless someone is quicker than me🤩 |
2cc4605 to
4c4858b
Compare
4ca0a6a to
8716fdf
Compare
|
@Saurabh7019 I just did a minor and in this process the dev gets rebased/aligned with main. |
4510320 to
c333458
Compare
There was a problem hiding this comment.
Pull request overview
This PR adds comprehensive telemetry support to the SPFx Toolkit extension to track usage patterns and improve feature prioritization. The implementation uses the official @vscode/extension-telemetry package and respects VS Code's built-in telemetry settings.
Key changes:
- Implemented a centralized
TelemetryServicesingleton with awithTelemetrywrapper for command handlers - Added telemetry tracking for extension lifecycle (activation/deactivation), authentication, and all user-facing actions across Actions, Tasks, and Environment views
- Updated documentation to explain what data is collected and how users can opt out
Reviewed changes
Copilot reviewed 13 out of 15 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/utils/telemetry.ts | New centralized telemetry service with command wrapper and event constants |
| src/extension.ts | Telemetry initialization on activation and disposal on deactivation |
| package.json | Added @vscode/extension-telemetry dependency and hardcoded AI connection string |
| npm-shrinkwrap.json | Lock file updates for new telemetry dependencies |
| src/webview/PnPWebview.ts | Wrapped samples gallery command with telemetry |
| src/services/executeWrappers/TerminalCommandExecuter.ts | Wrapped all terminal commands with telemetry, including npm script tracking |
| src/services/actions/SpfxAppCLIActions.ts | Wrapped app catalog management commands with telemetry |
| src/services/actions/Scaffolder.ts | Wrapped project scaffolding commands with telemetry |
| src/services/actions/IncreaseVersionActions.ts | Wrapped version increment command with telemetry |
| src/services/actions/Dependencies.ts | Wrapped dependency validation/installation commands with telemetry |
| src/services/actions/CopilotActions.ts | Wrapped Copilot integration command with telemetry |
| src/services/actions/CliActions.ts | Wrapped CLI action commands with telemetry |
| src/providers/AuthProvider.ts | Wrapped login/logout commands with telemetry |
| src/panels/CommandPanel.ts | Fixed trailing spaces in action tree item labels |
| docs/src/content/docs/about/telemetry.mdx | Comprehensive documentation on telemetry collection and privacy |
Files not reviewed (1)
- npm-shrinkwrap.json: Language not supported
2f61fa0 to
697927c
Compare
|
@Saurabh7019 you are going to kill me I know. We need to rebase again, sorry for that 😉 |
697927c to
f17759b
Compare
Adam-it
left a comment
There was a problem hiding this comment.
First of all sorry for the huge hold up 🙏
Second of all, awesome work 👏 I left a few comments we could recheck before we proceed
also:
- Shouldn't we also add telemetry.json so that our extension will support retrieving a dump of all telemetry using vsce cli
- Did you maybe checked that the extension does not log anything to telemetry if we switch it off in VS Code?
- Should we consider our own setting for it so that for example someone would like to switch of telemetry only for SPFx Toolkit but leave it on for other extensions?
- Internally, lets open a discussion (best on the related topic on Basecamp) to organize a tenant for SPFx Toolkit with App Insights and how we will maintain it and what costs limits we will set
package.json
Outdated
| ], | ||
| "license": "MIT", | ||
| "main": "./dist/extension.js", | ||
| "aiConnectionString": "InstrumentationKey=137865b8-3555-4866-a2c9-7e35ead3d578;IngestionEndpoint=https://northeurope-2.in.applicationinsights.azure.com/;LiveEndpoint=https://northeurope.livediagnostics.monitor.azure.com/;ApplicationId=d253fe5b-d38c-41f5-841d-afed5bcd9557", |
There was a problem hiding this comment.
What is the idea we have for it. I suggest we keep this empty and add this as a new encrypted secret in the repo which would be only populated before vsce package on the pipeline on release and pre release only.
Any alternatives?
There was a problem hiding this comment.
I suggest we keep this empty and add this as a new encrypted secret in the repo which would be only populated before vsce package on the pipeline on release and pre release only.
This sounds perfect to me. I may need your help to implement it though :)
| "dev": true, | ||
| "license": "MIT" | ||
| }, | ||
| "node_modules/@nevware21/ts-async": { |
There was a problem hiding this comment.
synce this is not a MS maintained package lets run a one time security scan for it
There was a problem hiding this comment.
np audit doesn't flag this.
src/providers/AuthProvider.ts
Outdated
| commands.registerCommand(Commands.login, TelemetryService.withTelemetry(Commands.login, AuthProvider.signIn)) | ||
| ); | ||
| subscriptions.push( | ||
| commands.registerCommand(Commands.logout, AuthProvider.logout) | ||
| commands.registerCommand(Commands.logout, TelemetryService.withTelemetry(Commands.logout, AuthProvider.logout)) |
There was a problem hiding this comment.
TBH i would not track those.
What would be the benefit of knowing this?
I think what is more important is what functionalities are used?
Do users use spfx chat participant, or SPFx move app... if not lets remove those
| if (telemetryService) { | ||
| telemetryService.sendEvent('Extension Deactivated'); | ||
| telemetryService.dispose(); | ||
| } |
There was a problem hiding this comment.
This is cool.
TBH I wonder if, and when does it really happen 🤔. if at all 😜
There was a problem hiding this comment.
It's only appropriate that deactivation is also tracked when activation is.. for the lifecycle. It will happen when VS code is closed or when the extension is uninstalled.
6a2e279 to
a1f018b
Compare
|
2c84a0b to
fd39e59
Compare
|
We just had a new minor release. We should rebase this branch before we proceed |
## 🎯 Aim The scaffolding form is not adapted for all versions of SPFx, and most probably it will take a lot of time until we do that. For now the aim is to adapt the scaffolding options in order to inform the user that they only support latest version of SPFx ## 📷 Result <img width="1536" height="821" alt="image" src="https://github.com/user-attachments/assets/79322abb-dc38-40e8-9039-d74db80a6994" /> ## ✅ What was done - [X] Added notification to the scaffolding form - [X] Removed gulp fast serve - [X] Updated chat participant prompts ## 🔗 Related issue Closes pnp#667 --------- Co-authored-by: Saurabh Tripathi <saurabh7019@gmail.com>
…#701 (pnp#718) ## 🎯 Aim The aim of this PR is to update all `list` related CLI commands used by the LLM to base on CSV command output mode instead of JSON which is more optimal for LLM and reducses the amount of used context window. ## 📷 Result <img width="1536" height="817" alt="image" src="https://github.com/user-attachments/assets/b94bfd13-b8f4-4f6b-936d-25736e46d1e6" /> ## ✅ What was done - [X] updates output mode to CSV ## 🔗 Related issue Closes: pnp#701 --------- Co-authored-by: Saurabh Tripathi <saurabh7019@gmail.com>
## 🎯 Aim The aim of this PR clean up all left usage of yo-rc.json file so that SPFx Toolkit is ready for newer versions of SPFx ## 📷 Result <img width="1536" height="820" alt="image" src="https://github.com/user-attachments/assets/7b884b3b-0f2c-43b0-866e-b1ec1b478e24" /> ## ✅ What was done - [X] Removes ACE checker - [X] Refactors using yo-rc.json for getting solution name in ci-cd action to get the name from package.json ## 🔗 Related issue Closes: pnp#711
…np#716) ## 🎯 Aim The aim of this PR is to refactor the old `/setup` chat participant command to a dedicated Language Model Tool that will validate and install all needed dependencies for any version of SharePoint Framework ## 📷 Result <img width="1536" height="816" alt="image" src="https://github.com/user-attachments/assets/8d1205a2-0670-4690-ba9b-6d364a655d13" /> <img width="485" height="509" alt="image" src="https://github.com/user-attachments/assets/6ec798cb-4d31-4aa3-8f21-249d34757021" /> <img width="505" height="454" alt="image" src="https://github.com/user-attachments/assets/82341651-ce54-4cc3-b780-d1e41c31452b" /> <img width="492" height="636" alt="image" src="https://github.com/user-attachments/assets/ea7152bd-ea80-4051-8bd9-cd390dc591a2" /> <img width="504" height="438" alt="image" src="https://github.com/user-attachments/assets/0a808371-4ba7-40b5-8b4d-067fb7045d2d" /> <img width="501" height="652" alt="image" src="https://github.com/user-attachments/assets/1c802146-9e2e-425c-b406-4b23a4c27501" /> ## ✅ What was done - [X] Removed the `/setup` GitHub Copilot Chat command - [X] Added SPFx setup environment Language Model Tool ## 🔗 Related issue Closes: pnp#706 --------- Co-authored-by: Saurabh Tripathi <saurabh7019@gmail.com>
## 🎯 Aim This PR fixes issues in `prepare-sample-data.ps1` script to ensure all ACE samples are included in the `sp-dev-fx-samples.json`. ## ✅ What was done - [X] Better path handling for `.yo-rc.json` - [X] Appends results to `$samples` instead of overwriting `$output` ## 🔗 Related issue Closes: pnp#723
… (pnp#691) ## 🎯 Aim This PR fixes the `nvm use` error on Windows when creating a new SPFx project via the sample gallery. On Windows, `nvm-windows` does not automatically read the `.nvmrc`, so the command fails without a version argument. The fix parses the `.nvmrc` file and sends `nvm use <version>` with the version number for `nvm` users.. for `nvs` users it checks for both `.nvmrc` and `.node-version` files and sends `nvs use` which works on all platforms. ## 📷 Result NA ## ✅ What was done - [X] modified `createTerminal` method in `TerminalCommandExecuter.ts` as agreed in the issue ## 🔗 Related issue Closes: pnp#519
pre-release 4.16.1
…#726) ## 🎯 Aim The aim is to totally remove all related code regarding the /info SPFx Chat Participant command and, in the documentation, replace it with guidance recommending the use of the CLI for the Microsoft 365 MCP server instead. ## 📷 Result <img width="1380" height="902" alt="image" src="https://github.com/user-attachments/assets/41335670-ee0c-41a2-99fd-bc0dad5f512c" /> ## ✅ What was done - [X] Removed all code related to the /info SPFx Chat Participant command - [X] Cleaned up any references, registrations, and configuration linked to the /info command - [X] Updated documentation to recommend using the CLI for Microsoft 365 MCP server for tenant info and management ## 🔗 Related issue Closes: pnp#708 --------- Co-authored-by: Adam-it <adam.wojcik.it@gmail.com>
…np#697) ## 🎯 Aim The aim is to add a new action that will validate user local setup for the currently open SPFx project. For example someone opens some old SPFx sample that uses SPFx 1.16 and would like to recheck if the current version of node and local dependencies are valid for that project. The aciton uses `spfx doctor` command to get the report and similar like the setup action will install the correct version of node and global dependencies if needed based on the compatibility matrix ## 📷 Result <img width="1536" height="815" alt="image" src="https://github.com/user-attachments/assets/22d601c8-8b57-403b-84e5-33bb3cb4aab2" /> <img width="1536" height="810" alt="image" src="https://github.com/user-attachments/assets/fcd31e92-ee89-4cc2-a4e9-abdd631f22c7" /> ## ✅ What was done - [X] Added new action - [ ] Updated docs 👉// TODO ## 🔗 Related issue Closes: pnp#518 --------- Co-authored-by: Saurabh Tripathi <saurabh7019@gmail.com>
## 🎯 Aim I noticed we are missing some docs about recently added langauge model tools and this PR is suppose to fill this gap
## 🎯 Aim I know the related issue was still not agreed but this one is quite easy and hits on my nerve .😁 What I usually do is move the SPFx Toolkit Task Pane with heft and npm actions to the Explorer view so the tasks sit right along my files I modified. The problem is that this panel does not show up until the extension gets active so this happens only when we go to the SPFx Toolkit view. So this means every time I opened a SPFx project I had to go to the SPFx Toolkit view and then go back to the explorer view just to see the task panel. This is quite frustrating. Folks might have customized the look and feel of VS Code, like moving the login and management views to some other part of VS Code, In this case they would also need to go to the view to make them active. That Is why I propose to just check if a project has a config\package-solution.json file, and if so, we may assume it is a SPFx project and make the extension active. If it will still not be an SPFx project, this will be caught by the IsSpfx helper method in CommandPanels and will just load the SPFx Toolkit with the functionality when you are not in a SPFx project ## 📷 Result <img width="684" height="788" alt="image" src="https://github.com/user-attachments/assets/bc8f7617-93b4-496d-803c-3a970457c254" /> ## 🔗 Related issue Closes: pnp#733
## 🎯 Aim The aim is to fix the way we handle MSAL refresh token. Currently, in the extension, after we sign in, we are more or less logged in 1 hour and after that the access token expires, and we need to perform sign in again. I noticed it was due to a simple bugi and the main origin and fix up was done and already published in CLI SPFx Toolkit npm package. No the token should allow to retain sign in from 4-90 days (depending on tenant settings) In order to test this PR is best 1. to uninstall SPFx Toolkit if you have it. 2. Then use `vsce package` to build a local vscix package and install SPFx Toolkit from your local package. 3. Next perform sign in 4. Wait for 1-2 hours and restart VS Code. You should still be signed in ## 🔗 Related issue Closes: pnp#500
|
we had a new minor release again. Maybe lets keep this PR as draft until I setup a tenant for it and make the changes. |
🎯 Aim
This PR adds telemetry support for SPFx Toolkit to help us understand how the extension is used and improve its features.
📷 Result
App Insights

CSV report

✅ What was done
vscode.openactions✅ How to test
🔗 Related issue
Closes: #570