Skip to content

Pre-filter query snapshots into #temp on Azure SQL DB (#857) #869

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
erikdarlingdata merged 1 commit into from
Apr 20, 2026
Merged

Pre-filter query snapshots into #temp on Azure SQL DB (#857) #869

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
121 changes: 113 additions & 8 deletions Lite/Services/RemoteCollectorService.QuerySnapshots.cs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ namespace PerformanceMonitorLite.Services;
public partial class RemoteCollectorService
{
private const string QuerySnapshotsBase = """

SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED;
SET LOCK_TIMEOUT 1000;

Expand Down Expand Up @@ -81,17 +81,124 @@ WHERE der.session_id <> @@SPID
AND der.session_id >= 50
AND dest.text IS NOT NULL
AND der.database_id <> ISNULL(DB_ID(N'PerformanceMonitor'), 0)
{2}
ORDER BY der.cpu_time DESC, der.parallel_worker_count DESC
OPTION(MAXDOP 1, RECOMPILE);
""";
private readonly static CompositeFormat QuerySnapshotsBaseFormat = CompositeFormat.Parse(QuerySnapshotsBase);

/*
* On Azure SQL Database with a contained / DB-scoped login (e.g. D365FO),
* the OUTER APPLY to sys.dm_exec_sql_text / sys.dm_exec_text_query_plan
* will be evaluated against master-scoped rows from sys.dm_exec_requests
* before the WHERE predicate applies, tripping error 300 (VIEW SERVER
* PERFORMANCE STATE denied). Materialising the filtered request rows
* into a #temp table first guarantees the DMFs only see handles from
* the current database. See #857.
*/
private const string QuerySnapshotsAzureBase = """

SET TRANSACTION ISOLATION LEVEL READ UNCOMMITTED;
SET LOCK_TIMEOUT 1000;

IF OBJECT_ID(N'tempdb..#req') IS NOT NULL DROP TABLE #req;
Copy link
Copy Markdown
Owner Author

@erikdarlingdata erikdarlingdata Apr 20, 2026

Choose a reason for hiding this comment

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

Good call keeping the defensive pre-drop — SqlClient pools connections, so a prior execution that threw before line 190 would leave #req on a pooled session. The paired DROP TABLE #req; at the tail is good hygiene too. If you want belt-and-braces, wrapping the body in BEGIN TRY / BEGIN CATCH with a cleanup DROP in the catch would prevent the next call on the same pooled connection from paying the pre-drop cost, but it isn't necessary.

Generated by Claude Code


SELECT
der.session_id,
der.database_id,
der.sql_handle,
der.plan_handle,
der.statement_start_offset,
der.statement_end_offset,
der.status,
der.blocking_session_id,
der.wait_type,
der.wait_time,
der.wait_resource,
der.cpu_time,
der.total_elapsed_time,
der.reads,
der.writes,
der.logical_reads,
der.granted_query_memory,
der.transaction_isolation_level,
der.dop,
der.parallel_worker_count,
der.percent_complete
INTO #req
FROM sys.dm_exec_requests AS der
WHERE der.session_id <> @@SPID
AND der.session_id >= 50
AND der.database_id = DB_ID()
AND der.database_id <> ISNULL(DB_ID(N'PerformanceMonitor'), 0);
Copy link
Copy Markdown
Owner Author

@erikdarlingdata erikdarlingdata Apr 20, 2026

Choose a reason for hiding this comment

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

Minor: der.database_id <> ISNULL(DB_ID(N'PerformanceMonitor'), 0) is vestigial in the Lite path — Lite targets don't host a PerformanceMonitor database (Lite uses local DuckDB), so DB_ID('PerformanceMonitor') is always NULL and the predicate reduces to database_id <> 0. Combined with der.database_id = DB_ID() one line up, it's effectively a no-op. Harmless, but you could drop it to keep the Azure pre-filter tight. Same applies to line 83 in the boxed template, though that's out of scope here.

Generated by Claude Code


SELECT /* PerformanceMonitorLite */
der.session_id,
database_name = d.name,
elapsed_time_formatted =
CASE
WHEN der.total_elapsed_time < 0
THEN '00 00:00:00.000'
ELSE RIGHT(REPLICATE('0', 2) + CONVERT(varchar(10), der.total_elapsed_time / 86400000), 2) +
' ' + RIGHT(CONVERT(varchar(30), DATEADD(second, der.total_elapsed_time / 1000, 0), 120), 9) +
'.' + RIGHT('000' + CONVERT(varchar(3), der.total_elapsed_time % 1000), 3)
END,
query_text = SUBSTRING(dest.text, (der.statement_start_offset / 2) + 1,
((CASE der.statement_end_offset WHEN -1 THEN DATALENGTH(dest.text)
ELSE der.statement_end_offset END - der.statement_start_offset) / 2) + 1),
query_plan = TRY_CAST(deqp.query_plan AS nvarchar(max)),
{0}
der.status,
der.blocking_session_id,
der.wait_type,
wait_time_ms = CONVERT(bigint, der.wait_time),
der.wait_resource,
cpu_time_ms = CONVERT(bigint, der.cpu_time),
total_elapsed_time_ms = CONVERT(bigint, der.total_elapsed_time),
der.reads,
der.writes,
der.logical_reads,
granted_query_memory_gb = CONVERT(decimal(38, 2), (der.granted_query_memory / 128. / 1024.)),
transaction_isolation_level =
CASE der.transaction_isolation_level
WHEN 0 THEN 'Unspecified'
WHEN 1 THEN 'Read Uncommitted'
WHEN 2 THEN 'Read Committed'
WHEN 3 THEN 'Repeatable Read'
WHEN 4 THEN 'Serializable'
WHEN 5 THEN 'Snapshot'
ELSE '???'
END,
der.dop,
der.parallel_worker_count,
des.login_name,
des.host_name,
des.program_name,
des.open_transaction_count,
der.percent_complete
Comment on lines +134 to +177
Copy link
Copy Markdown
Owner Author

@erikdarlingdata erikdarlingdata Apr 20, 2026

Choose a reason for hiding this comment

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

The final projection here is a near byte-for-byte copy of the projection in QuerySnapshotsBase (lines 28–71). Any future column add/rename/type change now has to land in two templates, and the Lite collector's AppendValue ordinal reads (lines 257–281) are driven off both. Worth a TODO or a comment pointer between the two so a drift is at least obvious. Not a blocker for this PR.

Generated by Claude Code

FROM #req AS der
JOIN sys.dm_exec_sessions AS des
ON des.session_id = der.session_id
OUTER APPLY sys.dm_exec_sql_text(COALESCE(der.sql_handle, der.plan_handle)) AS dest
OUTER APPLY sys.dm_exec_text_query_plan(der.plan_handle, der.statement_start_offset, der.statement_end_offset) AS deqp
LEFT JOIN sys.databases AS d
ON d.database_id = der.database_id
{1}
WHERE dest.text IS NOT NULL
ORDER BY der.cpu_time DESC, der.parallel_worker_count DESC
OPTION(MAXDOP 1, RECOMPILE);

DROP TABLE #req;
""";
private readonly static CompositeFormat QuerySnapshotsAzureBaseFormat = CompositeFormat.Parse(QuerySnapshotsAzureBase);
Copy link
Copy Markdown
Owner Author

@erikdarlingdata erikdarlingdata Apr 20, 2026

Choose a reason for hiding this comment

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

No unit test covers the new Azure template branch. BuildQuerySnapshotsQuery(_, isAzureSqlDatabase: true) with both supportsLiveQueryPlan values is now a meaningfully different code path (different template, different placeholder count) and would regress silently if {0}/{1} drift. A couple of golden-string/contains assertions in Lite.Tests — e.g. "contains INTO #req", "contains DB_ID()", "does not contain {2}" — would be cheap insurance.

Generated by Claude Code


/// <summary>
/// Builds the query snapshots SQL with or without live query plan support.
/// Used by both the collector and the live snapshot button.
/// On Azure SQL Database the result set is scoped to the current database only,
/// because logins without access to master can't resolve cross-database requests (see #857).
/// On Azure SQL Database the request rows are first materialised into a
/// #temp table scoped to the current database, so the downstream OUTER APPLYs
/// to sys.dm_exec_sql_text / sys.dm_exec_text_query_plan only ever see
/// current-DB handles — avoiding the VIEW SERVER PERFORMANCE STATE error
/// that D365FO-style DB-scoped logins hit (see #857).
/// </summary>
internal static string BuildQuerySnapshotsQuery(bool supportsLiveQueryPlan, bool isAzureSqlDatabase)
{
Expand All @@ -101,10 +208,8 @@ internal static string BuildQuerySnapshotsQuery(bool supportsLiveQueryPlan, bool
var liveQueryPlanApply = supportsLiveQueryPlan
? "OUTER APPLY sys.dm_exec_query_statistics_xml(der.session_id) AS deqs"
: "";
var azureScopePredicate = isAzureSqlDatabase
? "AND der.database_id = DB_ID()"
: "";
return string.Format(null, QuerySnapshotsBaseFormat, liveQueryPlanColumn, liveQueryPlanApply, azureScopePredicate);
var template = isAzureSqlDatabase ? QuerySnapshotsAzureBaseFormat : QuerySnapshotsBaseFormat;
return string.Format(null, template, liveQueryPlanColumn, liveQueryPlanApply);
}

/// <summary>
Expand Down
Loading