Skip to content

Release v2.8.0#878

Merged
erikdarlingdata merged 46 commits intomainfrom
dev
Apr 22, 2026
Merged

Release v2.8.0#878
erikdarlingdata merged 46 commits intomainfrom
dev

Conversation

@erikdarlingdata
Copy link
Copy Markdown
Owner

Summary

Release v2.8.0.

See CHANGELOG.md for the full list.

Test plan

  • Installer.Tests — 46/46 passed
  • dotnet build -c Debug — 0 errors
  • Fresh install on sql2016 (--reinstall) — 54/54 scripts, 48 HEALTHY, 0 errors
  • Upgrade install on sql2017 (2.7.0→2.8.0) — 3 new indexes present, data survived
  • Upgrade install on sql2022 (2.7.0→2.8.0) + idempotency re-run
  • Multi-hop upgrade on sql2019 — v2.5.0→2.8.0 and v2.6.0→2.8.0 via CLI (filesystem provider)
  • Dashboard GUI upgrade on sql2025 (2.6.0→2.8.0, embedded-resource provider — [BUG] Invalid Column Name errors for collect.database_size_stats_collector #772 regression surface)
  • Cloud platform testing — Azure SQL DB + AWS RDS, fresh installs, 60-min burn-in clean (0 errors)
  • installation_history SUCCESS with correct previous_version across all paths
  • Data survival verified via row-count diffs before/after each upgrade

🤖 Generated with Claude Code

ClaudioESSilva and others added 30 commits April 15, 2026 17:20
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Add trailing newlines to ScrollPanBehavior files
…ving race

Addresses security findings from #840:
- #846: Escape single quotes in file paths interpolated into read_parquet() and COPY TO
- #847: Use DuckDB $1 parameters for DateTime values instead of string interpolation
- #849: Make IsArchiving volatile-backed to prevent stale reads across threads

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…ening

Harden DuckDB queries and fix IsArchiving race condition
Moves Teams and Slack webhook URLs from plaintext settings.json/preferences.json
to Windows Credential Manager (DPAPI-encrypted), matching the existing pattern
used for SMTP passwords and SQL Server credentials.

Includes automatic migration: on first settings load, any plaintext URLs are
moved to Credential Manager and removed from the JSON file.

Closes #848

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Encrypt webhook URLs with DPAPI via Credential Manager
…rst visit

Initial tab open and Refresh button now only load the currently visible tab.
First switch to any tab triggers a full refresh of that tab (all sub-tabs).
Subsequent refreshes only hit the active sub-tab.

Ctrl+Click on Refresh Tab (or Ctrl+F5) refreshes all tabs at once.
Apply to All Tabs retains existing full-refresh behavior.

Fixes #835 — prevents heavy queries (e.g. GetQueryStatsAsync) from running
on tab open when the user is only viewing Overview.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
GetQueryStatsAsync, GetProcedureStatsAsync, and GetQueryStoreDataAsync
were returning unbounded result sets. With 49 databases and 742K rows
in query_stats over 3 days, the GROUP BY with plan XML could produce
thousands of rows and timeout after 120 seconds.

TOP 500 ordered by avg CPU desc is plenty for a grid view and prevents
the query from consuming unbounded memory on large installations.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Lazy-load server tabs and cap query grid results to TOP 500
The CAST(DECOMPRESS(...)) NOT LIKE N'WAITFOR%' filter was decompressing
query text on every row in query_stats and query_store_data just to skip
WAITFOR queries. WAITFOR has no plan and no meaningful stats — it only
matters in query snapshots (active sessions), where the filter remains.

On a 742K-row query_stats table, this was a significant contributor to
the 120-second query timeouts reported in #835.

The snapshot filters (report.query_snapshots) and MCP phased queries
are untouched — they filter after TOP on already-hydrated text.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
All three grid queries now use a 3-phase pattern:
1. Aggregate numerics into temp table (no DECOMPRESS)
2. Sum across lifetimes, rank TOP 500
3. OUTER APPLY to decompress text/plan for only the 500 winners

On a 742K-row query_stats table, this reduces DECOMPRESS calls from
742K to 500 — eliminating the 16+ minute query times reported in #835.

Matches the existing phased pattern used by the MCP query tools.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…press

Remove WAITFOR DECOMPRESS filters from query stats and query store
TDE moved to Standard Edition in SQL 2019, so dm_db_persisted_sku_features
no longer reports it as Enterprise-only. Add version check to give
version-appropriate licensing guidance instead of falsely claiming no
databases use TDE.

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…on-awareness

Fix FinOps TDE recommendation on SQL Server 2019+
Port PS PRs #216, #217, #219, #224, #229, #230, #231 to PM.

PlanAnalyzer changes:
- Rule 5: Suppress for Key Lookups (point lookups mislead per-execution estimates)
- Rule 8: Enhanced parallel skew with batch mode sort detection and practical context
- Rule 9: Large memory grant shows top 3 consumers sorted by row count
- Rule 10: Key lookup overhaul — show output columns, check predicate filtering, softer advice
- Rules 11/12/29: Suppress on 0-execution nodes (operator never ran)
- Rule 11: I/O wait severity elevation when scan hits disk
- Rule 24: FormatNodeRef helper includes object name for data access operators
- Rule 26: Suppress when row goal prediction was correct, specific cause detection
- Wait stats: DescribeWaitType with full wait type coverage, multi-wait summary
- New helpers: GetWaitLabel, HasSignificantIoWaits, IdentifyRowGoalCause, FormatNodeRef
- GetOperatorOwnElapsedMs changed to internal for BenefitScorer access

BenefitScorer (new file):
- Stage 1: MaxBenefitPercent for operator-level rules (filter, spill, lookup, etc.)
- Stage 2: Wait stats benefit scoring with parallel allocation (Joe's formula)

PlanModels additions:
- MaxBenefitPercent and ActionableFix on PlanWarning
- WaitBenefit class and WaitBenefits list on PlanStatement

Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
…apr16

Sync PlanAnalyzer + BenefitScorer from PerformanceStudio (Apr 9-16)
…857)

On Azure SQL DB, some logins (e.g. Microsoft Dynamics 365 FO) are granted
access only to a specific user database and not to master. The three
collectors that enumerate databases via master — query_stats,
database_size_stats, file_io_stats — would fail the first time and
produce an empty screen.

GetAzureDatabaseListAsync now catches known access-denied/login-failed
errors from the master connection, caches the per-server decision, and
returns the connection's InitialCatalog as a single-element list. The
three callers already loop per-database, so single-DB mode works without
further changes.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…lback

Fall back to single-DB mode when Azure master is inaccessible (#857)
Phase 3 OUTER APPLY hydration of compressed query_text/plan_text was forcing
an Eager Index Spool over the full collect.query_stats table (and similar
for procedure_stats / query_store_data), which took 104 seconds on a
742K-row table in #835.

Changes:
- Remove CONVERT(binary(8), nvarchar-hash, 1) anti-pattern from OUTER APPLY
  WHERE clauses by keeping query_hash as native binary(8) in temp tables.
  query_hash is only converted to nvarchar(20) in the final output projection.
- Add three nonclustered indexes (install script and upgrade script):
    IX_query_stats_hash_lookup (query_hash, database_name, collection_time DESC)
    IX_procedure_stats_name_lookup (database_name, schema_name, object_name, collection_time DESC)
    IX_query_store_data_id_lookup (database_name, query_id, collection_time DESC)
- Indexes use SORT_IN_TEMPDB = ON and DATA_COMPRESSION = PAGE.
- ONLINE = ON is applied conditionally via dynamic SQL based on
  SERVERPROPERTY('EngineEdition') — Enterprise/Developer/Azure only, since
  Standard/Web/Express don't support online index operations.

Tested against CADelete's 742K-row table: Phase 3 went from 104s to
well under 1s (5s total for the full three-phase query).

Fixes #835

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…exes

Add nonclustered indexes for query/procedure/query store lookups
On Azure SQL Database, logins without access to master can't resolve
cross-database rows returned by sys.dm_exec_requests, which caused the
Live Snapshot button and the query snapshots collector to error in
D365FO-style environments (reported by @TrudAX in #857 after PR #858).

BuildQuerySnapshotsQuery now takes an isAzureSqlDatabase flag and emits
AND der.database_id = DB_ID() only when true. Boxed SQL Server, MI, and
elastic pool behavior is unchanged. The Live Snapshot button path gets
the flag through a new ServerTab constructor parameter wired from the
cached ServerConnectionStatus.SqlEngineEdition.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…ure-scope

Scope query snapshots to current database on Azure SQL DB (#857)
- Chart X-axis prints the date line only on the first tick and on ticks
  where the date changes; all other ticks show time only. Format respects
  current culture (en-GB → dd/MM, de-DE → dd.MM, 24h clocks, etc.).
  Implemented as a DateTimeTicksBottomDateChange() extension in
  Lite/Helpers/AxesExtensions.cs and applied to every DateTimeTicksBottom
  call site in ServerTab and CorrelatedTimelineLanesControl.
- Server name no longer duplicated in the ServerTab header status line;
  ConnectionStatusText now shows just "Connecting..." / "Last refresh: ...".
- Chart tick label font bumped from 12 to 13 for readability.
- New SubTabItemStyle (thin accent underline, transparent background) in
  all three themes, applied to Queries / Memory / File I/O / Blocking /
  Perfmon / Running Jobs sub-TabControls so sub-tab selection no longer
  looks identical to main-tab selection.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…lish

Polish Lite chart axes and sub-tab styling
Dashboard polish (ports the same items merged to Lite in #862):
- New Dashboard/Helpers/AxesExtensions.cs with DateTimeTicksBottomDateChange(),
  culture-aware (dd/MM for en-GB, dd.MM for de-DE, 24h clocks, etc.). All 52
  call sites of DateTimeTicksBottom() across 10 files swapped to use it.
- TabHelpers.ApplyTheme + ReapplyAxisColors bump chart tick label font from
  12 to 13 so numbers read cleaner on wide charts.
- SubTabItemStyle added to Dark / Light / CoolBreeze themes: thin accent
  underline + transparent background instead of filled cyan, so sub-tabs
  don't look identical to main tabs when selected. Wired via
  ItemContainerStyle on 11 sub-TabControls (Overview's inner tabs,
  Collection Health's inner tabs, Locking, ConfigChanges, CurrentConfig,
  FinOps, Memory, ResourceMetrics ×2, SystemEvents, QueryPerformance).

LSP diagnostics cleanup (tracked work from chore/lsp-diagnostics-cleanup):
- Small nullability/warning fixes across Dashboard and Lite services,
  analysis helpers, and BenefitScorer / PlanAnalyzer.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…eanup

Port Lite chart/tab polish to Dashboard + LSP diagnostics cleanup
Root cause: the control wired `Unloaded += ...Dispose()` on the crosshair
manager, and WPF fires Unloaded for transient reasons (tab virtualization,
layout rebuilds, etc.), not just when the control is actually going away.
Dispose() clears the manager's lane list, after which ReattachVLines runs
over an empty list and the crosshair is gone permanently.

Changes:
- Remove the Unloaded → Dispose() handler in both Lite and Dashboard copies.
  The manager holds only managed state (a Popup + lane references) — GC
  will clean it up with the control.
- Remove the now-redundant `_isRefreshing` flag from CorrelatedCrosshairManager.
  The `lane.VLine == null` check in OnMouseMove is a sufficient "not ready"
  guard and is self-healing once VLines are recreated.
- Wrap ReattachVLines in a try/finally on the control side, with a new
  idempotent EnsureVLinesAttached() safety net that only creates VLines
  for lanes where they're still null.
- Make CreateVLine catch per-lane exceptions so one failing chart can't
  prevent the others from recovering.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
erikdarlingdata and others added 15 commits April 18, 2026 20:20
…tuck

Fix Overview crosshair disappearing after tab switches / layout passes
Chart previously filtered to HIGH severity only (indicator>=3), which on most
servers never fires, producing an empty chart even when sp_pressuredetector-
level medium pressure (indicator=2) was occurring constantly. Switch to stacked
bars per hour, split by SQL Server (process) vs Operating System (system), with
severe events capped on top of medium in a darker shade. Extend ChartHoverHelper
to support BarPlot tooltips. Add MCP guidance for interpreting indicator values
and routing to the right follow-up tool.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Lite was missing the RING_BUFFER_RESOURCE_MONITOR collector entirely — no
collector, no table, no chart, no MCP tool. This adds the full feature:

- Schema: new memory_pressure_events table + index, schema v25, added to
  ArchivableTables, server-id-fix list, and ArchiveService.
- Collector: CollectMemoryPressureEventsAsync queries the ring buffer and
  client-side-dedupes against DuckDB's MAX(sample_time). Azure SQL DB returns
  zero rows (ring buffer not exposed there). Scheduled every 5 min (Aggressive
  and Balanced presets) or 15 min (Low-Impact).
- UI: new 'Memory Pressure Events' sub-tab on the Memory tab with the same
  stacked-bar chart as Dashboard (SQL Server medium/severe, Operating System
  medium/severe). Wired into full-load and sub-tab-switch refresh paths.
- Hover: ported the BarPlot support from Dashboard's ChartHoverHelper so bar
  tooltips work and report the correct segment height for stacked bars.
- MCP: new get_memory_pressure_events tool + the 'Interpreting Memory
  Pressure Events' guidance section in McpInstructions.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Companion update to the new memory_pressure_events table added in this PR.
SchemaStatements_MatchTableCount asserts the total table count; needs to
move from 29 to 30 to reflect the new table.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…chart

Fix and port Memory Pressure Events feature (#865)
Right-click > View Plan on a Blocked Process Reports row silently fell
through (no handler case) and Get Actual Plan erred with "no query text."

- Split the grid onto its own BlockedProcessContextMenu with separate
  View Blocked Plan / View Blocking Plan actions; drop Get Actual Plan
  (re-executing a mid-transaction blocked query is a foot-gun).
- Parse all <frame> entries from the BPR XML's executionStack, filter
  the 42-byte all-zero sql_handle placeholder (dynamic SQL / system
  context), default stmtstart=0 / stmtend=-1 per the dm_exec_text_query_plan
  convention. Matches sp_HumanEventsBlockViewer's XPath and join shape.
- Add FetchPlanBySqlHandleAsync keyed on sql_handle + statement offsets
  against sys.dm_exec_query_stats. Caller iterates frames until one
  resolves; falls back to a clear "plan no longer in cache" message.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…869)

Follow-up to #861. The DB_ID() predicate in the WHERE clause wasn't
enough — the OUTER APPLYs to sys.dm_exec_sql_text and
sys.dm_exec_text_query_plan were still being evaluated against
master-scoped rows from sys.dm_exec_requests before the filter was
applied, tripping VIEW SERVER PERFORMANCE STATE errors for DB-scoped
logins (D365FO). A CTE or derived table wouldn't guarantee the
filter order, so materialise the filtered request rows into #req
first and drive the DMFs off that.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…#870)

The collector loop already classifies SQL errors 229 / 297 / 300 as
PERMISSIONS status and excludes them from the failure rate, but it
keeps re-running the collector every interval and logging an
identical denial each time. For DB-scoped logins on Azure SQL DB
(e.g. D365FO) this churns the collection log and gives no new
information — the permission won't change mid-session.

Flag the collector on first denial and short-circuit RunCollectorAsync
so we don't make the round-trip or the log entry. Flag is in-memory
per (server, collector) — cleared on app restart so newly granted
permissions are picked up on the next launch.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
sys.dm_exec_query_statistics_xml requires VIEW SERVER PERFORMANCE
STATE on Azure SQL Database regardless of scope, so DB-scoped logins
(e.g. D365FO) still hit error 300 even after the #temp pre-filter
landed in #869. The OUTER APPLY evaluates the DMF for every session
in #req and fails on the permission check before returning rows.

Force supportsLiveQueryPlan=false for SqlEngineEdition=5 in both the
collector and the Live Snapshot button paths. Boxed SQL Server and
Azure MI (edition 8) still get live plans as before.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Sort recommendations by severity rank (High=1, Medium=2, Low=3)
instead of alphabetically. Adds SeveritySort property to
RecommendationRow and uses it as SortMemberPath for the Severity
column. Display strings are unchanged.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…order

Fix FinOps recommendation severity sort order (#872)
Severity column was sorting alphabetically (High, Low, Medium) instead
of by severity ranking. Added SeveritySort computed property on
FinOpsRecommendation, ordered results by it, and wired the DataGrid
column's SortMemberPath so click-sort matches the default order.

Mirrors the Lite fix in PR #874.
…-severity-order

Fix FinOps severity sort order in Dashboard (#872)
)

Azure SQL Database DBs hosted in an elastic pool (notably D365FO
customer tenants) enforce VIEW SERVER PERFORMANCE STATE on
sys.dm_os_schedulers regardless of the login's DB-scoped grants —
VIEW DATABASE STATE + VIEW DATABASE PERFORMANCE STATE on the user DB
are not sufficient. Verified by reproducing the failure in a
standard Azure SQL DB elastic pool with a contained DB user; bare
sys.dm_exec_requests/sys.dm_os_sys_info/sys.dm_os_performance_counters
succeed but sys.dm_os_memory_clerks / sys.dm_os_schedulers /
sys.dm_os_waiting_tasks fail with error 300.

The other failing collectors (memory_clerks, waiting_tasks,
tempdb_stats) have no DB-scoped alternative and will stay skip-gated
via #870 for these users.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Adds nonclustered indexes to collect.query_stats, procedure_stats, and
query_store_data for Dashboard grid lookups (#835). Ports Memory Pressure
Events to Lite (#865). Multiple Azure SQL DB collector fixes (#857).
FinOps severity sort order fix (#872). Grid auto-scrolling (#843).

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Copy link
Copy Markdown
Owner Author

@erikdarlingdata erikdarlingdata left a comment

Choose a reason for hiding this comment

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

Automated review for #878 — v2.8.0 release promotion (devmain).

What this PR does

Promotes 80 commits from dev to main for the 2.8.0 release: three new Dashboard/Lite indexes with a matching upgrade script, Memory Pressure Events ported to Lite, grid auto-scroll (Lite+Dashboard), FinOps severity sort fix, Azure SQL DB collector hardening (#857 rollup), blocked-process-report plan lookup fix, FinOps TDE gating on SQL 2019+, phased DECOMPRESS in query-performance grid queries with TOP 500 caps, DPAPI-backed webhook URL storage (Dashboard only), and version bumps.

What's good

  • Schema upgrade discipline: install/02_create_tables.sql:1498 and upgrades/2.7.0-to-2.8.0/01_add_query_lookup_indexes.sql are in sync, idempotent via IF NOT EXISTS on sys.indexes, and the upgrade is registered in upgrades/2.7.0-to-2.8.0/upgrade.txt. EngineEdition gate for ONLINE = ON (3/5/8) is correct for supported editions.
  • PlanAnalyzer parity: Dashboard/Services/PlanAnalyzer.cs and Lite/Services/PlanAnalyzer.cs are byte-equivalent outside namespace/using lines. Same for the two new shared files (BenefitScorer.cs, ScrollPanBehavior.cs, AxesExtensions.cs) and the three theme files.
  • Phased DECOMPRESS in Dashboard/Services/DatabaseService.QueryPerformance.cs: splitting into numeric aggregate → TOP 500 rank → hydrate winners is the right shape for avoiding DECOMPRESS across the whole table; DROP TABLE IF EXISTS guards at the top of each phase are correct for the single-batch pattern.
  • Webhook credential migration in Dashboard/SettingsWindow.xaml.cs:217-234 correctly clears plaintext from prefs.json after moving to Credential Manager.
  • Build workflow (.github/workflows/build.yml) is unchanged — signing policy not in play.

What needs attention

  1. Base branch: PR targets main. That's correct for a release promotion (CHANGELOG + version bumps), so flagging for acknowledgment only, not as a blocker. Feature branches should still target dev.
  2. Lite webhook parity (inline on Dashboard/Services/WebhookAlertService.cs): DPAPI/Credential Manager storage shipped to Dashboard only. Lite still reads App.TeamsWebhookUrl / App.SlackWebhookUrl in plaintext (Lite/Services/WebhookAlertService.cs:59,64,133,252). This inverts the "Lite-first" ordering and creates a silent asymmetry where the security-hardening release note applies to one edition. Acceptable if intentional given Lite's cross-platform surface, but the changelog line should be edition-scoped or the gap closed.
  3. PlanAnalyzer micro-drift: Dashboard/Services/PlanAnalyzer.cs:1831 has private sealed record ScanImpact(...) vs Lite/Services/PlanAnalyzer.cs:1828 private record. Not reachable as a behavioral difference (private nested record) but breaks the "these two files are byte-equivalent outside namespace" invariant. Align one way or the other.
  4. Migration block structure (inline on Dashboard/SettingsWindow.xaml.cs:217-234): runs on every Settings open and does two separate SavePreferences calls; see inline for suggested one-shot flag.
  5. Test coverage: BenefitScorer adds 653 lines in each edition and the phased-DECOMPRESS paths reshape three significant queries, but only Lite.Tests/DuckDbSchemaTests.cs (2/2) was touched. Nothing caught this in tests — noting for post-release backfill.
  6. SQL index duplication (inline on install/02_create_tables.sql): definitions live in two files; if one changes the other must follow. Not a blocker.

Not blocking; posting as comments only — maintainer call on each.


Generated by Claude Code

Comment on lines +218 to 234
if (!string.IsNullOrWhiteSpace(prefs.TeamsWebhookUrl))
{
WebhookAlertService.SaveTeamsWebhookUrl(prefs.TeamsWebhookUrl);
prefs.TeamsWebhookUrl = "";
_preferencesService.SavePreferences(prefs);
}
if (!string.IsNullOrWhiteSpace(prefs.SlackWebhookUrl))
{
WebhookAlertService.SaveSlackWebhookUrl(prefs.SlackWebhookUrl);
prefs.SlackWebhookUrl = "";
_preferencesService.SavePreferences(prefs);
}

/* Load webhook URLs from Credential Manager */
TeamsWebhookUrlTextBox.Text = WebhookAlertService.GetTeamsWebhookUrl();
SlackWebhookUrlTextBox.Text = WebhookAlertService.GetSlackWebhookUrl();
UpdateTeamsControlStates();
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

This migration path runs every time the Settings window opens, not just on the first post-upgrade open, and does two separate SavePreferences(prefs) calls when both URLs exist (line 222 and line 228). On the first open both branches fire; on every subsequent open both IsNullOrWhiteSpace guards short-circuit, so the behavior is correct but the structure reads like one-time migration code that's actually permanent. Suggest collapsing to a single save at the end of the block, and/or gating the whole migration on a preference flag (e.g. prefs.WebhookUrlsMigrated) so you can drop this code in a future release.


Generated by Claude Code

public static string GetTeamsWebhookUrl() => GetWebhookUrl(TeamsWebhookCredentialKey);
public static string GetSlackWebhookUrl() => GetWebhookUrl(SlackWebhookCredentialKey);
public static void SaveTeamsWebhookUrl(string url) => SaveWebhookUrl(TeamsWebhookCredentialKey, url);
public static void SaveSlackWebhookUrl(string url) => SaveWebhookUrl(SlackWebhookCredentialKey, url);
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Cross-edition parity gap: Lite's webhook service still reads plaintext URLs from App.TeamsWebhookUrl / App.SlackWebhookUrl (see Lite/Services/WebhookAlertService.cs:59,64,133,252), and Lite isn't touched in this PR. CredentialService is Windows-only (DPAPI via Credential Manager), so Lite needs a platform-appropriate equivalent (libsecret on Linux, Keychain on macOS, or at minimum DPAPI-only on Windows Lite with a documented gap elsewhere). Shipping DPAPI encryption for Dashboard while Lite keeps storing webhook URLs in plaintext prefs.json is a silent asymmetry worth calling out in the changelog — or ideally closing before release.


Generated by Claude Code

Comment on lines +1498 to 1577
/*
Nonclustered indexes to support OUTER APPLY lookups in the Dashboard's
query/procedure/query store grid queries. Without these, the optimizer builds
an Eager Index Spool over the entire table to service the lookups, which can
take minutes on large installations (see #835).

ONLINE = ON is only supported on Enterprise/Developer/Azure editions. The
options string is built dynamically based on SERVERPROPERTY('EngineEdition').
*/
DECLARE @online_option nvarchar(20) =
CASE
WHEN CAST(SERVERPROPERTY(N'EngineEdition') AS integer) IN (3, 5, 8)
THEN N', ONLINE = ON'
ELSE N''
END;

DECLARE @index_sql nvarchar(max);

IF NOT EXISTS
(
SELECT
1/0
FROM sys.indexes
WHERE object_id = OBJECT_ID(N'collect.query_stats')
AND name = N'IX_query_stats_hash_lookup'
)
BEGIN
SET @index_sql = N'
CREATE NONCLUSTERED INDEX
IX_query_stats_hash_lookup
ON collect.query_stats
(query_hash, database_name, collection_time DESC)
WITH
(SORT_IN_TEMPDB = ON, DATA_COMPRESSION = PAGE' + @online_option + N');';
EXEC sys.sp_executesql @index_sql;
PRINT 'Created collect.query_stats.IX_query_stats_hash_lookup index';
END;

IF NOT EXISTS
(
SELECT
1/0
FROM sys.indexes
WHERE object_id = OBJECT_ID(N'collect.procedure_stats')
AND name = N'IX_procedure_stats_name_lookup'
)
BEGIN
SET @index_sql = N'
CREATE NONCLUSTERED INDEX
IX_procedure_stats_name_lookup
ON collect.procedure_stats
(database_name, schema_name, object_name, collection_time DESC)
WITH
(SORT_IN_TEMPDB = ON, DATA_COMPRESSION = PAGE' + @online_option + N');';
EXEC sys.sp_executesql @index_sql;
PRINT 'Created collect.procedure_stats.IX_procedure_stats_name_lookup index';
END;

IF NOT EXISTS
(
SELECT
1/0
FROM sys.indexes
WHERE object_id = OBJECT_ID(N'collect.query_store_data')
AND name = N'IX_query_store_data_id_lookup'
)
BEGIN
SET @index_sql = N'
CREATE NONCLUSTERED INDEX
IX_query_store_data_id_lookup
ON collect.query_store_data
(database_name, query_id, collection_time DESC)
WITH
(SORT_IN_TEMPDB = ON, DATA_COMPRESSION = PAGE' + @online_option + N');';
EXEC sys.sp_executesql @index_sql;
PRINT 'Created collect.query_store_data.IX_query_store_data_id_lookup index';
END;

PRINT 'All collection tables created successfully';
GO
Copy link
Copy Markdown
Owner Author

Choose a reason for hiding this comment

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

Schema upgrade path is correctly handled — this install block is mirrored by upgrades/2.7.0-to-2.8.0/01_add_query_lookup_indexes.sql and both are idempotent via IF NOT EXISTS on sys.indexes. Two small things:

  1. The index definitions now live in two places. If a future change flips DATA_COMPRESSION or column order for one index, both files have to be updated in lockstep — there's no single source of truth. Not a blocker for 2.8.0, but worth noting.
  2. EngineEdition 3 = Enterprise/Developer, 5 = Azure SQL DB, 8 = Managed Instance. Standard/Web (2) and Express (4) correctly get the no-ONLINE path. Looks right for the supported matrix.

Generated by Claude Code

Lite webhook URLs still read from plaintext settings — avoids implying
the security hardening shipped to both editions.

Co-authored-by: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@erikdarlingdata erikdarlingdata merged commit de610a1 into main Apr 22, 2026
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants