From 70872532ec9fe1ee473b9205db4bde02644b067c Mon Sep 17 00:00:00 2001 From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com> Date: Thu, 30 Jul 2026 12:17:42 -0400 Subject: [PATCH] Move Lite's data out of the installer's install root (#1832) Lite's data root was %LOCALAPPDATA%\PerformanceMonitorLite, which is also Velopack's install root. Re-running Setup.exe over an existing install renames that folder aside and deletes it, so an installer-based upgrade destroyed the DuckDB store, the Parquet archive, the logs, and settings.json before the new build ever ran. In-app updates were always safe (they replace current\ in place), which is why the loss looked random. Data now lives in the sibling %LOCALAPPDATA%\PerformanceMonitorLite-Data and DataRootMigration moves it there on first start, before anything reads settings or opens the store. It is an explicit allow-list, not "move the folder contents": Velopack's own Update.exe, current\ and packages\ share the old folder and must stay. Nothing in the new root is overwritten and nothing is deleted; a DATA-MOVED.txt signpost is left behind. Second defect, same failure: App.LoadAlertSettings read the four webhook secrets from Credential Manager at the tail of its settings.json parse, behind the "no settings.json" early exit. With settings.json gone the Settings window's webhook boxes rendered empty while the credentials still existed, and Save wrote the blanks back through SaveWebhookUrl, which deletes on blank. The reads now run first and unconditionally. Also: one tray balloon per launch when an update is found, pointing at Help > About (the safe in-place route); the title-bar suffix was the only signal. README, docs, and the bug-report template updated to the new path, with an explicit warning that upgrading a pre-fix install via Setup.exe still loses data one final time. Co-Authored-By: Claude Fable 5 --- .github/ISSUE_TEMPLATE/bug_report.yml | 2 +- CHANGELOG.md | 13 + .../AlertSettingsCredentialLoadTests.cs | 168 ++++++++++ Lite.Tests/AppAlertSettingsTests.cs | 5 + Lite.Tests/DataRootMigrationTests.cs | 305 ++++++++++++++++++ Lite/App.xaml.cs | 78 +++-- Lite/MainWindow.xaml.cs | 17 + Lite/Services/ConfigSeeder.cs | 2 +- Lite/Services/DataRootMigration.cs | 279 ++++++++++++++++ Lite/Services/IgnoredWaitTypes.cs | 2 +- README.md | 12 +- docs/how-collection-works.md | 2 +- 12 files changed, 857 insertions(+), 28 deletions(-) create mode 100644 Lite.Tests/AlertSettingsCredentialLoadTests.cs create mode 100644 Lite.Tests/DataRootMigrationTests.cs create mode 100644 Lite/Services/DataRootMigration.cs diff --git a/.github/ISSUE_TEMPLATE/bug_report.yml b/.github/ISSUE_TEMPLATE/bug_report.yml index d6254da86d..841abd0aec 100644 --- a/.github/ISSUE_TEMPLATE/bug_report.yml +++ b/.github/ISSUE_TEMPLATE/bug_report.yml @@ -85,7 +85,7 @@ body: id: errors attributes: label: Error Messages / Log Output - description: Paste any error messages or relevant log entries. Dashboard logs are in %LOCALAPPDATA%\PerformanceMonitor\Logs\; Lite logs are in %LOCALAPPDATA%\PerformanceMonitorLite\Logs\. + description: Paste any error messages or relevant log entries. Dashboard logs are in %LOCALAPPDATA%\PerformanceMonitor\Logs\; Lite logs are in %LOCALAPPDATA%\PerformanceMonitorLite-Data\logs\ (on versions before the data-root move, %LOCALAPPDATA%\PerformanceMonitorLite\logs\). render: text validations: required: false diff --git a/CHANGELOG.md b/CHANGELOG.md index 6c7f67e7bd..97664c1e01 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ## [Unreleased] +### Important + +- **Lite's data now lives OUTSIDE the install directory - and on any version older than this one, upgrading with `Setup.exe` destroys it** ([#1832]) - Lite kept everything in `%LOCALAPPDATA%\PerformanceMonitorLite`, which is also the installer's own install root. Re-running `Setup.exe` over an existing install renames that folder aside and deletes it, so an installer-based upgrade took the DuckDB store, the Parquet archive, the logs, and `settings.json` with it, in one move, before any code from the new build ran. **In-app updates were always safe** - Help > About replaces the `current\` subfolder in place and never touches the data sitting beside it - which is precisely why the loss looked random instead of reproducible. Two things always survived and still do: the monitored-server list (`%ProgramData%\PerformanceMonitorLite\config\servers.json`, machine-wide) and every password and webhook URL in Windows Credential Manager. + + **The one-time caveat: reaching THIS version via `Setup.exe` still loses your data.** The installer deletes the old directory before the new build starts, so no fix shipping inside the new build can prevent it - the code that would migrate your data has not run yet when the data is deleted. Upgrade **in place** instead and nothing is lost: **Help > About** downloads and applies the update, or extract the portable ZIP over your existing copy. From this version forward `Setup.exe` is safe, because the data is no longer where the installer writes. + + Data now lives in the sibling `%LOCALAPPDATA%\PerformanceMonitorLite-Data`, and the first start MOVES `config\`, `archive\`, `logs\`, `monitor.duckdb` (plus its WAL) and `alert_state.json` there from the old location automatically - a same-volume rename, so a multi-GB store transfers instantly. Nothing already in the new location is overwritten, nothing anywhere is deleted, and a `DATA-MOVED.txt` signpost is left behind pointing at the new path. The move is per-artifact rather than all-or-nothing, so a run interrupted by a locked store file finishes itself on the next start rather than stranding the store in a folder the next installer run would delete. The updater's own `Update.exe`, `current\` and `packages\` share that old folder and are deliberately left where they are. + ### Added - **A blocking alert that fires on total blocked WAIT TIME, not just how many sessions are blocked** ([#1839]) - the existing gate counts blocked-process events in a rolling window, which cannot tell one session blocked for an hour from one blocked for a second: the count is 1 either way. `blockingWaitSecondsThreshold` (Lite: Settings > Alerts, "Total blocked wait"; Darling: `darling.json` + the Viewer's Settings window) sums `wait_time_ms` across the rows of the LATEST blocking snapshot and fires when that total reaches the threshold. **Defaults to 0 = off in both SKUs**, so nothing starts alerting because you took an update. It is a SNAPSHOT sum rather than a windowed one on purpose - the alert answers "how much aggregate blocked wait is happening right now", and summing a window would accumulate blocking that has already ended and could never clear while the window still held it. Unlike the count gate's rolling-window edge trigger it is LEVEL-triggered, the shape the High CPU check already uses: it re-fires every cooldown while the wait stays above the threshold and resolves with "Blocking Wait Cleared" when it drops below, so standing blocking keeps announcing itself instead of going quiet after one alert. It reports under its own "Blocking Wait Time" metric rather than reusing "Blocking Detected", so mute rules, cooldowns, and history rows for the two gates never tangle - but it carries the same blocked-process incident content, built from the rows the sweep already fetched for the count gate rather than a second query. A STALE snapshot neither fires it nor holds it active ([#1812]'s rule, at the blocking collector's cadence): a stopped collector leaves a "latest" snapshot that would otherwise read as NOW, and a level-triggered alert on frozen rows would re-fire forever - staleness resolves rather than latching, the opposite of the running-jobs treatment, because for a level condition the worse failure is staying latched on data of unknown age. The value travels as a real number ([#1830]), never parsed back out of the display text. Both gates stay under the one `blockingEnabled` toggle. Darling stores migrate to **schema v40** (one additive `NOT NULL DEFAULT 0` column) - automatic, no operator action. Two adjacent corrections ride along: DuckDB widens `SUM(BIGINT)` to HUGEINT, which the .NET driver hands back as a `BigInteger` that `Convert.ToInt64` cannot convert, so the read is cast in SQL on both stores (caught by the new round-trip test, and it would have thrown at alert-check time); and Lite's MCP `get_alert_settings` had been reporting the blocking COUNT under the key `threshold_seconds` - a name copied from the deprecated Dashboard, whose blocking threshold genuinely is seconds - which is now `threshold_count` beside the new `wait_threshold_seconds`. @@ -17,6 +25,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 ### Fixed +- **Opening the Settings window after a data loss no longer deletes the webhook URLs that survived it** ([#1832]) - `App.LoadAlertSettings` read the Teams, Slack, and generic webhook secrets out of Credential Manager at the TAIL of its `settings.json` parse, behind an `if (!File.Exists(path)) return;` early exit. With `settings.json` gone - exactly the state the install-directory wipe above produced - the load returned before those four reads ever happened, so the Settings window's webhook boxes rendered EMPTY while the credentials were still sitting in the store. Clicking Save then wrote those blanks back through `SaveWebhookUrl`, which DELETES the credential when handed a blank: the secrets that had survived the data loss were destroyed by looking at the settings window afterwards. The credential reads now run FIRST and unconditionally, before the early exit, so the boxes populate from Credential Manager whether or not `settings.json` exists and Save round-trips what it displayed. The Save path is otherwise unchanged - blank still means delete, which is how you clear a webhook on purpose. A settings-file-absent test pins that all four keys are read. + +- **A tray notification when an update is available, not just a title-bar suffix** ([#1832]) - the startup update check appended "Update vX available (Help > About)" to the window title and did nothing else, and a title bar nobody reads is how installs both stayed on old builds and got "upgraded" by re-downloading `Setup.exe` - the one path that deleted their data. The check now also raises one tray balloon per launch pointing at Help > About and saying that route updates in place and keeps your data. Same check, same cadence, one more surface; the update UX is otherwise untouched. + - **Top Procedures collects on Azure SQL Database** ([#1833]) - `procedure_stats` was one of two database-scoped collectors that never opted into the Azure per-database connection path: it ran once on the server entry's own connection - whose catalog defaults to `master` when the entry's Database field is blank - and its Azure variant's `WHERE s.database_id = DB_ID()` then matched only master's procedures. Zero user rows, logged `SUCCESS`, empty Top Procedures grid that looked healthy, while Top Queries (which does run per database) kept working right next to it - the asymmetry that fingerprinted the bug. The collector now declares `RunsPerDatabase` on Azure like `query_stats` and its five database-scoped siblings, and the per-database connection makes `DB_ID()` mean each user database in turn - exactly what that predicate was written for. Both Lite and Darling get it (shared definition). The other collector with the same gap is Query Store, which needs a per-database rework on Azure rather than a one-line override - tracked as [#1836], with the invisible-zero-rows Collection Health gap that hid both defects tracked as [#1837]. - **Chart x-axis timestamps now honor the Local/Server/UTC time toggle** ([#1831]) - Lite plots chart X values in server time and converts for display at render, but the ONE render surface doing the axis labels - the single shared `LabelFormatter` all three apps reach - never consulted the display mode, so every chart's bottom axis showed server time no matter what the toggle said, while grids, slicers, tooltips, and the crosshair all converted around it. Refresh and reconnect could not help: they re-plotted the same unconverted labels. The formatter now converts each label through the `UiTimeContext` hook Lite wired at startup for exactly this purpose and never used here; the Darling Viewer pre-converts its plotted X and deliberately leaves the hook at identity, so the change is a no-op there by construction - the double-conversion trap a Viewer-style port into Lite would have hit is documented at the site and pinned by test. Three companions ride along, all the same defect in different clothes: the display-mode toggle now re-plots the charts immediately (it refreshed nine grids and six slicers and zero charts, so even the fixed formatter only showed on the next data cycle); the Queries heatmap's hand-built axis labels convert like that chart's own tooltip always did; and the three history windows switch to the shared formatter, gaining both the conversion and the date-change labels. The deprecated Dashboard's startup hack that force-pinned the dropdown to ServerTime ("charts always render in server time") is retired - and its saved preference, which was written on every change and read never, is finally restored at startup. New tests pin the formatter's conversion, the Viewer's identity no-op, and the display-date boundary behavior - nothing anywhere tested the formatter before. @@ -1975,3 +1987,4 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 [#1837]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1837 [#1830]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1830 [#1839]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1839 +[#1832]: https://github.com/erikdarlingdata/PerformanceMonitor/issues/1832 diff --git a/Lite.Tests/AlertSettingsCredentialLoadTests.cs b/Lite.Tests/AlertSettingsCredentialLoadTests.cs new file mode 100644 index 0000000000..9f66579267 --- /dev/null +++ b/Lite.Tests/AlertSettingsCredentialLoadTests.cs @@ -0,0 +1,168 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor Lite. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.Collections.Generic; +using System.IO; +using PerformanceMonitorLite; +using Xunit; + +namespace PerformanceMonitorLite.Tests; + +/// +/// #1832 second defect: webhook secrets live in Windows Credential Manager, never in settings.json, but +/// used to read them at the TAIL of its settings.json parse - behind +/// the "no settings.json, nothing to do" early exit. So on an install whose settings.json had been +/// destroyed (which is exactly what re-running Setup.exe over the old data root produced), the Settings +/// window's webhook boxes rendered EMPTY while the credentials were still in the store, and Save wrote the +/// blanks back through SaveWebhookUrl - which DELETES on blank. Looking at the settings window finished off +/// the secrets that had survived the data loss. +/// +/// The reads must therefore happen regardless of settings.json. That is a wiring property: both halves +/// work fine in isolation and the bug lives entirely in the order they run in, so only a test that removes +/// settings.json and watches which credential keys get requested can see it. +/// +/// Shares a collection with because both write the App webhook statics +/// and xUnit runs separate classes in parallel. +/// +[Collection("app-webhook-statics")] +public class AlertSettingsCredentialLoadTests +{ + private static readonly string[] ExpectedKeys = + { "TeamsWebhook", "SlackWebhook", "GenericWebhook", "GenericWebhookHeaders" }; + + private static string NewTempDir(string tag) + { + var dir = Path.Combine(Path.GetTempPath(), $"pmlite_{tag}_{Guid.NewGuid():N}"); + Directory.CreateDirectory(dir); + return dir; + } + + /// + /// The credential WRITER for every case that must not write. The real one edits Windows Credential + /// Manager for the user running the suite, so a case that unexpectedly reaches the #1506 legacy + /// migration would overwrite a live webhook URL rather than fail a test. + /// + private static void FailOnWrite(string key, string value) => + Assert.Fail($"LoadAlertSettings wrote credential '{key}' when nothing should have been saved."); + + /// + /// The regression itself: no settings.json, and all four secrets are still read and still land on the + /// statics the Settings window binds its boxes to. + /// + [Fact] + public void LoadAlertSettings_ReadsWebhookCredentials_WhenSettingsJsonIsMissing() + { + var configDir = NewTempDir("nosettings"); + try + { + Assert.False(File.Exists(Path.Combine(configDir, "settings.json"))); + + var requested = new List(); + App.LoadAlertSettings(configDir, key => { requested.Add(key); return "stored:" + key; }, FailOnWrite); + + foreach (var key in ExpectedKeys) + { + Assert.Contains(key, requested); + } + + /* SettingsWindow populates its boxes straight off these, so a blank here IS the blank box that + Save then wrote back as a delete. */ + Assert.Equal("stored:TeamsWebhook", App.TeamsWebhookUrl); + Assert.Equal("stored:SlackWebhook", App.SlackWebhookUrl); + Assert.Equal("stored:GenericWebhook", App.GenericWebhookUrl); + Assert.Equal("stored:GenericWebhookHeaders", App.GenericWebhookHeadersJson); + } + finally + { + Directory.Delete(configDir, true); + } + } + + /// + /// A settings.json that exists but is corrupt takes the catch, which is the same trap one layer in. + /// + [Fact] + public void LoadAlertSettings_ReadsWebhookCredentials_WhenSettingsJsonIsCorrupt() + { + var configDir = NewTempDir("corrupt"); + try + { + File.WriteAllText(Path.Combine(configDir, "settings.json"), "{ this is not json"); + + var requested = new List(); + App.LoadAlertSettings(configDir, key => { requested.Add(key); return "stored:" + key; }, FailOnWrite); + + foreach (var key in ExpectedKeys) + { + Assert.Contains(key, requested); + } + + Assert.Equal("stored:TeamsWebhook", App.TeamsWebhookUrl); + } + finally + { + Directory.Delete(configDir, true); + } + } + + /// + /// The path that always worked keeps working: a real settings.json still gets parsed, and the secrets + /// still come from the credential store rather than the file. + /// + [Fact] + public void LoadAlertSettings_StillParsesSettings_AndStillPrefersTheCredentialStore() + { + var configDir = NewTempDir("present"); + try + { + File.WriteAllText( + Path.Combine(configDir, "settings.json"), + "{\"teams_webhook_enabled\":true,\"teams_proxy_address\":\"proxy.sentinel.test\"}"); + + App.TeamsWebhookEnabled = false; + App.LoadAlertSettings(configDir, key => "stored:" + key, FailOnWrite); + + Assert.True(App.TeamsWebhookEnabled); + Assert.Equal("proxy.sentinel.test", App.TeamsProxyAddress); + Assert.Equal("stored:TeamsWebhook", App.TeamsWebhookUrl); + } + finally + { + Directory.Delete(configDir, true); + } + } + + /// + /// The #1506 legacy migration is unchanged by the hoist: a plaintext URL still in settings.json is + /// written to the credential store AND becomes the live value, which is what the old tail read-back + /// produced. The injected reader returns a different value, so a regression to "the stored value wins" + /// is visible rather than coincidentally equal. + /// + [Fact] + public void LoadAlertSettings_LegacyPlaintextUrl_StillOverridesTheStoredValue() + { + var configDir = NewTempDir("legacy"); + try + { + File.WriteAllText( + Path.Combine(configDir, "settings.json"), + "{\"teams_webhook_url\":\"https://legacy.sentinel.test/hook\"}"); + + var written = new List<(string Key, string Value)>(); + App.LoadAlertSettings(configDir, key => "stored:" + key, (key, value) => written.Add((key, value))); + + Assert.Equal(("TeamsWebhook", "https://legacy.sentinel.test/hook"), Assert.Single(written)); + Assert.Equal("https://legacy.sentinel.test/hook", App.TeamsWebhookUrl); + } + finally + { + Directory.Delete(configDir, true); + } + } +} diff --git a/Lite.Tests/AppAlertSettingsTests.cs b/Lite.Tests/AppAlertSettingsTests.cs index a5821417c5..5c82d98137 100644 --- a/Lite.Tests/AppAlertSettingsTests.cs +++ b/Lite.Tests/AppAlertSettingsTests.cs @@ -26,7 +26,12 @@ namespace PerformanceMonitorLite.Tests; /// in parallel — touching the shared analysis statics here could flake that class. /// The pass-through contract is type-uniform (every member is `=> App.X`), so the /// SMTP/webhook coverage proves the invariant for the analysis members too. +/// +/// Shares a collection with for the same reason: that class +/// drives App.LoadAlertSettings, which writes the four webhook statics this one asserts on, and xUnit runs +/// separate classes in parallel. /// +[Collection("app-webhook-statics")] public class AppAlertSettingsTests { /// diff --git a/Lite.Tests/DataRootMigrationTests.cs b/Lite.Tests/DataRootMigrationTests.cs new file mode 100644 index 0000000000..bd9ac42374 --- /dev/null +++ b/Lite.Tests/DataRootMigrationTests.cs @@ -0,0 +1,305 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor Lite. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.Collections.Generic; +using System.IO; +using PerformanceMonitorLite.Services; +using Xunit; + +namespace PerformanceMonitorLite.Tests; + +/// +/// #1832: Lite's data root used to BE the installer's install root, so re-running Setup.exe deleted the +/// store, the archive, the logs, and settings.json. The data now lives in a sibling directory and startup +/// moves it there once. +/// +/// Two invariants carry the whole fix and both are easy to break by accident: the migration must never +/// overwrite anything already in the new root (that would destroy the data of anyone who already +/// migrated), and it must never move the updater's own files out of the install root (that would break +/// the installed app). Everything below is a temp-directory exercise of one of those. +/// +public class DataRootMigrationTests +{ + private static string NewTempDir(string tag) + { + var dir = Path.Combine(Path.GetTempPath(), $"pmlite_{tag}_{Guid.NewGuid():N}"); + Directory.CreateDirectory(dir); + return dir; + } + + /// Writes a file, creating any missing parent directories. + private static void WriteFile(string root, string relativePath, string content) + { + var full = Path.Combine(root, relativePath); + Directory.CreateDirectory(Path.GetDirectoryName(full)!); + File.WriteAllText(full, content); + } + + /// The shape of a real pre-fix install: our data sitting alongside the updater's files. + private static void PopulateLegacyRoot(string root) + { + WriteFile(root, Path.Combine("config", "settings.json"), "{\"alerts_enabled\":true}"); + WriteFile(root, Path.Combine("config", "servers.json"), "[]"); + WriteFile(root, Path.Combine("archive", "2026-07.parquet"), "archive-bytes"); + WriteFile(root, Path.Combine("logs", "lite_20260729.log"), "log-line"); + WriteFile(root, "monitor.duckdb", "duckdb-bytes"); + WriteFile(root, "monitor.duckdb.wal", "wal-bytes"); + WriteFile(root, "alert_state.json", "{}"); + + /* The updater's files. These must survive untouched. */ + WriteFile(root, "Update.exe", "velopack"); + WriteFile(root, "velopack.log", "velopack"); + WriteFile(root, Path.Combine("current", "PerformanceMonitorLite.exe"), "app"); + WriteFile(root, Path.Combine("packages", "PerformanceMonitorLite-3.3.0-full.nupkg"), "pkg"); + } + + private static void AssertUpdaterFilesIntact(string legacyRoot) + { + Assert.True(File.Exists(Path.Combine(legacyRoot, "Update.exe"))); + Assert.True(File.Exists(Path.Combine(legacyRoot, "velopack.log"))); + Assert.True(File.Exists(Path.Combine(legacyRoot, "current", "PerformanceMonitorLite.exe"))); + Assert.True(File.Exists(Path.Combine(legacyRoot, "packages", "PerformanceMonitorLite-3.3.0-full.nupkg"))); + } + + /// A fresh install: nothing was ever written to the old location. + [Fact] + public void Migrate_IsANoOp_WhenTheLegacyRootDoesNotExist() + { + var parent = NewTempDir("noold"); + try + { + var legacy = Path.Combine(parent, "PerformanceMonitorLite"); + var target = Path.Combine(parent, "PerformanceMonitorLite-Data"); + var log = new List(); + + var result = DataRootMigration.Migrate(legacy, target, log.Add); + + Assert.Equal(DataRootMigrationStatus.NothingToMigrate, result.Status); + Assert.Empty(result.Moved); + Assert.Empty(log); + /* The migration does not create the new root; App does that immediately afterwards. */ + Assert.False(Directory.Exists(target)); + } + finally + { + Directory.Delete(parent, true); + } + } + + /// + /// An install that has already been migrated once, or a Setup.exe install that never ran the old + /// build: the install root exists and holds updater files, but none of ours. + /// + [Fact] + public void Migrate_IsANoOp_WhenTheLegacyRootHoldsOnlyUpdaterFiles() + { + var parent = NewTempDir("updateronly"); + try + { + var legacy = Path.Combine(parent, "PerformanceMonitorLite"); + var target = Path.Combine(parent, "PerformanceMonitorLite-Data"); + WriteFile(legacy, "Update.exe", "velopack"); + WriteFile(legacy, Path.Combine("current", "PerformanceMonitorLite.exe"), "app"); + var log = new List(); + + var result = DataRootMigration.Migrate(legacy, target, log.Add); + + Assert.Equal(DataRootMigrationStatus.NothingToMigrate, result.Status); + Assert.False(Directory.Exists(target)); + Assert.True(File.Exists(Path.Combine(legacy, "Update.exe"))); + Assert.True(File.Exists(Path.Combine(legacy, "current", "PerformanceMonitorLite.exe"))); + } + finally + { + Directory.Delete(parent, true); + } + } + + /// + /// The upgrade case this whole change exists for: a populated old root, a new root that does not + /// exist yet. Everything of ours moves, the updater's files stay, and a signpost is left behind. + /// + [Fact] + public void Migrate_MovesEveryOwnedArtifact_AndLeavesTheUpdaterAlone() + { + var parent = NewTempDir("move"); + try + { + var legacy = Path.Combine(parent, "PerformanceMonitorLite"); + var target = Path.Combine(parent, "PerformanceMonitorLite-Data"); + PopulateLegacyRoot(legacy); + var log = new List(); + + var result = DataRootMigration.Migrate(legacy, target, log.Add); + + Assert.Equal(DataRootMigrationStatus.Migrated, result.Status); + Assert.Empty(result.Failed); + + /* Arrived, with contents. */ + Assert.Equal("duckdb-bytes", File.ReadAllText(Path.Combine(target, "monitor.duckdb"))); + Assert.Equal("wal-bytes", File.ReadAllText(Path.Combine(target, "monitor.duckdb.wal"))); + Assert.Equal("{}", File.ReadAllText(Path.Combine(target, "alert_state.json"))); + Assert.Equal("{\"alerts_enabled\":true}", File.ReadAllText(Path.Combine(target, "config", "settings.json"))); + Assert.Equal("[]", File.ReadAllText(Path.Combine(target, "config", "servers.json"))); + Assert.Equal("archive-bytes", File.ReadAllText(Path.Combine(target, "archive", "2026-07.parquet"))); + Assert.Equal("log-line", File.ReadAllText(Path.Combine(target, "logs", "lite_20260729.log"))); + + /* Departed. A move, not a copy - a leftover store would be deleted by the next Setup.exe run + and would also double the disk footprint of a multi-GB store. */ + Assert.False(File.Exists(Path.Combine(legacy, "monitor.duckdb"))); + Assert.False(Directory.Exists(Path.Combine(legacy, "config"))); + Assert.False(Directory.Exists(Path.Combine(legacy, "archive"))); + Assert.False(Directory.Exists(Path.Combine(legacy, "logs"))); + + /* The old root itself is NOT deleted - the updater owns it. */ + Assert.True(Directory.Exists(legacy)); + AssertUpdaterFilesIntact(legacy); + + var marker = Path.Combine(legacy, DataRootMigration.MarkerFileName); + Assert.True(File.Exists(marker)); + Assert.Contains(target, File.ReadAllText(marker), StringComparison.Ordinal); + } + finally + { + Directory.Delete(parent, true); + } + } + + /// + /// Both roots complete: the new root is the live one and the old copy is stale. Nothing moves, nothing + /// is overwritten, and no signpost is planted over a folder we did not touch. + /// + [Fact] + public void Migrate_LeavesBothAlone_WhenTheNewRootAlreadyHasAStore() + { + var parent = NewTempDir("both"); + try + { + var legacy = Path.Combine(parent, "PerformanceMonitorLite"); + var target = Path.Combine(parent, "PerformanceMonitorLite-Data"); + PopulateLegacyRoot(legacy); + WriteFile(target, Path.Combine("config", "settings.json"), "{\"alerts_enabled\":false}"); + WriteFile(target, "monitor.duckdb", "current-duckdb"); + var log = new List(); + + var result = DataRootMigration.Migrate(legacy, target, log.Add); + + Assert.Equal(DataRootMigrationStatus.AlreadyMigrated, result.Status); + Assert.Empty(result.Moved); + + /* The new root's copies win, untouched. */ + Assert.Equal("current-duckdb", File.ReadAllText(Path.Combine(target, "monitor.duckdb"))); + Assert.Equal("{\"alerts_enabled\":false}", File.ReadAllText(Path.Combine(target, "config", "settings.json"))); + + /* And the old root is left exactly as it was, including the artifacts the new root lacks. */ + Assert.Equal("duckdb-bytes", File.ReadAllText(Path.Combine(legacy, "monitor.duckdb"))); + Assert.True(Directory.Exists(Path.Combine(legacy, "archive"))); + Assert.True(File.Exists(Path.Combine(legacy, "alert_state.json"))); + Assert.False(File.Exists(Path.Combine(legacy, DataRootMigration.MarkerFileName))); + + /* Silence here would strand data with no way to find out - the skip has to say so. */ + Assert.Single(log); + } + finally + { + Directory.Delete(parent, true); + } + } + + /// An old root that only ever got partway through a first run: move what is actually there. + [Fact] + public void Migrate_MovesOnlyWhatExists_WhenTheLegacyRootIsPartial() + { + var parent = NewTempDir("partial"); + try + { + var legacy = Path.Combine(parent, "PerformanceMonitorLite"); + var target = Path.Combine(parent, "PerformanceMonitorLite-Data"); + WriteFile(legacy, Path.Combine("config", "settings.json"), "{}"); + WriteFile(legacy, "Update.exe", "velopack"); + var log = new List(); + + var result = DataRootMigration.Migrate(legacy, target, log.Add); + + Assert.Equal(DataRootMigrationStatus.Migrated, result.Status); + Assert.Equal(new[] { "config" }, result.Moved); + Assert.True(File.Exists(Path.Combine(target, "config", "settings.json"))); + Assert.False(Directory.Exists(Path.Combine(legacy, "config"))); + Assert.False(File.Exists(Path.Combine(target, "monitor.duckdb"))); + Assert.True(File.Exists(Path.Combine(legacy, "Update.exe"))); + } + finally + { + Directory.Delete(parent, true); + } + } + + /// + /// The self-heal case. A previous run moved config\ and then died before the store followed (a locked + /// file, a killed process). The new root now has settings.json but no store, and the store is still + /// sitting in the folder the next Setup.exe deletes. Finish the job. + /// + [Fact] + public void Migrate_FinishesAnInterruptedRun_WithoutOverwritingWhatAlreadyLanded() + { + var parent = NewTempDir("resume"); + try + { + var legacy = Path.Combine(parent, "PerformanceMonitorLite"); + var target = Path.Combine(parent, "PerformanceMonitorLite-Data"); + PopulateLegacyRoot(legacy); + WriteFile(target, Path.Combine("config", "settings.json"), "{\"already\":\"moved\"}"); + var log = new List(); + + var result = DataRootMigration.Migrate(legacy, target, log.Add); + + Assert.Equal(DataRootMigrationStatus.Migrated, result.Status); + + /* The store follows its settings across. */ + Assert.Equal("duckdb-bytes", File.ReadAllText(Path.Combine(target, "monitor.duckdb"))); + Assert.False(File.Exists(Path.Combine(legacy, "monitor.duckdb"))); + + /* The already-landed config is NOT clobbered by the old one, and the old one is left behind. */ + Assert.Equal("{\"already\":\"moved\"}", File.ReadAllText(Path.Combine(target, "config", "settings.json"))); + Assert.Contains("config", result.Kept); + Assert.True(File.Exists(Path.Combine(legacy, "config", "settings.json"))); + + AssertUpdaterFilesIntact(legacy); + } + finally + { + Directory.Delete(parent, true); + } + } + + /// + /// A self-move would be a rename of a folder onto itself. Guarded, because getting it wrong destroys + /// the very data this class exists to protect. + /// + [Fact] + public void Migrate_RefusesToMoveARootOntoItself() + { + var root = NewTempDir("same"); + try + { + PopulateLegacyRoot(root); + var log = new List(); + + var result = DataRootMigration.Migrate(root, root + Path.DirectorySeparatorChar, log.Add); + + Assert.Equal(DataRootMigrationStatus.NothingToMigrate, result.Status); + Assert.True(File.Exists(Path.Combine(root, "monitor.duckdb"))); + Assert.True(Directory.Exists(Path.Combine(root, "config"))); + } + finally + { + Directory.Delete(root, true); + } + } +} diff --git a/Lite/App.xaml.cs b/Lite/App.xaml.cs index eaa1c31d2d..1e7f092791 100644 --- a/Lite/App.xaml.cs +++ b/Lite/App.xaml.cs @@ -65,8 +65,10 @@ being handed back the stale in-memory version. The coordinator owns the mutex + /// /// Gets the per-user config directory path. Holds settings.json, schedules, and - /// other per-user preferences. Stays in %LOCALAPPDATA% so Velopack updates can - /// replace the app directory without losing data. + /// other per-user preferences. Lives under the data root, which is deliberately a + /// SIBLING of the install directory rather than inside it — Velopack's in-place + /// update left data alone, but re-running Setup.exe deletes the install directory + /// outright (#1832). See . /// public static string ConfigDirectory { get; private set; } = string.Empty; @@ -356,11 +358,29 @@ an older-but-elevated one raises an actionable error. */ System.Windows.Media.RenderOptions.ProcessRenderMode = System.Windows.Interop.RenderMode.SoftwareOnly; - // Initialize paths — store data in %LOCALAPPDATA% so Velopack updates - // can replace the app directory without losing data - var appDataRoot = Path.Combine( - Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData), - "PerformanceMonitorLite"); + /* Initialize paths. Data lives in %LOCALAPPDATA%\PerformanceMonitorLite-Data, NOT in + %LOCALAPPDATA%\PerformanceMonitorLite — that second path is Velopack's install root, and + re-running Setup.exe over an existing install renames it aside and deletes it, taking the + store, the archive and settings.json with it (#1832). Local rather than roaming: the DuckDB + store must not be dragged across a roaming profile. Migration runs first, before anything + reads settings or opens the store; AppLogger buffers until Initialize, so its log lines + land in the file a few statements later. */ + var localAppData = Environment.GetFolderPath(Environment.SpecialFolder.LocalApplicationData); + var appDataRoot = Path.Combine(localAppData, Services.DataRootMigration.DataRootName); + + var migration = Services.DataRootMigration.Migrate( + Path.Combine(localAppData, Services.DataRootMigration.LegacyRootName), + appDataRoot, + message => AppLogger.Info("DataRoot", message)); + + if (migration.Failed.Count > 0) + { + AppLogger.Error("DataRoot", + $"{migration.Failed.Count} item(s) could not be moved out of the install directory and are " + + $"NOT in use: {string.Join(", ", migration.Failed)}. Close any other Lite instance and restart " + + "to retry."); + } + DataDirectory = appDataRoot; ConfigDirectory = Path.Combine(appDataRoot, "config"); DatabasePath = Path.Combine(appDataRoot, "monitor.duckdb"); @@ -533,11 +553,34 @@ private static void LoadDefaultTimeRange() catch { /* Use default */ } } - public static void LoadAlertSettings() + public static void LoadAlertSettings() => LoadAlertSettings(ConfigDirectory, GetWebhookUrl, SaveWebhookUrl); + + /// + /// Path- and secret-store-injected form, so the "secrets load even without settings.json" contract is + /// testable without touching the process-wide config directory or the real Credential Manager. + /// is required rather than optional on purpose: this method WRITES to the + /// credential store on the #1506 legacy-plaintext-URL path, and a test that forgot to intercept that + /// would silently overwrite the operator's real webhook URL. + /// + internal static void LoadAlertSettings( + string configDirectory, + Func readSecret, + Action writeSecret) { + /* Webhook secrets live in Credential Manager, never in settings.json, so they load FIRST and + unconditionally — before the early return below. They used to load at the tail of the + settings.json parse, which meant a missing settings.json (exactly what the #1832 install-root + wipe produced) left the Settings window's webhook boxes blank while the credentials were still + there. Saving from that window then wrote the blank back, and SaveWebhookUrl DELETES on blank — + so opening Settings once after the data loss destroyed the surviving webhook URLs too. */ + TeamsWebhookUrl = readSecret(TeamsWebhookCredentialKey); + SlackWebhookUrl = readSecret(SlackWebhookCredentialKey); + GenericWebhookUrl = readSecret(GenericWebhookCredentialKey); + GenericWebhookHeadersJson = readSecret(GenericWebhookHeadersCredentialKey); + try { - var path = Path.Combine(ConfigDirectory, "settings.json"); + var path = Path.Combine(configDirectory, "settings.json"); if (!File.Exists(path)) return; using var doc = System.Text.Json.JsonDocument.Parse(File.ReadAllText(path)); @@ -658,13 +701,16 @@ cannot drive a nonsense threshold in either app. */ if (root.TryGetProperty("generic_proxy_address", out v)) GenericWebhookProxyAddress = v.GetString() ?? ""; if (root.TryGetProperty("generic_body_template", out v)) GenericWebhookBodyTemplate = v.GetString() ?? ""; - /* Migrate webhook URLs from plaintext settings.json to Credential Manager */ + /* Migrate webhook URLs from plaintext settings.json to Credential Manager. A legacy plaintext + URL still wins over whatever the store held, matching the old order (save, then read back); + the live property is set here rather than re-reading, since we just wrote the value. */ if (root.TryGetProperty("teams_webhook_url", out v)) { var legacyUrl = v.GetString() ?? ""; if (!string.IsNullOrWhiteSpace(legacyUrl)) { - SaveWebhookUrl(TeamsWebhookCredentialKey, legacyUrl); + writeSecret(TeamsWebhookCredentialKey, legacyUrl); + TeamsWebhookUrl = legacyUrl; } } if (root.TryGetProperty("slack_webhook_url", out v)) @@ -672,17 +718,11 @@ cannot drive a nonsense threshold in either app. */ var legacyUrl = v.GetString() ?? ""; if (!string.IsNullOrWhiteSpace(legacyUrl)) { - SaveWebhookUrl(SlackWebhookCredentialKey, legacyUrl); + writeSecret(SlackWebhookCredentialKey, legacyUrl); + SlackWebhookUrl = legacyUrl; } } - /* Load webhook URLs from Credential Manager. The generic channel's headers JSON rides the same - secure store as a URL — it carries the Authorization bearer token (#1506). */ - TeamsWebhookUrl = GetWebhookUrl(TeamsWebhookCredentialKey); - SlackWebhookUrl = GetWebhookUrl(SlackWebhookCredentialKey); - GenericWebhookUrl = GetWebhookUrl(GenericWebhookCredentialKey); - GenericWebhookHeadersJson = GetWebhookUrl(GenericWebhookHeadersCredentialKey); - /* SMTP settings */ if (root.TryGetProperty("smtp_enabled", out v)) SmtpEnabled = v.GetBoolean(); if (root.TryGetProperty("smtp_server", out v)) SmtpServer = v.GetString() ?? ""; diff --git a/Lite/MainWindow.xaml.cs b/Lite/MainWindow.xaml.cs index 9a2ef83b08..c2ea65e000 100644 --- a/Lite/MainWindow.xaml.cs +++ b/Lite/MainWindow.xaml.cs @@ -328,6 +328,7 @@ private async Task CheckForUpdatesOnStartupAsync() Dispatcher.Invoke(() => { Title = $"Performance Monitor Lite — Update v{newVersion.TargetFullRelease.Version} available (Help > About)"; + AnnounceUpdate($"v{newVersion.TargetFullRelease.Version}"); }); return; } @@ -344,6 +345,7 @@ private async Task CheckForUpdatesOnStartupAsync() Dispatcher.Invoke(() => { Title = $"Performance Monitor Lite — Update {result.LatestVersion} available (Help > About)"; + AnnounceUpdate(result.LatestVersion); }); } } @@ -353,6 +355,21 @@ private async Task CheckForUpdatesOnStartupAsync() } } + /// + /// One tray balloon per session when an update is found. The title-bar suffix was the only signal, + /// and a title bar nobody reads is how installs stayed on old builds — or got upgraded by re-running + /// Setup.exe, which used to delete the data directory (#1832). Help > About does it safely and in + /// place. The update check itself runs once per launch, so no extra suppression is needed. + /// + private void AnnounceUpdate(string? version) + { + _trayService?.ShowNotification( + "Update Available", + $"Performance Monitor Lite {version} is available. Open Help > About to install it — that updates in " + + "place and keeps your data.", + Hardcodet.Wpf.TaskbarNotification.BalloonIcon.Info); + } + private bool _closingCleanupStarted; private bool _closingCleanupDone; diff --git a/Lite/Services/ConfigSeeder.cs b/Lite/Services/ConfigSeeder.cs index 7975960048..adbb8cfb2f 100644 --- a/Lite/Services/ConfigSeeder.cs +++ b/Lite/Services/ConfigSeeder.cs @@ -12,7 +12,7 @@ namespace PerformanceMonitorLite.Services; /// -/// Seeds the per-user config directory (%LOCALAPPDATA%\PerformanceMonitorLite\config) from the config +/// Seeds the per-user config directory (%LOCALAPPDATA%\PerformanceMonitorLite-Data\config) from the config /// files bundled next to the exe, on first run. Without this a fresh install / nightly extract starts /// with an empty config dir, so the per-user ignored_wait_types.json never exists and wait-stat /// filtering silently becomes a no-op — every benign wait (SOS_WORK_DISPATCHER, DISPATCHER_QUEUE_SEMAPHORE, diff --git a/Lite/Services/DataRootMigration.cs b/Lite/Services/DataRootMigration.cs new file mode 100644 index 0000000000..40cedc1651 --- /dev/null +++ b/Lite/Services/DataRootMigration.cs @@ -0,0 +1,279 @@ +/* + * Copyright (c) 2026 Erik Darling, Darling Data LLC + * + * This file is part of the SQL Server Performance Monitor Lite. + * + * Licensed under the MIT License. See LICENSE file in the project root for full license information. + */ + +using System; +using System.Collections.Generic; +using System.IO; +using System.Text; + +namespace PerformanceMonitorLite.Services; + +/// +/// What did. +/// +internal enum DataRootMigrationStatus +{ + /// The legacy root is absent, or holds none of Lite's artifacts. A fresh install lands here. + NothingToMigrate, + + /// The new root already holds a complete store, so the legacy root was left entirely alone. + AlreadyMigrated, + + /// Every artifact Lite owns in the legacy root moved across. + Migrated, + + /// At least one move failed. What failed stays in the legacy root and is retried next launch. + PartiallyMigrated +} + +/// +/// The outcome of one migration attempt. names artifacts that existed in BOTH roots — +/// the new root's copy wins and the legacy one is never touched. +/// +internal sealed class DataRootMigrationResult +{ + public DataRootMigrationStatus Status { get; init; } + public IReadOnlyList Moved { get; init; } = Array.Empty(); + public IReadOnlyList Kept { get; init; } = Array.Empty(); + public IReadOnlyList Failed { get; init; } = Array.Empty(); +} + +/// +/// #1832: moves Lite's per-user data out of %LOCALAPPDATA%\PerformanceMonitorLite — which is also +/// Velopack's install root — into the sibling %LOCALAPPDATA%\PerformanceMonitorLite-Data. +/// +/// Re-running Setup.exe over an existing install renames the install root aside and deletes it, so every +/// release installed that way destroyed the DuckDB store, the Parquet archive, the logs, and settings.json. +/// In-app updates never did (Velopack updates the current\ subfolder in place), which is why the +/// data loss looked random. +/// +/// The migration is deliberately an explicit allow-list rather than "move the folder contents": the legacy +/// root ALSO holds Velopack's own Update.exe, current\, packages\ and +/// velopack.log. Moving those would break the installed app and its updater. Only the artifacts +/// listed here belong to us, and only those move. +/// +internal static class DataRootMigration +{ + /// The Velopack install root, which is where data used to live. + internal const string LegacyRootName = "PerformanceMonitorLite"; + + /// The new per-user data root. A LOCAL sibling — the DuckDB store must never roam. + internal const string DataRootName = "PerformanceMonitorLite-Data"; + + /// Signpost left in the legacy root so someone browsing it can find their data. + internal const string MarkerFileName = "DATA-MOVED.txt"; + + /// + /// Directories Lite owns under its data root. monitor.duckdb.tmp is DuckDB's scratch directory — + /// it only survives an unclean shutdown, and it belongs with the database file it spilled for. + /// + private static readonly string[] s_directories = { "config", "archive", "logs", "monitor.duckdb.tmp" }; + + /// Files Lite owns directly under its data root. + private static readonly string[] s_files = { "monitor.duckdb", "monitor.duckdb.wal", "alert_state.json" }; + + /// + /// The two artifacts that make a root "the live install": user settings and the store. If BOTH are + /// already in the new root, a previous launch finished the job and the legacy root is stale. + /// + private static bool HasCompleteStore(string root) => + File.Exists(Path.Combine(root, "config", "settings.json")) + && File.Exists(Path.Combine(root, "monitor.duckdb")); + + /// + /// Moves everything Lite owns from to , then + /// leaves a marker behind. Never deletes the legacy root itself — Velopack owns it. + /// + /// Nothing in the new root is ever overwritten: an artifact present in both roots is left in both, with + /// the new root's copy live. Per-artifact rather than all-or-nothing, so a run interrupted halfway (a + /// locked store file, a killed process) finishes on the next launch instead of stranding the store in a + /// folder the next Setup.exe will delete. + /// + /// Runs before the logger is initialized, so is the only channel out. Never + /// throws: a failed migration must not stop the app from starting. + /// + internal static DataRootMigrationResult Migrate(string legacyRoot, string newRoot, Action log) + { + var moved = new List(); + var kept = new List(); + var failed = new List(); + + try + { + if (!Directory.Exists(legacyRoot) + || string.Equals( + Path.TrimEndingDirectorySeparator(Path.GetFullPath(legacyRoot)), + Path.TrimEndingDirectorySeparator(Path.GetFullPath(newRoot)), + StringComparison.OrdinalIgnoreCase)) + { + return new DataRootMigrationResult { Status = DataRootMigrationStatus.NothingToMigrate }; + } + + var pending = new List<(string Name, bool IsDirectory)>(); + foreach (var name in s_directories) + { + if (Directory.Exists(Path.Combine(legacyRoot, name))) pending.Add((name, true)); + } + foreach (var name in s_files) + { + if (File.Exists(Path.Combine(legacyRoot, name))) pending.Add((name, false)); + } + + if (pending.Count == 0) + { + return new DataRootMigrationResult { Status = DataRootMigrationStatus.NothingToMigrate }; + } + + if (HasCompleteStore(newRoot)) + { + log($"Data root '{newRoot}' is already populated, so the copy still in '{legacyRoot}' was left " + + $"untouched and is NOT in use ({string.Join(", ", Names(pending))}). Delete it by hand once " + + "you are satisfied nothing is missing."); + return new DataRootMigrationResult + { + Status = DataRootMigrationStatus.AlreadyMigrated, + Kept = Names(pending) + }; + } + + Directory.CreateDirectory(newRoot); + + foreach (var (name, isDirectory) in pending) + { + var source = Path.Combine(legacyRoot, name); + var target = Path.Combine(newRoot, name); + + if (isDirectory ? Directory.Exists(target) : File.Exists(target)) + { + kept.Add(name); + continue; + } + + try + { + if (isDirectory) + { + Directory.Move(source, target); + } + else + { + File.Move(source, target); + } + + moved.Add(name); + } + catch (Exception ex) + { + failed.Add(name); + log($"Could not move '{source}' to '{target}': {ex.Message}. It stays where it is and the " + + "move is retried on the next start."); + } + } + + if (moved.Count == 0 && failed.Count == 0) + { + return new DataRootMigrationResult + { + Status = DataRootMigrationStatus.AlreadyMigrated, + Kept = kept + }; + } + + if (moved.Count > 0) + { + log($"Moved Lite's data out of the install directory '{legacyRoot}' and into '{newRoot}' (#1832): " + + $"{string.Join(", ", moved)}. Re-running Setup.exe deletes the install directory, which is why " + + "data kept there did not survive an installer upgrade."); + } + + if (kept.Count > 0) + { + log($"Left in '{legacyRoot}' because '{newRoot}' already had a copy: {string.Join(", ", kept)}."); + } + + /* Only claim the move in the marker once nothing is still behind — a partial run retries next + launch and writes the marker then. */ + if (failed.Count == 0) + { + TryWriteMarker(legacyRoot, newRoot, moved, log); + } + + return new DataRootMigrationResult + { + Status = failed.Count > 0 ? DataRootMigrationStatus.PartiallyMigrated : DataRootMigrationStatus.Migrated, + Moved = moved, + Kept = kept, + Failed = failed + }; + } + catch (Exception ex) + { + log($"Data directory migration failed: {ex.Message}. Lite starts against '{newRoot}' regardless; " + + $"anything left in '{legacyRoot}' is untouched."); + return new DataRootMigrationResult + { + Status = failed.Count > 0 || moved.Count > 0 + ? DataRootMigrationStatus.PartiallyMigrated + : DataRootMigrationStatus.NothingToMigrate, + Moved = moved, + Kept = kept, + Failed = failed + }; + } + } + + private static string[] Names(List<(string Name, bool IsDirectory)> items) + { + var names = new string[items.Count]; + for (var i = 0; i < items.Count; i++) + { + names[i] = items[i].Name; + } + + return names; + } + + /// + /// Drops a plain-text signpost in the legacy root. The old root is NOT deleted — it is the Velopack + /// install directory and still holds Update.exe, current\ and packages\. + /// + private static void TryWriteMarker(string legacyRoot, string newRoot, List moved, Action log) + { + try + { + var marker = Path.Combine(legacyRoot, MarkerFileName); + var text = new StringBuilder(); + text.AppendLine("Performance Monitor Lite data has MOVED."); + text.AppendLine(); + text.AppendLine("This folder is the application install directory. The installer owns it: running"); + text.AppendLine("Setup.exe again renames it aside and deletes it, which used to destroy the"); + text.AppendLine("monitoring history stored here (issue #1832)."); + text.AppendLine(); + text.AppendLine("Your data now lives in:"); + text.AppendLine(); + text.AppendLine(" " + newRoot); + text.AppendLine(); + text.Append("Moved on ").Append(DateTime.Now.ToString("yyyy-MM-dd HH:mm:ss", System.Globalization.CultureInfo.InvariantCulture)).AppendLine(":"); + foreach (var name in moved) + { + text.Append(" ").AppendLine(name); + } + + text.AppendLine(); + text.AppendLine("Nothing was deleted. This file is only a signpost and is safe to remove."); + + File.WriteAllText(marker, text.ToString()); + } + catch (Exception ex) + { + /* The signpost is a courtesy. Failing to write it must not fail the migration that already + succeeded. */ + log($"Could not write '{MarkerFileName}' in '{legacyRoot}': {ex.Message}"); + } + } +} diff --git a/Lite/Services/IgnoredWaitTypes.cs b/Lite/Services/IgnoredWaitTypes.cs index 4c7920176e..200fd1da63 100644 --- a/Lite/Services/IgnoredWaitTypes.cs +++ b/Lite/Services/IgnoredWaitTypes.cs @@ -16,7 +16,7 @@ namespace PerformanceMonitorLite.Services; /// /// Single source for the configured "ignored" (benign/idle) wait types — the per-user editable -/// %LOCALAPPDATA%\PerformanceMonitorLite\config\ignored_wait_types.json, falling back to the copy +/// %LOCALAPPDATA%\PerformanceMonitorLite-Data\config\ignored_wait_types.json, falling back to the copy /// bundled next to the exe. Used by BOTH the collector (skip at collection) and the wait-stats tab /// queries (skip at display) so the two can't drift. Display-side filtering is what hides waits already /// collected before the filter was active — copying the JSON only stops NEW collection, it can't remove diff --git a/README.md b/README.md index 7902b64a09..ab1c3acbc9 100644 --- a/README.md +++ b/README.md @@ -93,7 +93,7 @@ All editions include real-time alerts (system tray + email + webhooks), charts a ## Quick Start — Lite 1. Download **[`PerformanceMonitorLite-win-Setup.exe`](https://github.com/erikdarlingdata/PerformanceMonitor/releases/latest)** (requires [.NET 10 Desktop Runtime](https://dotnet.microsoft.com/en-us/download/dotnet/10.0)) -2. Run the installer — it installs to `%LocalAppData%\PerformanceMonitorLite`, adds **Start Menu** and **Desktop** shortcuts, and registers the app under **Apps & Features** so it shows up in Windows search and can be uninstalled normally. Auto-update is wired in. +2. Run the installer — it installs to `%LocalAppData%\PerformanceMonitorLite`, adds **Start Menu** and **Desktop** shortcuts, and registers the app under **Apps & Features** so it shows up in Windows search and can be uninstalled normally. Auto-update is wired in. Your data goes in `%LocalAppData%\PerformanceMonitorLite-Data`, a separate folder the installer never touches. 3. Launch from the Start Menu or Desktop shortcut. 4. Click **+ Add Server**, enter connection details, test, save. 5. Double-click the server in the sidebar to connect. @@ -102,6 +102,8 @@ Data starts flowing within 1–5 minutes. That's it. No installation on your ser **Upgrading from zip?** Click **Import Settings** then **Import Data** in the sidebar and point both at your old Lite folder. Settings imports server connections, alert thresholds, SMTP config, and schedules. Data imports historical DuckDB + Parquet archives. **Auto-update users** (installed via Setup.exe) get updates automatically — no manual import needed. +> **Warning — upgrading an install older than the data-relocation fix:** on those versions Lite kept its data *inside* the install directory, and re-running `Setup.exe` over an existing install deletes that directory before the new build ever runs. It takes the DuckDB store, the Parquet archive, the logs, and `settings.json` with it. Upgrade **in place** instead: **Help > About** downloads and applies the update without touching your data, or extract the portable ZIP over your existing copy. From this release on, data lives outside the install directory and is moved there automatically on first start, so `Setup.exe` is safe again. (Your monitored-server list is unaffected either way — it lives in `%ProgramData%\PerformanceMonitorLite\config` — and so are the passwords in Windows Credential Manager.) + **Always On AG?** Enable **ReadOnlyIntent** in the connection settings to route Lite's monitoring queries to a readable secondary, keeping the primary clear. Enable **MultiSubnetFailover** for multi-subnet failover scenarios. ### Lite Collectors @@ -153,7 +155,7 @@ Darling runs this same shared collector set across a fleet of servers (latch sta ### Lite Data Storage -All data is stored in `%LOCALAPPDATA%\PerformanceMonitorLite\` — separate from the executable, so auto-updates don't affect your data. +All data is stored in `%LOCALAPPDATA%\PerformanceMonitorLite-Data\` — a different folder from the install directory (`%LOCALAPPDATA%\PerformanceMonitorLite\`), so neither an in-app update nor re-running `Setup.exe` can disturb it. Data from an older install is moved into the new folder automatically the first time this version starts. - **Hot data** in DuckDB 1.5.2 — non-blocking checkpoints, free block reuse, stable file size without periodic resets - **Archive** to Parquet with ZSTD compression (~10x reduction) — automatic monthly compaction keeps file count low (~75 files vs thousands) @@ -165,9 +167,9 @@ All data is stored in `%LOCALAPPDATA%\PerformanceMonitorLite\` — separate from | File | Location | Purpose | |---|---|---| | `servers.json` | `%ProgramData%\PerformanceMonitorLite\config\` (machine-wide) | Server connections, shared across all Windows users on the machine. Passwords stay per-user in Windows Credential Manager. Optional **Utility Database** per server for community procs installed outside master. | -| `settings.json` | `%LOCALAPPDATA%\PerformanceMonitorLite\config\` (per-user) | Retention, MCP server, startup behavior, alert thresholds, SMTP configuration | -| `collection_schedule.json` | `%LOCALAPPDATA%\PerformanceMonitorLite\config\` (per-user) | Per-collector enable/disable and frequency | -| `ignored_wait_types.json` | `%LOCALAPPDATA%\PerformanceMonitorLite\config\` (per-user) | 124 benign wait types excluded by default | +| `settings.json` | `%LOCALAPPDATA%\PerformanceMonitorLite-Data\config\` (per-user) | Retention, MCP server, startup behavior, alert thresholds, SMTP configuration | +| `collection_schedule.json` | `%LOCALAPPDATA%\PerformanceMonitorLite-Data\config\` (per-user) | Per-collector enable/disable and frequency | +| `ignored_wait_types.json` | `%LOCALAPPDATA%\PerformanceMonitorLite-Data\config\` (per-user) | 124 benign wait types excluded by default | When a second Windows user on the same machine launches Lite, they see the shared `servers.json` immediately. SQL Auth and Entra MFA passwords are scoped to each user's own Credential Manager, so they'll be prompted once per server; Windows Auth works without any prompt. diff --git a/docs/how-collection-works.md b/docs/how-collection-works.md index d153e635ac..4b4331b92f 100644 --- a/docs/how-collection-works.md +++ b/docs/how-collection-works.md @@ -166,7 +166,7 @@ Once a day, the `PerformanceMonitor - Data Retention` job runs a `DELETE` loop p ### What's different -Lite is a standalone WPF app — **no SQL Agent involved, no PerformanceMonitor database**. The app itself is the collector, and the storage is a local DuckDB file (`%LocalAppData%\PerformanceMonitorLite\pm_lite.duckdb`). +Lite is a standalone WPF app — **no SQL Agent involved, no PerformanceMonitor database**. The app itself is the collector, and the storage is a local DuckDB file (`%LocalAppData%\PerformanceMonitorLite-Data\monitor.duckdb`). The shape still mirrors Full: a dispatcher picks collectors, each collector pulls from DMVs and writes to a destination table, and a reader service hands data to the UI.