From be1a9c846e270025a58aec8c68bf6eda366c99cc Mon Sep 17 00:00:00 2001
From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com>
Date: Wed, 9 Sep 2026 12:11:55 -0400
Subject: [PATCH 01/14] Record which credential DefaultAzureCredential
selected, on an event-id allowlist
Lite's EntraDefaultCredential mode signs in as whichever Azure identity the
machine already has, in an order the driver owns and the app cannot narrow.
#3218 disclosed that in dialog text; nothing recorded which identity won.
A narrowly-scoped EventListener enables the Azure-Identity source for exactly
the duration of one connection open, forwards Azure.Identity's event 13 and
nothing else, and writes the selected credential's type name through AppLogger.
The filter is an allowlist on the event id because the level cannot narrow this
- event 13 is itself Informational, so that is the lowest level that delivers it
- and no event in that source declares Keywords, so EnableEvents has no
dimension to exclude the sensitive siblings on.
---
.../EntraCredentialSelectionModeGateTests.cs | 110 +++
Lite.Tests/EntraCredentialSelectionTests.cs | 695 ++++++++++++++++++
Lite/Helpers/EntraCredentialSelection.cs | 352 +++++++++
Lite/Services/ServerManager.cs | 13 +-
Lite/Windows/AddServerDialog.xaml.cs | 16 +-
5 files changed, 1183 insertions(+), 3 deletions(-)
create mode 100644 Lite.Tests/EntraCredentialSelectionModeGateTests.cs
create mode 100644 Lite.Tests/EntraCredentialSelectionTests.cs
create mode 100644 Lite/Helpers/EntraCredentialSelection.cs
diff --git a/Lite.Tests/EntraCredentialSelectionModeGateTests.cs b/Lite.Tests/EntraCredentialSelectionModeGateTests.cs
new file mode 100644
index 000000000..a43ec645f
--- /dev/null
+++ b/Lite.Tests/EntraCredentialSelectionModeGateTests.cs
@@ -0,0 +1,110 @@
+/*
+ * 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.Linq;
+using System.Reflection;
+using Microsoft.Data.SqlClient;
+using PerformanceMonitor.Common;
+using PerformanceMonitorLite.Helpers;
+using PerformanceMonitorLite.Models;
+using Xunit;
+
+namespace PerformanceMonitorLite.Tests;
+
+///
+/// The one link cannot make: that the app's OWN
+/// authentication mode reaches the credential-selection listener.
+///
+/// EntraCredentialSelectionLog.Begin keys off
+/// rather than off the mode string, because
+/// that is the keyword the driver acts on and it therefore cannot disagree with the connection
+/// string. The cost of that choice is one degree of indirection between
+/// and the gate — so these pins run the
+/// real and close it.
+///
+/// Separate file because it needs , whose closure reaches
+/// Windows-only credential storage; the listener pins themselves need none of that and run in a
+/// plain net10.0 harness on any platform.
+///
+public class EntraCredentialSelectionModeGateTests
+{
+ ///
+ /// Derived from by reflection rather than from a list
+ /// typed here, so a seventh mode is a decision this test forces rather than a row nobody added.
+ /// The count FLOOR is the positive control: "exactly one mode is instrumented" is also true of a
+ /// reflection helper that enumerated one mode, or none — #3218 shipped a count pin whose helper
+ /// enumerated nothing and passed.
+ ///
+ /// Every arm is handed a username, a password, an Azure client id AND a managed-identity
+ /// client id, so a mode whose arm was copied from a neighbour still gets everything it could
+ /// need and the gate's answer is about the mode rather than about missing inputs.
+ ///
+ [Fact]
+ public void ExactlyOneAuthenticationMode_AttachesTheListener()
+ {
+ var modes = AllAuthenticationModes();
+
+ Assert.True(
+ modes.Count >= 6,
+ $"reflected {modes.Count} authentication modes off AuthenticationTypes; the sweep below is "
+ + "vacuous unless it is enumerating the real set");
+
+ var instrumented = new List();
+ foreach (var mode in modes)
+ {
+ var builder = new SqlConnectionStringBuilder { DataSource = "example-server" };
+ ServerConnection.ApplyAuthentication(
+ builder,
+ mode,
+ username: "someone",
+ password: "a-secret",
+ azureClientId: "an-azure-client-id",
+ managedIdentityClientId: "a-managed-identity-client-id");
+
+ using var listener = EntraCredentialSelectionLog.Begin(builder);
+ if (listener is not null)
+ {
+ instrumented.Add(mode);
+ }
+ }
+
+ Assert.Equal(new[] { AuthenticationTypes.EntraDefaultCredential }, instrumented);
+ }
+
+ ///
+ /// The same claim stated positively and on its own, so a sweep that silently stopped covering
+ /// this mode cannot leave the feature untested — Assert.Equal on an empty list against an
+ /// empty expectation is not a failure any sweep would report here, but it is a failure of this.
+ ///
+ [Fact]
+ public void TheEntraDefaultCredentialMode_AttachesTheListener()
+ {
+ var builder = new SqlConnectionStringBuilder { DataSource = "example-server" };
+ ServerConnection.ApplyAuthentication(
+ builder,
+ AuthenticationTypes.EntraDefaultCredential,
+ username: "someone",
+ password: "a-secret",
+ azureClientId: "an-azure-client-id",
+ managedIdentityClientId: "a-managed-identity-client-id");
+
+ Assert.Equal(SqlAuthenticationMethod.ActiveDirectoryDefault, builder.Authentication);
+
+ using var listener = EntraCredentialSelectionLog.Begin(builder);
+ Assert.NotNull(listener);
+ }
+
+ private static List AllAuthenticationModes() =>
+ typeof(AuthenticationTypes)
+ .GetFields(BindingFlags.Public | BindingFlags.Static)
+ .Where(f => f.IsLiteral && !f.IsInitOnly && f.FieldType == typeof(string))
+ .Select(f => (string)f.GetRawConstantValue()!)
+ .ToList();
+}
diff --git a/Lite.Tests/EntraCredentialSelectionTests.cs b/Lite.Tests/EntraCredentialSelectionTests.cs
new file mode 100644
index 000000000..09b4cfa9c
--- /dev/null
+++ b/Lite.Tests/EntraCredentialSelectionTests.cs
@@ -0,0 +1,695 @@
+/*
+ * 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.Concurrent;
+using System.Collections.Generic;
+using System.Diagnostics.Tracing;
+using System.IO;
+using System.Linq;
+using System.Reflection;
+using System.Runtime.CompilerServices;
+using Darling.Tests;
+using Microsoft.Data.SqlClient;
+using Microsoft.Extensions.Logging;
+using PerformanceMonitorLite.Helpers;
+using PerformanceMonitorLite.Services;
+using Xunit;
+
+namespace PerformanceMonitorLite.Tests;
+
+///
+/// #3218 shipped EntraDefaultCredential and disclosed, in dialog text, that the credential
+/// search order is the driver's and that a machine with several Azure identities connects as
+/// whichever comes first. Nothing recorded WHICH. This is the recording.
+///
+/// Every pin here raises the real event on the real event source.
+/// Azure.Identity's AzureIdentityEventSource is reached by reflection — it is
+/// internal, so this is the only way in from outside the assembly — and its actual
+/// DefaultAzureCredentialCredentialSelected method is invoked. So the id, the level, the
+/// payload shape and the source name are the ones that ship rather than a stand-in's, and an
+/// upstream move fails these tests instead of passing them.
+///
+/// The failure mode this file exists to avoid. A listener test that constructs a
+/// listener and asserts nothing was logged passes trivially in a harness where no
+/// DefaultAzureCredential ever runs — the shape that put two vacuous pins into #3218. So
+/// every negative here is paired with a positive on the SAME listener: the sibling event is raised
+/// first and must not be forwarded, then event 13 is raised and must be. A listener that received
+/// nothing fails the second half, which is what makes the first half evidence.
+///
+/// What is NOT claimed. That a real Entra tenant issues a token to a real
+/// az login session through this path — still unverified, still needs a tenant and a Windows
+/// host. This file answers only for what the listener does with the event once it is raised.
+///
+public sealed class EntraCredentialSelectionTests : IDisposable
+{
+ /* Azure.Identity 1.18.0, AzureIdentityEventSource.cs — every id and signature reflected below is
+ read from that file at tag Azure.Identity_1.18.0, which is what this repository resolves
+ transitively through Microsoft.Data.SqlClient.Extensions.Azure 7.0.2 (see #3219: it is not
+ pinned, which is exactly why the upstream contract is pinned HERE). */
+ private const string EventSourceTypeName = "Azure.Identity.AzureIdentityEventSource, Azure.Identity";
+
+ /// :307 — the event this listener exists for. One string credentialType.
+ private const string SelectedMethod = "DefaultAzureCredentialCredentialSelected";
+
+ ///
+ /// :371 — event 18, Informational, and it carries TENANT IDS. The sibling that says why
+ /// the callback filter has to be the barrier.
+ ///
+ private const string TenantIdMethod = "TenantIdDiscoveredAndUsed";
+
+ ///
+ /// :427 — event 26, Informational, and its FIRST payload slot is also called
+ /// credentialType and holds a real credential type name. The sharpest negative control
+ /// available: it passes the type-name shape check, it passes the level, it passes the source
+ /// name, and the ONLY thing that keeps it out of the log is the event-id allowlist.
+ ///
+ private const string ManagedIdentitySelectedMethod = "ManagedIdentityCredentialSelected";
+
+ /// :399 — event 21, Warning. Proves the level admits MORE than Informational.
+ private const string WarningMethod = "UserAssignedManagedIdentityNotSupported";
+
+ /// A plausible winner, and the value every positive assertion looks for.
+ private const string CliCredential = "Azure.Identity.AzureCliCredential";
+
+ ///
+ /// The process-wide minimum is restored, and the last-reported name forgotten, because both are
+ /// static and a leak from here would change what a neighbouring test observes.
+ ///
+ public void Dispose()
+ {
+ AppLogger.SetMinimumLevel(AppLogger.DefaultMinimumLevel);
+ EntraCredentialSelectionLog.ResetForTests();
+ }
+
+ // ---- The upstream contract this listener is built on ---------------------------------
+
+ ///
+ /// The four facts the listener hard-codes, read off the shipped assembly rather than off a
+ /// version note. Azure.Identity arrives transitively and unpinned (#3219), so a SqlClient
+ /// bump can move it with no code change anywhere — this is the test that makes such a move loud.
+ ///
+ [Fact]
+ public void TheUpstreamEventContract_IsWhatTheListenerAssumes()
+ {
+ var type = EventSourceType();
+ var source = Singleton();
+
+ Assert.Equal(
+ EntraCredentialSelectionListener.AzureIdentitySourceName,
+ source.Name);
+
+ var method = type.GetMethod(SelectedMethod, BindingFlags.Public | BindingFlags.Instance);
+ Assert.NotNull(method);
+
+ var parameters = method!.GetParameters();
+ Assert.Single(parameters);
+ Assert.Equal(typeof(string), parameters[0].ParameterType);
+
+ var attribute = method.GetCustomAttribute();
+ Assert.NotNull(attribute);
+ Assert.Equal(EntraCredentialSelectionListener.CredentialSelectedEventId, attribute!.EventId);
+ Assert.Equal(EventLevel.Informational, attribute.Level);
+ }
+
+ ///
+ /// Why the filter is an allowlist and not a level or a keyword. Not one event in
+ /// that source declares Keywords, so EnableEvents has no dimension to exclude the
+ /// sensitive siblings on; and event 13 is itself Informational, so there is no level that
+ /// admits it and not them.
+ ///
+ /// The positive control is the event COUNT: "no event declares keywords" is also true of a
+ /// type with no events, which is not the claim. And the second assertion is a FLOOR on how many
+ /// events share event 13's level, not the exact number — the exact number (20 of 29 at
+ /// Informational in 1.18.0) is a measurement that goes stale on any upstream bump, while
+ /// "event 13 is not alone at its level" is the property the design rests on and does not.
+ ///
+ [Fact]
+ public void NoEventInThatSourceDeclaresKeywords_AndEventThirteenIsNotAloneAtItsLevel()
+ {
+ var events = EventSourceType()
+ .GetMethods(BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance)
+ .Select(m => m.GetCustomAttribute())
+ .Where(a => a is not null)
+ .Select(a => a!)
+ .ToList();
+
+ Assert.True(
+ events.Count > 1,
+ $"reflected {events.Count} [Event]-attributed methods on the Azure-Identity source; the "
+ + "keyword assertion below is vacuous unless there are events to assert about");
+
+ var withKeywords = events
+ .Where(a => a.Keywords != EventKeywords.None)
+ .Select(a => a.EventId)
+ .ToList();
+
+ Assert.True(
+ withKeywords.Count == 0,
+ "an event in Azure-Identity now declares Keywords (ids: "
+ + string.Join(", ", withKeywords)
+ + "). EnableEvents could then exclude the sensitive siblings on that dimension, which "
+ + "is a better barrier than a callback filter — reconsider the design rather than "
+ + "just updating this number.");
+
+ var atInformational = events
+ .Count(a => a.Level == EventLevel.Informational);
+
+ Assert.True(
+ atInformational > 1,
+ "event 13 is now the only Informational event in Azure-Identity, so the level alone would "
+ + $"isolate it (counted {atInformational}). The allowlist is still correct but its "
+ + "stated reason has changed.");
+ }
+
+ // ---- The mechanism working, and the sibling that must not ----------------------------
+
+ ///
+ /// The positive half, and the hazard-1 falsifier at the same time. The event source is
+ /// forced to EXIST before the listener is constructed, so
+ /// EventListener.OnEventSourceCreated is reached from the BASE CONSTRUCTOR — the path on
+ /// which a derived field assigned in a constructor body is still null. If the listener ever grows
+ /// a constructor that the callback depends on, it stops enabling the source and this test goes
+ /// red rather than the app going quietly blind.
+ ///
+ [Fact]
+ public void EventThirteen_ArrivesAsTheCredentialTypeName_WithTheSourceAlreadyCreated()
+ {
+ var source = Singleton();
+ Assert.Equal(EntraCredentialSelectionListener.AzureIdentitySourceName, source.Name);
+
+ using var listener = new EntraCredentialSelectionListener();
+
+ Raise(SelectedMethod, CliCredential);
+
+ Assert.Equal(CliCredential, listener.SelectedCredentialType);
+ Assert.False(listener.RejectedPayload);
+ }
+
+ ///
+ /// Both directions on one listener, which is the whole point. Three siblings are
+ /// raised first and none may be forwarded; then event 13 is raised and must be. A listener that
+ /// simply received nothing — the vacuous shape — fails the final assertion, so the three
+ /// preceding ones are evidence of filtering rather than of silence.
+ ///
+ /// The siblings are chosen to defeat each barrier in turn:
+ /// ManagedIdentityCredentialSelected's first payload slot IS a valid credential type name,
+ /// so only the id keeps it out; TenantIdDiscoveredAndUsed carries tenant ids, which is the
+ /// value that must never reach the log; and UserAssignedManagedIdentityNotSupported is at
+ /// Warning, proving the enabled level admits events more severe than the one requested and
+ /// not merely the one asked for.
+ ///
+ [Fact]
+ public void SensitiveSiblings_AreNotForwarded_AndTheSameListenerStillCapturesEventThirteen()
+ {
+ Singleton();
+ using var listener = new EntraCredentialSelectionListener();
+
+ Raise(ManagedIdentitySelectedMethod, "Azure.Identity.ManagedIdentityCredential", "some-id");
+ Assert.Null(listener.SelectedCredentialType);
+ Assert.False(listener.RejectedPayload);
+
+ Raise(TenantIdMethod, "00000000-0000-0000-0000-000000000000", "11111111-1111-1111-1111-111111111111");
+ Assert.Null(listener.SelectedCredentialType);
+
+ Raise(WarningMethod, "SomeEnvironment");
+ Assert.Null(listener.SelectedCredentialType);
+
+ /* The discriminator. Everything above is consistent with a listener that never enabled the
+ source at all until this line succeeds. */
+ Raise(SelectedMethod, CliCredential);
+ Assert.Equal(CliCredential, listener.SelectedCredentialType);
+ }
+
+ ///
+ /// An INDEPENDENT instrument for the claim the allowlist rests on: that the siblings really
+ /// do reach a callback at this level, rather than being filtered out before it. A recording
+ /// listener enabling the same source at the same level records every id it sees; the production
+ /// listener over the identical three events keeps one.
+ ///
+ /// Without this, "the sibling was not forwarded" could equally mean the level never
+ /// delivered it — in which case the callback filter would be doing nothing and could be deleted
+ /// without any test noticing.
+ ///
+ [Fact]
+ public void TheEnabledLevelDeliversTheSiblingsToTheCallback_AndOnlyEventThirteenIsKept()
+ {
+ Singleton();
+
+ using var recorder = new RecordingListener();
+ using var listener = new EntraCredentialSelectionListener();
+
+ Raise(SelectedMethod, CliCredential);
+ Raise(TenantIdMethod, "00000000-0000-0000-0000-000000000000", "11111111-1111-1111-1111-111111111111");
+ Raise(WarningMethod, "SomeEnvironment");
+
+ var seen = recorder.SeenEventIds;
+
+ Assert.Contains(EntraCredentialSelectionListener.CredentialSelectedEventId, seen);
+ Assert.True(
+ seen.Count >= 3,
+ "the recording listener saw " + string.Join(", ", seen.Order())
+ + " — fewer than the three events raised, so the level did not deliver the siblings and "
+ + "the allowlist below is not what excluded them");
+
+ Assert.Equal(CliCredential, listener.SelectedCredentialType);
+ }
+
+ ///
+ /// The listener enables Azure-Identity and nothing else. Enabling an
+ /// is process-global in effect — once enabled, IsEnabled()
+ /// answers true inside the owning library and it performs formatting work it otherwise skips. A
+ /// name check dropped from OnEventSourceCreated would turn this into a listener that
+ /// enables EVERY event source in the process at Informational, and every other pin in this
+ /// file would stay green: the callback allowlist would still keep the log clean, so the only
+ /// symptom would be cost paid forever by every instrumented library in the app.
+ ///
+ /// The control is the first assertion: a source that is not enabled while the listener is
+ /// alive is only evidence if some source demonstrably IS.
+ ///
+ [Fact]
+ public void TheListenerEnablesAzureIdentity_AndNoOtherEventSource()
+ {
+ var azureIdentity = Singleton();
+ using var bystander = new BystanderEventSource();
+
+ using var listener = new EntraCredentialSelectionListener();
+
+ Assert.True(
+ azureIdentity.IsEnabled(EventLevel.Informational, EventKeywords.None),
+ "the listener did not enable Azure-Identity, so the assertion below proves nothing");
+
+ Assert.False(
+ bystander.IsEnabled(),
+ $"the listener enabled {bystander.Name} as well as Azure-Identity. Enabling a source is "
+ + "process-global in effect, so this is formatting work paid by an unrelated library "
+ + "on every event for the life of every connection attempt.");
+ }
+
+ // ---- The shape check, which is the barrier against a payload REORDER -----------------
+
+ [Theory]
+ [InlineData("Azure.Identity.AzureCliCredential")]
+ [InlineData("Azure.Identity.EnvironmentCredential")]
+ [InlineData("Azure.Identity.Outer+Inner")]
+ [InlineData("A.B")]
+ [InlineData("Azure.Identity.Some_Credential2")]
+ public void IsCredentialTypeName_TakesATypeName(string value) =>
+ Assert.True(EntraCredentialSelectionLog.IsCredentialTypeName(value));
+
+ ///
+ /// Every rejected case is the shape of a real payload carried by a sibling event in that source
+ /// at a level this listener enables, so each one is a value that a reordered or reshaped payload
+ /// could actually put in front of the read.
+ ///
+ [Theory]
+ [InlineData(null)]
+ [InlineData("")]
+ [InlineData("00000000-0000-0000-0000-000000000000")] // a tenant id: hyphens
+ [InlineData("someone@example.com")] // an account upn: @
+ [InlineData("https://database.windows.net/.default")] // a scope: : and /
+ [InlineData("DefaultAzureCredential failed to retrieve a token")] // a message: spaces
+ [InlineData("AzureCliCredential")] // no namespace at all
+ [InlineData(".Leading")]
+ [InlineData("Trailing.")]
+ [InlineData("Azure.Identity.Thing`1[[System.String, System.Private.CoreLib]]")]
+ public void IsCredentialTypeName_RejectsEverySensitiveSiblingShape(string? value) =>
+ Assert.False(EntraCredentialSelectionLog.IsCredentialTypeName(value));
+
+ [Fact]
+ public void IsCredentialTypeName_RejectsSomethingLongerThanAnyTypeName() =>
+ Assert.False(EntraCredentialSelectionLog.IsCredentialTypeName(
+ "Azure.Identity." + new string('x', EntraCredentialSelectionLog.MaxCredentialTypeNameLength)));
+
+ ///
+ /// A real event 13 whose payload is a tenant id rather than a type name — which is what a
+ /// future upstream parameter reorder looks like from here. It must be declined, the value must
+ /// not be retained, and the resulting log line must not contain it.
+ ///
+ /// The positive control is the second half: the same listener then takes a well-shaped
+ /// value, so "declined" is a decision about the payload and not a listener that stopped
+ /// working.
+ ///
+ [Fact]
+ public void AReorderedPayload_IsDeclinedAndNeverReachesTheLine()
+ {
+ const string TenantIdShaped = "00000000-0000-0000-0000-000000000000";
+
+ Singleton();
+ using var listener = new EntraCredentialSelectionListener();
+
+ Raise(SelectedMethod, TenantIdShaped);
+
+ Assert.Null(listener.SelectedCredentialType);
+ Assert.True(listener.RejectedPayload);
+
+ var line = EntraCredentialSelectionLog.Decide(
+ listener.SelectedCredentialType, listener.RejectedPayload, lastReported: null);
+
+ Assert.Equal(LogLevel.Debug, line.Level);
+ Assert.DoesNotContain(TenantIdShaped, line.Message, StringComparison.Ordinal);
+ Assert.Null(line.Reported);
+
+ Raise(SelectedMethod, CliCredential);
+ Assert.Equal(CliCredential, listener.SelectedCredentialType);
+ }
+
+ // ---- The gate: this mode and no other ------------------------------------------------
+
+ [Fact]
+ public void Begin_ReturnsAListener_ForActiveDirectoryDefault()
+ {
+ using var listener = EntraCredentialSelectionLog.Begin(
+ new SqlConnectionStringBuilder { Authentication = SqlAuthenticationMethod.ActiveDirectoryDefault });
+
+ Assert.NotNull(listener);
+ }
+
+ ///
+ /// Every other the driver defines, swept rather than
+ /// sampled, plus a builder that sets no keyword at all and a null builder. A mode added to the
+ /// enum upstream lands here as a new theory row automatically, and lands on the null arm — which
+ /// is the safe direction: it enables no event source.
+ ///
+ [Theory]
+ [MemberData(nameof(EveryOtherAuthenticationMethod))]
+ public void Begin_ReturnsNull_ForEveryOtherAuthenticationMethod(SqlAuthenticationMethod method)
+ {
+ var builder = new SqlConnectionStringBuilder { Authentication = method };
+
+ Assert.Null(EntraCredentialSelectionLog.Begin(builder));
+ }
+
+ public static TheoryData EveryOtherAuthenticationMethod()
+ {
+ var data = new TheoryData();
+ foreach (var method in Enum.GetValues())
+ {
+ if (method != SqlAuthenticationMethod.ActiveDirectoryDefault)
+ {
+ data.Add(method);
+ }
+ }
+
+ return data;
+ }
+
+ [Fact]
+ public void Begin_ReturnsNull_ForABuilderWithNoAuthenticationKeywordAndForNull()
+ {
+ Assert.Null(EntraCredentialSelectionLog.Begin(new SqlConnectionStringBuilder()));
+ Assert.Null(EntraCredentialSelectionLog.Begin(null));
+ }
+
+ [Fact]
+ public void Report_DoesNothingForANullListener() =>
+ EntraCredentialSelectionLog.Report(null);
+
+ // ---- What gets written, and at which level -------------------------------------------
+
+ [Fact]
+ public void Decide_ReportsAFirstObservationAtInformation()
+ {
+ var line = EntraCredentialSelectionLog.Decide(CliCredential, rejectedPayload: false, lastReported: null);
+
+ Assert.Equal(LogLevel.Information, line.Level);
+ Assert.Contains(CliCredential, line.Message, StringComparison.Ordinal);
+ Assert.Equal(CliCredential, line.Reported);
+ }
+
+ [Fact]
+ public void Decide_ReportsAnUnchangedRepeatAtDebug_AndRemembersNothingNew()
+ {
+ var line = EntraCredentialSelectionLog.Decide(
+ CliCredential, rejectedPayload: false, lastReported: CliCredential);
+
+ Assert.Equal(LogLevel.Debug, line.Level);
+ Assert.Null(line.Reported);
+ }
+
+ ///
+ /// A CHANGED selection is the one thing here worth an line
+ /// after the first: the driver clears its static credential cache, so a different source
+ /// genuinely can start winning mid-process, and that is a different identity connecting.
+ ///
+ [Fact]
+ public void Decide_ReportsAChangeAtInformation()
+ {
+ var line = EntraCredentialSelectionLog.Decide(
+ "Azure.Identity.EnvironmentCredential",
+ rejectedPayload: false,
+ lastReported: CliCredential);
+
+ Assert.Equal(LogLevel.Information, line.Level);
+ Assert.Contains("Azure.Identity.EnvironmentCredential", line.Message, StringComparison.Ordinal);
+ Assert.Equal("Azure.Identity.EnvironmentCredential", line.Reported);
+ }
+
+ ///
+ /// Nothing observed is a line saying so, not silence — and it names the reason, because "no line"
+ /// and "the event fires once per process" are indistinguishable to whoever reads the log.
+ ///
+ [Fact]
+ public void Decide_SaysNothingWasObserved_RatherThanSayingNothing()
+ {
+ var line = EntraCredentialSelectionLog.Decide(null, rejectedPayload: false, lastReported: null);
+
+ Assert.Equal(LogLevel.Debug, line.Level);
+ Assert.False(string.IsNullOrWhiteSpace(line.Message));
+ Assert.Null(line.Reported);
+ }
+
+ ///
+ /// End to end through the real , at the level a default install
+ /// runs at. The first observation must actually reach the log; the unchanged repeat must not.
+ /// Nothing else in this file would catch a that wrote every line at
+ /// — the messages would still be correct and the whole feature would
+ /// be invisible on every shipped install.
+ ///
+ [Fact]
+ public void Report_WritesTheFirstObservationAtTheDefaultLevel_AndNotTheRepeat()
+ {
+ Singleton();
+ EntraCredentialSelectionLog.ResetForTests();
+ Assert.Equal(LogLevel.Information, AppLogger.MinimumLevel);
+
+ AppLogger.DrainBufferedLines();
+
+ using (var first = new EntraCredentialSelectionListener())
+ {
+ Raise(SelectedMethod, CliCredential);
+ EntraCredentialSelectionLog.Report(first);
+ }
+
+ var written = Ours();
+ Assert.Single(written);
+ Assert.Contains(CliCredential, written[0], StringComparison.Ordinal);
+ Assert.Contains("INFO", written[0], StringComparison.Ordinal);
+
+ using (var again = new EntraCredentialSelectionListener())
+ {
+ Raise(SelectedMethod, CliCredential);
+ EntraCredentialSelectionLog.Report(again);
+ }
+
+ Assert.Empty(Ours());
+ }
+
+ ///
+ /// The same repeat, with the minimum lowered — so "not written at the default" above is a level
+ /// decision rather than a that produced no line at all.
+ ///
+ [Fact]
+ public void Report_WritesTheRepeatAndTheNotObservedLine_OnceDebugIsAdmitted()
+ {
+ Singleton();
+ EntraCredentialSelectionLog.ResetForTests();
+ AppLogger.SetMinimumLevel(LogLevel.Debug);
+ AppLogger.DrainBufferedLines();
+
+ using (var first = new EntraCredentialSelectionListener())
+ {
+ Raise(SelectedMethod, CliCredential);
+ EntraCredentialSelectionLog.Report(first);
+ }
+
+ Assert.Single(Ours());
+
+ using (var again = new EntraCredentialSelectionListener())
+ {
+ Raise(SelectedMethod, CliCredential);
+ EntraCredentialSelectionLog.Report(again);
+ }
+
+ var repeat = Ours();
+ Assert.Single(repeat);
+ Assert.Contains("DEBUG", repeat[0], StringComparison.Ordinal);
+
+ /* And the third arm: a listener that observed nothing at all, which is what every connection
+ after the first actually looks like. */
+ using (var silent = new EntraCredentialSelectionListener())
+ {
+ EntraCredentialSelectionLog.Report(silent);
+ }
+
+ var absent = Ours();
+ Assert.Single(absent);
+ Assert.Contains("DEBUG", absent[0], StringComparison.Ordinal);
+ Assert.DoesNotContain(CliCredential, absent[0], StringComparison.Ordinal);
+ }
+
+ // ---- Both connection-open sites are instrumented, in the right order -----------------
+
+ ///
+ /// Behavioural coverage cannot reach either site: one is a WPF Window needing a
+ /// dispatcher and a live form, the other needs a reachable SQL Server. So the call sites are
+ /// asserted in source, on StripCommentsAndStrings' output — which is what makes this a
+ /// check on CODE rather than on my own comments. Both sites carry a comment naming
+ /// EntraCredentialSelectionLog and explaining it, so a raw substring search over either
+ /// file would be satisfied by the comment alone and would pass with every call deleted. #3201's
+ /// equivalent pin did exactly that until the strip was added.
+ ///
+ /// The ORDER is the discriminating assertion. Both names being present is satisfied
+ /// by a Report placed before the open, which would capture nothing on every connection
+ /// forever while reading as fully instrumented.
+ ///
+ [Theory]
+ [InlineData("Lite/Services/ServerManager.cs", "CheckConnectionAsync(string serverId")]
+ [InlineData("Lite/Windows/AddServerDialog.xaml.cs", "RunConnectionTestAsync()")]
+ public void EveryConnectionOpenSite_BeginsBeforeTheOpenAndReportsAfterIt(
+ string relativePath, string methodAnchor)
+ {
+ var source = CSharpSourceWalker.StripCommentsAndStrings(ReadRepoFile(relativePath));
+
+ var signature = source.IndexOf(methodAnchor, StringComparison.Ordinal);
+ Assert.True(signature >= 0, $"{methodAnchor} is the connection-open method in {relativePath} and must exist");
+
+ var brace = source.IndexOf('{', signature);
+ Assert.True(brace > signature, $"{methodAnchor} must have a body");
+
+ var body = CSharpSourceWalker.BraceBalanced(source, brace);
+
+ var begin = body.IndexOf("EntraCredentialSelectionLog.Begin", StringComparison.Ordinal);
+ var open = body.IndexOf("connection.OpenAsync", StringComparison.Ordinal);
+ var report = body.IndexOf("EntraCredentialSelectionLog.Report", StringComparison.Ordinal);
+
+ Assert.True(open >= 0, $"{relativePath} must still open the connection in {methodAnchor}");
+ Assert.True(begin >= 0, $"{relativePath} must attach the credential-selection listener in {methodAnchor}");
+ Assert.True(report >= 0, $"{relativePath} must report the credential selection in {methodAnchor}");
+
+ Assert.True(
+ begin < open,
+ "the listener must be attached BEFORE the open, or Azure.Identity raises event 13 with "
+ + "nothing subscribed and the selection is never observed");
+ Assert.True(
+ open < report,
+ "the selection must be reported AFTER the open, or it is read before the event that "
+ + "produces it and every connection reports nothing");
+ }
+
+ // ---- Helpers -------------------------------------------------------------------------
+
+ ///
+ /// Azure.Identity's event source type. Asserted non-null rather than skipped: this whole
+ /// file is about a specific upstream event, and a harness that cannot see the assembly must fail
+ /// loudly rather than report an absence it has no instrument for (the #3218 lesson).
+ ///
+ private static Type EventSourceType()
+ {
+ var type = Type.GetType(EventSourceTypeName, throwOnError: false);
+
+ Assert.NotNull(type);
+ return type!;
+ }
+
+ ///
+ /// The real singleton, which also FORCES the event source to exist — so a listener constructed
+ /// after this call reaches OnEventSourceCreated from the base constructor.
+ ///
+ private static EventSource Singleton()
+ {
+ var property = EventSourceType().GetProperty("Singleton", BindingFlags.Public | BindingFlags.Static);
+ Assert.NotNull(property);
+
+ var value = property!.GetValue(null) as EventSource;
+ Assert.NotNull(value);
+ return value!;
+ }
+
+ /// Invokes one of the source's real event methods, so a real event is written.
+ private static void Raise(string methodName, params object[] arguments)
+ {
+ var method = EventSourceType().GetMethod(
+ methodName,
+ BindingFlags.Public | BindingFlags.NonPublic | BindingFlags.Instance,
+ arguments.Select(a => a.GetType()).ToArray());
+
+ Assert.NotNull(method);
+ method!.Invoke(Singleton(), arguments);
+ }
+
+ ///
+ /// This feature's lines out of the process-wide buffer, so a concurrently-running test's lines
+ /// are not mistaken for these and these are not mistaken for theirs.
+ ///
+ private static List Ours() =>
+ AppLogger.DrainBufferedLines()
+ .Where(l => l.Contains($"[{EntraCredentialSelectionLog.LogSource}]", StringComparison.Ordinal))
+ .ToList();
+
+ private static string ReadRepoFile(string relativePath, [CallerFilePath] string thisFile = "")
+ {
+ var dir = Path.GetDirectoryName(thisFile)!;
+ var parts = relativePath.Split('/');
+ while (dir is not null && !File.Exists(Path.Combine(new[] { dir }.Concat(parts).ToArray())))
+ {
+ dir = Path.GetDirectoryName(dir);
+ }
+
+ Assert.NotNull(dir);
+ return File.ReadAllText(Path.Combine(new[] { dir! }.Concat(parts).ToArray()));
+ }
+
+ ///
+ /// An independent instrument: records every event id the enabled level actually delivers, so the
+ /// production listener's filtering can be told apart from a level that delivered nothing.
+ ///
+ /// The bag is a FIELD INITIALISER, which in C# runs BEFORE the base constructor — the same
+ /// hazard the production listener avoids with consts. Assigned in a constructor body it would be
+ /// null when fires for an already-existing source, which is
+ /// exactly the case every test here sets up.
+ ///
+ ///
+ /// An unrelated event source, existing only so "the listener enabled nothing else" has something
+ /// to be false about. Named outside the Azure- family on purpose.
+ ///
+ [EventSource(Name = "PerformanceMonitorLite-Tests-Bystander")]
+ private sealed class BystanderEventSource : EventSource
+ {
+ }
+
+ private sealed class RecordingListener : EventListener
+ {
+ private readonly ConcurrentBag _seen = new();
+
+ internal List SeenEventIds => _seen.ToList();
+
+ protected override void OnEventSourceCreated(EventSource eventSource)
+ {
+ if (string.Equals(
+ eventSource?.Name,
+ EntraCredentialSelectionListener.AzureIdentitySourceName,
+ StringComparison.Ordinal))
+ {
+ EnableEvents(eventSource!, EventLevel.Informational);
+ }
+ }
+
+ protected override void OnEventWritten(EventWrittenEventArgs eventData) => _seen.Add(eventData.EventId);
+ }
+}
diff --git a/Lite/Helpers/EntraCredentialSelection.cs b/Lite/Helpers/EntraCredentialSelection.cs
new file mode 100644
index 000000000..0f09ae639
--- /dev/null
+++ b/Lite/Helpers/EntraCredentialSelection.cs
@@ -0,0 +1,352 @@
+/*
+ * 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.Diagnostics.Tracing;
+using Microsoft.Data.SqlClient;
+using Microsoft.Extensions.Logging;
+using PerformanceMonitorLite.Services;
+
+namespace PerformanceMonitorLite.Helpers;
+
+///
+/// Captures which credential DefaultAzureCredential actually selected, by listening to the
+/// one Azure-Identity event that says so.
+///
+/// Observability only. Nothing here participates in acquiring a token, building a
+/// connection string, or ordering the credential chain. Remove the type and the app connects
+/// identically; the only difference is that nobody can tell which identity it connected as.
+///
+/// The event. Azure.Identity 1.18.0,
+/// Credentials/DefaultAzureCredential.cs:178, on the success path immediately after
+/// GetTokenFromSourcesAsync picks a source:
+/// AzureIdentityEventSource.Singleton.DefaultAzureCredentialCredentialSelected(credential.GetType().FullName).
+/// Its declaration is AzureIdentityEventSource.cs:307 — event id 13
+/// (AzureIdentityEventSource.cs:34), Level = EventLevel.Informational, one
+/// string credentialType parameter — on the source named Azure-Identity
+/// (AzureIdentityEventSource.cs:18).
+///
+/// An ALLOWLIST on the event id, because the level cannot narrow this and keywords do not
+/// exist here. Event 13 is itself Informational, so Informational is the LOWEST
+/// level that delivers it — and
+/// admits every event at or above the requested severity, which in this source is 28 of its 29
+/// events: 20 at Informational (19 besides this one), plus 4 Warning, 2
+/// Error, 1 Critical and 1 LogAlways. Only the single Verbose event is
+/// excluded. Several of the 28 carry exactly what an application log must never hold:
+/// TenantIdDiscoveredAndUsed / TenantIdDiscoveredAndNotUsed carry tenant ids,
+/// AuthenticatedAccountDetails carries account details, GetTokenFailed carries a
+/// formatted Exception, the six MsalLog* events carry MSAL's own log lines, and
+/// GetToken / GetTokenSucceeded carry scopes and parent request ids.
+///
+/// And keyword filtering cannot help: not one of the 29 events in that file declares
+/// Keywords, so EnableEvents has no dimension to exclude the noisy siblings on. That
+/// makes 's filter the only barrier between this app's log and a tenant
+/// id — which is why it is an allowlist on rather than a
+/// denylist of the events known to be sensitive today. A denylist is the form that fails when a
+/// future Azure.Identity adds an event; an allowlist is the form that survives it, because
+/// the new event is not event 13.
+///
+/// What leaves this type is [0] and nothing
+/// else, and only when it has the shape of a CLR type name (see
+/// ). Never
+/// eventData.ToString(), never the payload collection, never
+/// formatted with its arguments.
+///
+internal sealed class EntraCredentialSelectionListener : EventListener
+{
+ ///
+ /// Azure.Identity 1.18.0, AzureIdentityEventSource.cs:18. Matched
+ /// : this is a wire identifier, not display text.
+ ///
+ internal const string AzureIdentitySourceName = "Azure-Identity";
+
+ ///
+ /// Azure.Identity 1.18.0, AzureIdentityEventSource.cs:34
+ /// (DefaultAzureCredentialCredentialSelectedEvent). The whole allowlist.
+ ///
+ internal const int CredentialSelectedEventId = 13;
+
+ /* Both values the OnEventSourceCreated callback needs are const, and that is the mitigation for
+ the classic EventListener bug rather than an accident of style. EventListener's BASE
+ CONSTRUCTOR calls OnEventSourceCreated for every EventSource that already exists in the
+ process - and a C# derived-class constructor BODY runs after the base constructor, so a source
+ name or event id assigned there is still null/0 when that callback fires. The symptom is not an
+ exception: the listener simply never enables the source and silently observes nothing forever.
+ Azure-Identity is a lazily-created singleton, so whether it pre-exists depends on whether
+ anything has touched Azure.Identity yet - which makes this a bug that appears and disappears
+ with unrelated ordering. Nothing on this type is assigned in a constructor, there is no
+ constructor, and EntraCredentialSelectionListenerTests constructs the listener AFTER forcing
+ the source to exist, which is the path that goes through the base constructor. */
+
+ private volatile string? _selected;
+ private volatile bool _rejectedPayload;
+
+ ///
+ /// The credential type name event 13 reported, or null if it has not arrived (or arrived in a
+ /// shape this type declines to forward — see ).
+ ///
+ internal string? SelectedCredentialType => _selected;
+
+ ///
+ /// True when event 13 arrived but its first payload slot was not something this type is willing
+ /// to log. Kept separate from "nothing arrived" so a future payload change is a legible signal
+ /// rather than silence — an absence and a refusal need different answers, and the refused value
+ /// itself is never retained.
+ ///
+ internal bool RejectedPayload => _rejectedPayload;
+
+ ///
+ protected override void OnEventSourceCreated(EventSource eventSource)
+ {
+ if (eventSource is null ||
+ !string.Equals(eventSource.Name, AzureIdentitySourceName, StringComparison.Ordinal))
+ {
+ return;
+ }
+
+ /* Informational is not a choice - it is the lowest level that delivers event 13, because
+ event 13 is itself Informational. See this type's remarks for what else that admits and
+ why OnEventWritten's allowlist, not this level, is the barrier. */
+ EnableEvents(eventSource, EventLevel.Informational);
+ }
+
+ ///
+ protected override void OnEventWritten(EventWrittenEventArgs eventData)
+ {
+ /* A conjunction, so neither half alone can admit an event. The id is the allowlist; the
+ source name means an id-13 collision from some other source this listener never enabled
+ still cannot reach the log. */
+ if (eventData is null ||
+ eventData.EventId != CredentialSelectedEventId ||
+ !string.Equals(eventData.EventSource?.Name, AzureIdentitySourceName, StringComparison.Ordinal))
+ {
+ return;
+ }
+
+ var payload = eventData.Payload;
+ var credentialType = payload is { Count: > 0 } ? payload[0] as string : null;
+
+ /* Payload[0] only, and only if it looks like a type name. Both halves are load-bearing: the
+ index is what keeps a future second parameter out of the log, and the shape check is what
+ keeps a REORDERED first parameter out of it. A tenant id, a UPN, a scope URL and an
+ exception message each fail the shape check on a character a type name cannot contain. */
+ if (EntraCredentialSelectionLog.IsCredentialTypeName(credentialType))
+ {
+ _selected = credentialType;
+ }
+ else
+ {
+ _rejectedPayload = true;
+ }
+ }
+}
+
+///
+/// Decides whether a captured credential selection is worth a log line, and writes it.
+///
+/// Two lifetimes, both deliberate.
+///
+/// The listener's window is one connection open, not the process. While the source is
+/// enabled, AzureIdentityEventSource.IsEnabled(EventLevel.Informational, …) answers true
+/// inside Azure.Identity and it performs the formatting work its seven
+/// if (IsEnabled(…)) guards otherwise skip — formatting scope arrays, rendering exceptions.
+/// A monitoring tool that left it on would pay that on every token operation for the life of the
+/// process, forever, to learn a fact that can only be reported once. So is
+/// called immediately before the open and the listener is disposed immediately after it; an
+/// undisposed keeps receiving events, so the disposal is the window.
+/// The cost of the narrow window is that an event raised outside it is missed, which is why BOTH of
+/// Lite's connection-open sites carry one rather than only the dialog a user is looking at.
+///
+/// Only the last REPORTED name outlives the attempt. Nothing else is retained: the
+/// listener is gone and the captured value is read once. exists so an
+/// unchanged repeat is a line instead of an
+/// one, because the selection is a process fact and a monitoring
+/// tool restating it once per server per sweep would be noise. A CHANGED name still reports at
+/// — the driver clears its credential cache
+/// (ActiveDirectoryAuthenticationProvider.cs:136-138), so a different source genuinely can
+/// win later, and that is the one thing here worth interrupting someone with.
+///
+/// Why this reports at most once in practice, stated rather than discovered. Two
+/// caches sit above the event and both are process-wide. DefaultAzureCredential raises event
+/// 13 only on the branch that has no cached credential yet
+/// (DefaultAzureCredential.cs:168-179): once _credentialLock holds a value, later
+/// token requests take the HasValue branch and raise nothing. And SqlClient caches the
+/// DefaultAzureCredential INSTANCE in a static map keyed by authority, scope, audience
+/// and client id (Microsoft.Data.SqlClient.Extensions.Azure 7.0.2,
+/// ActiveDirectoryAuthenticationProvider.cs:29 and :258-262). So event 13 fires at most
+/// once per process per key, and a second connection to the same tenant reports nothing at all. That
+/// absence is a line saying so, rather than silence: an empty result
+/// must not be mistakable for "nothing happened".
+///
+/// No server name on the line, and that is not squeamishness. The selection is scoped
+/// to the process and to the driver's cache key, not to a server — the credential chain resolves
+/// from environment variables, an az login session and machine identity, none of which are
+/// per-server. Naming the server that happened to observe it would assert a per-server fact that is
+/// not true, and the next connection to a different server in the same tenant reports nothing while
+/// using the very same credential.
+///
+internal static class EntraCredentialSelectionLog
+{
+ /// The AppLogger source column for these lines.
+ internal const string LogSource = "EntraCredential";
+
+ ///
+ /// The longest value forwarded as a credential type name. Azure.Identity's longest
+ /// credential type name is well under 60 characters; this is a bound on what an unexpected
+ /// payload could put in the log, not a measurement of anything.
+ ///
+ internal const int MaxCredentialTypeNameLength = 256;
+
+ private static volatile string? s_lastReported;
+
+ ///
+ /// A listener for a connection about to be opened with
+ /// — which is what
+ /// maps to,
+ /// and the only mode whose token comes from a DefaultAzureCredential — or null for
+ /// every other mode. A null is a using that disposes nothing and a
+ /// that does nothing, so no other mode enables the source or pays for it.
+ ///
+ /// Keyed off the BUILDER rather than off the app's own mode string on purpose: this is the
+ /// keyword the driver will actually act on, so the gate cannot disagree with the connection
+ /// string, and it follows a credential profile into this mode for free if one is ever offered
+ /// there (today they offer SQL, service principal and managed identity only).
+ ///
+ internal static EntraCredentialSelectionListener? Begin(SqlConnectionStringBuilder? builder) =>
+ builder?.Authentication == SqlAuthenticationMethod.ActiveDirectoryDefault
+ ? new EntraCredentialSelectionListener()
+ : null;
+
+ ///
+ /// Whether a captured payload has the shape of a CLR type name, and may therefore be logged.
+ ///
+ /// The second barrier, and the one that survives a payload REORDER. The allowlist
+ /// in guarantees the event is
+ /// event 13; this guarantees the value taken from it is not something else. Every sensitive
+ /// sibling payload in that source fails on a character a namespace-qualified type name cannot
+ /// contain: a tenant id has -, an account UPN has @, a scope has : and
+ /// /, an exception message and an MSAL log line have spaces. A dot is required, so a bare
+ /// word is not a type name either.
+ ///
+ /// Generic type names (`1[[…]]) are rejected by the same rule. No credential type in
+ /// Azure.Identity is generic, and rejecting one costs a line
+ /// saying the payload was declined — which is the direction to fail in.
+ ///
+ internal static bool IsCredentialTypeName(string? value)
+ {
+ if (string.IsNullOrEmpty(value) || value.Length > MaxCredentialTypeNameLength)
+ {
+ return false;
+ }
+
+ foreach (var c in value)
+ {
+ if (!char.IsAsciiLetterOrDigit(c) && c != '.' && c != '_' && c != '+')
+ {
+ return false;
+ }
+ }
+
+ return value.Contains('.', StringComparison.Ordinal) && value[0] != '.' && value[^1] != '.';
+ }
+
+ ///
+ /// What a captured (or absent, or declined) selection should produce, given what was last
+ /// reported. Pure and static: no clock, no I/O, the caller passes the previous state and
+ /// persists the returned one, which is what makes the whole decision testable without a log
+ /// file. One return value carries the level, the text and the new state together so a caller
+ /// cannot write the line and drop the state.
+ ///
+ ///
+ /// .
+ ///
+ ///
+ /// .
+ ///
+ /// The last name reported at .
+ internal static EntraCredentialSelectionLine Decide(
+ string? credentialTypeName,
+ bool rejectedPayload,
+ string? lastReported)
+ {
+ if (!IsCredentialTypeName(credentialTypeName))
+ {
+ /* Re-checked here rather than trusted from the listener, so this function's answer does
+ not depend on which of two types applied the rule. The declined value is NOT in the
+ message - naming it would be the leak the check exists to prevent. */
+ return rejectedPayload
+ ? new EntraCredentialSelectionLine(
+ LogLevel.Debug,
+ "DefaultAzureCredential reported a credential selection whose payload was not a "
+ + "type name, so it was not logged. Azure.Identity's event 13 payload may have "
+ + "changed shape.",
+ Reported: null)
+ : new EntraCredentialSelectionLine(
+ LogLevel.Debug,
+ "DefaultAzureCredential did not report which credential it selected on this "
+ + "connection. Expected on any connection but the first: Azure.Identity "
+ + "raises that event once per credential instance, and SqlClient caches the "
+ + "instance for the process.",
+ Reported: null);
+ }
+
+ return string.Equals(credentialTypeName, lastReported, StringComparison.Ordinal)
+ ? new EntraCredentialSelectionLine(
+ LogLevel.Debug,
+ $"DefaultAzureCredential selected {credentialTypeName} (unchanged).",
+ Reported: null)
+ : new EntraCredentialSelectionLine(
+ LogLevel.Information,
+ $"DefaultAzureCredential selected {credentialTypeName}.",
+ Reported: credentialTypeName);
+ }
+
+ ///
+ /// Reads the listener and writes the line. A no-op for a null listener, which is every
+ /// mode but this one.
+ ///
+ internal static void Report(EntraCredentialSelectionListener? listener)
+ {
+ if (listener is null)
+ {
+ return;
+ }
+
+ var line = Decide(listener.SelectedCredentialType, listener.RejectedPayload, s_lastReported);
+
+ if (line.Reported is not null)
+ {
+ s_lastReported = line.Reported;
+ }
+
+ if (line.Level == LogLevel.Information)
+ {
+ AppLogger.Info(LogSource, line.Message);
+ }
+ else
+ {
+ AppLogger.Debug(LogSource, line.Message);
+ }
+ }
+
+ ///
+ /// Forgets the last reported name, so a test can exercise the first-observation arm. Never
+ /// called by the app.
+ ///
+ internal static void ResetForTests() => s_lastReported = null;
+}
+
+///
+/// One log line: the level it is written at, its text, and the name to remember as reported (null
+/// when nothing should be remembered). A record struct so the three cannot be returned separately
+/// and recombined wrongly.
+///
+internal readonly record struct EntraCredentialSelectionLine(
+ LogLevel Level,
+ string Message,
+ string? Reported);
diff --git a/Lite/Services/ServerManager.cs b/Lite/Services/ServerManager.cs
index 42bf9353d..e15695a86 100644
--- a/Lite/Services/ServerManager.cs
+++ b/Lite/Services/ServerManager.cs
@@ -443,7 +443,18 @@ public async Task CheckConnectionAsync(string serverId,
};
using var connection = new SqlConnection(builder.ConnectionString);
- await connection.OpenAsync();
+
+ /* Observability only: the listener reads one Azure-Identity event and writes the chosen
+ credential's type name. It is non-null for EntraDefaultCredential alone, so every other
+ mode disposes nothing and logs nothing, and the window is exactly this open rather than
+ the life of the process - see EntraCredentialSelectionLog for both lifetime decisions.
+ This is also the site that usually gets there first: the sweep runs on a timer, and the
+ driver's static credential cache means the event fires at most once per process. */
+ using (var credentialSelection = EntraCredentialSelectionLog.Begin(builder))
+ {
+ await connection.OpenAsync();
+ EntraCredentialSelectionLog.Report(credentialSelection);
+ }
// Connection succeeded — server is reachable regardless of DMV permissions below.
status.IsOnline = true;
diff --git a/Lite/Windows/AddServerDialog.xaml.cs b/Lite/Windows/AddServerDialog.xaml.cs
index e92137a2d..3543ba23e 100644
--- a/Lite/Windows/AddServerDialog.xaml.cs
+++ b/Lite/Windows/AddServerDialog.xaml.cs
@@ -373,8 +373,20 @@ private SqlConnectionStringBuilder BuildConnectionBuilder()
try
{
- using var connection = new SqlConnection(BuildConnectionBuilder().ConnectionString);
- await connection.OpenAsync();
+ var builder = BuildConnectionBuilder();
+ using var connection = new SqlConnection(builder.ConnectionString);
+
+ /* Observability only, window exactly this open, non-null for EntraDefaultCredential alone
+ - see EntraCredentialSelectionLog. Instrumented here AS WELL AS in ServerManager, not
+ instead of it: the event fires at most once per process, so whichever site opens the
+ first EntraDefaultCredential connection is the only one that can observe it, and which
+ one that is depends on whether a server was already saved when the sweep ran. */
+ using (var credentialSelection = EntraCredentialSelectionLog.Begin(builder))
+ {
+ await connection.OpenAsync();
+ EntraCredentialSelectionLog.Report(credentialSelection);
+ }
+
using var cmd = new SqlCommand("SELECT @@VERSION", connection);
var version = await cmd.ExecuteScalarAsync() as string;
serverVersion = version?.Split('\n')[0]?.Trim();
From a1b16a3e527079e5556e1a1cf76ae78e1db3ec21 Mon Sep 17 00:00:00 2001
From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com>
Date: Wed, 9 Sep 2026 12:16:42 -0400
Subject: [PATCH 02/14] Isolate the character rule and pin Decide's own re-read
of it
Mutation found two pins green that should have been red. Every rejection case
for the type-name shape check was also missing a dot, so the dot requirement
alone rejected it and the character allowlist was never under test - widening
it by a hyphen left the suite green. And Decide re-reads the rule rather than
trusting the listener's verdict, which nothing exercised: keying the refusal
off the bool alone was also green.
---
Lite.Tests/EntraCredentialSelectionTests.cs | 47 +++++++++++++++++++++
1 file changed, 47 insertions(+)
diff --git a/Lite.Tests/EntraCredentialSelectionTests.cs b/Lite.Tests/EntraCredentialSelectionTests.cs
index 09b4cfa9c..f59198921 100644
--- a/Lite.Tests/EntraCredentialSelectionTests.cs
+++ b/Lite.Tests/EntraCredentialSelectionTests.cs
@@ -321,6 +321,27 @@ public void IsCredentialTypeName_TakesATypeName(string value) =>
public void IsCredentialTypeName_RejectsEverySensitiveSiblingShape(string? value) =>
Assert.False(EntraCredentialSelectionLog.IsCredentialTypeName(value));
+ ///
+ /// The character rule, isolated. Every case above that a real sibling payload looks like is
+ /// ALSO missing a dot, so the dot requirement alone rejects it and the character allowlist is
+ /// never the thing under test — measured: widening the allowlist by a hyphen left the whole suite
+ /// green. Each row here is a well-formed dotted name differing from an acceptable one by exactly
+ /// one illegal character, so each character in that allowlist is load-bearing.
+ ///
+ [Theory]
+ [InlineData("Azure.Identity.Cli-Credential")] // hyphen: tenant/subscription ids
+ [InlineData("Azure.Identity Credential")] // space: any message or log line
+ [InlineData("Azure.Identity/Credential")] // slash: a scope or resource path
+ [InlineData("Azure.Identity:Credential")] // colon: a scheme or a scope
+ [InlineData("Azure.Identity@Credential")] // at: an account upn
+ [InlineData("Azure.Identity\\Credential")] // backslash: a down-level logon name
+ [InlineData("Azure.Identity,Credential")] // comma: an assembly-qualified name
+ [InlineData("Azure.Identity=Credential")] // equals: a connection-string fragment
+ [InlineData("Azure.Identity\tCredential")] // tab: a delimiter in a rendered payload
+ [InlineData("Azure.Identity\nCredential")] // newline: would split the log line in two
+ public void IsCredentialTypeName_RejectsADottedNameCarryingOneIllegalCharacter(string value) =>
+ Assert.False(EntraCredentialSelectionLog.IsCredentialTypeName(value));
+
[Fact]
public void IsCredentialTypeName_RejectsSomethingLongerThanAnyTypeName() =>
Assert.False(EntraCredentialSelectionLog.IsCredentialTypeName(
@@ -348,6 +369,12 @@ public void AReorderedPayload_IsDeclinedAndNeverReachesTheLine()
Assert.Null(listener.SelectedCredentialType);
Assert.True(listener.RejectedPayload);
+ /* Note what this can and cannot show. The refused value never LEAVES OnEventWritten - the
+ listener stores a bool, not the string - so Decide structurally cannot name it, and the
+ DoesNotContain below is a consequence of that rather than an independent check of it.
+ Measured: interpolating the value into that message changed nothing, because there is no
+ value here to interpolate. The independent checks are the two assertions above, and
+ Decide_RefusesAValueThatIsNotATypeName_EvenWhenTheListenerDidNot below. */
var line = EntraCredentialSelectionLog.Decide(
listener.SelectedCredentialType, listener.RejectedPayload, lastReported: null);
@@ -412,6 +439,26 @@ public void Report_DoesNothingForANullListener() =>
// ---- What gets written, and at which level -------------------------------------------
+ ///
+ /// re-reads the rule rather than
+ /// trusting the listener's verdict, and this is the pin that makes that re-read load-bearing.
+ /// Handed a value that is not a type name with rejectedPayload: false — the shape a
+ /// listener that stopped applying the rule would produce — it must still refuse to log it.
+ /// Without this, keying the refusal off the bool alone leaves every other pin here green.
+ ///
+ [Theory]
+ [InlineData("00000000-0000-0000-0000-000000000000")]
+ [InlineData("someone@example.com")]
+ [InlineData("https://database.windows.net/.default")]
+ public void Decide_RefusesAValueThatIsNotATypeName_EvenWhenTheListenerDidNot(string value)
+ {
+ var line = EntraCredentialSelectionLog.Decide(value, rejectedPayload: false, lastReported: null);
+
+ Assert.Equal(LogLevel.Debug, line.Level);
+ Assert.DoesNotContain(value, line.Message, StringComparison.Ordinal);
+ Assert.Null(line.Reported);
+ }
+
[Fact]
public void Decide_ReportsAFirstObservationAtInformation()
{
From e93ad47665011a4119f036f4df286a294fce1546 Mon Sep 17 00:00:00 2001
From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com>
Date: Wed, 9 Sep 2026 12:18:58 -0400
Subject: [PATCH 03/14] Say in the README that the log names the credential
that won
The mode's own section already warns that the credential order is the driver's
and that a machine with several Azure identities connects as whichever comes
first. That caveat is now answerable after the fact, so the section says so -
including that there is exactly one such line per run, and that the type name
is the only thing recorded.
---
README.md | 1 +
1 file changed, 1 insertion(+)
diff --git a/README.md b/README.md
index 00795893d..698ca1383 100644
--- a/README.md
+++ b/README.md
@@ -450,6 +450,7 @@ It signs in as **whichever Azure identity is already established on the machine*
- **With no Azure sign-in on the machine it fails rather than asking for one.** Run `az login` first. The connection dialog says which of the two failure modes happened — nothing found, or one found and broken — and the log names each source and why it declined.
- **The order is the driver's, not yours.** There is no seam to narrow the list: the driver constructs the credential chain itself and this app cannot pass options into it. On a machine with several Azure identities set up, this connects as whichever comes first, which is not necessarily the one you signed into Windows with. When a specific identity matters, use **Service Principal**, which names it.
+- **The log says which one won.** Lite records the credential type `DefaultAzureCredential` actually selected — `AzureCliCredential`, `EnvironmentCredential`, `ManagedIdentityCredential` and so on — so "whichever comes first" is answerable after the fact rather than only a caveat. Expect exactly one such line per run of the app: `Azure.Identity` reports the selection once per credential instance and the driver caches that instance for the process, so later connections have nothing new to report and say so at `Debug`. The type name is the only thing recorded; no tenant id, account or scope.
Darling does not offer this mode: the Darling service's connect path builds Windows-integrated or SQL-login connections only and acquires no tokens at all, so its viewer rejects every Azure mode at the credential step.
From 0fa42af403a861700a1aa18ae1b700cb6fad7f5b Mon Sep 17 00:00:00 2001
From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com>
Date: Wed, 9 Sep 2026 12:22:49 -0400
Subject: [PATCH 04/14] Pin payload slot 0 by NAME, on a real written event
The listener reads Payload[0] positionally. Azure.Identity arrives
transitively and unpinned, so a parameter added or reordered ahead of
credentialType needs no code change here to happen - the type-name shape check
declines such a value at runtime rather than logging it, and this makes the
move loud rather than merely survivable. Measured: the runtime does report
PayloadNames for this event, so the name is readable where it matters.
---
Lite.Tests/EntraCredentialSelectionTests.cs | 81 +++++++++++++++++++++
1 file changed, 81 insertions(+)
diff --git a/Lite.Tests/EntraCredentialSelectionTests.cs b/Lite.Tests/EntraCredentialSelectionTests.cs
index f59198921..28ebff860 100644
--- a/Lite.Tests/EntraCredentialSelectionTests.cs
+++ b/Lite.Tests/EntraCredentialSelectionTests.cs
@@ -117,6 +117,46 @@ public void TheUpstreamEventContract_IsWhatTheListenerAssumes()
Assert.Equal(EventLevel.Informational, attribute.Level);
}
+ ///
+ /// Payload slot 0 is the credential type, asserted by NAME on a real written event.
+ /// The listener reads Payload[0] positionally, so a future upstream parameter added or
+ /// reordered ahead of credentialType would silently change what that index means — and
+ /// Azure.Identity arrives transitively and unpinned (#3219), so such a move needs no code
+ /// change here to happen. The runtime barrier against it is the type-name shape check, which
+ /// declines the value rather than logging it; this is the pin that makes the move LOUD instead of
+ /// merely survivable.
+ ///
+ /// Read off rather than the reflected
+ /// method signature, because the name the runtime reports is what actually accompanies the
+ /// payload — and the two can disagree. The count assertion is the control: an empty
+ /// PayloadNames (which some event formats produce) would make an index check vacuous.
+ ///
+ [Fact]
+ public void PayloadSlotZero_IsTheCredentialType_OnARealWrittenEvent()
+ {
+ Singleton();
+ using var recorder = new PayloadNameListener();
+
+ Raise(SelectedMethod, CliCredential);
+
+ var captured = recorder.Captured
+ .Where(c => c.EventId == EntraCredentialSelectionListener.CredentialSelectedEventId)
+ .ToList();
+
+ Assert.Single(captured);
+
+ var (_, names, payload) = captured[0];
+
+ Assert.True(
+ names.Count > 0,
+ "the runtime reported no payload names for event 13, so an index assertion on them would "
+ + "be vacuous — this event format does not carry them and this pin needs rewriting");
+
+ Assert.Equal("credentialType", names[0]);
+ Assert.Single(payload);
+ Assert.Equal(CliCredential, payload[0]);
+ }
+
///
/// Why the filter is an allowlist and not a level or a keyword. Not one event in
/// that source declares Keywords, so EnableEvents has no dimension to exclude the
@@ -720,6 +760,47 @@ private sealed class BystanderEventSource : EventSource
{
}
+ ///
+ /// Captures the event id together with the runtime's own payload NAMES and values, so the
+ /// positional read the production listener performs can be checked against the name the runtime
+ /// attaches to that position.
+ ///
+ private sealed class PayloadNameListener : EventListener
+ {
+ private readonly List<(int EventId, IReadOnlyList Names, IReadOnlyList Payload)> _captured = new();
+
+ internal IReadOnlyList<(int EventId, IReadOnlyList Names, IReadOnlyList Payload)> Captured
+ {
+ get { lock (_captured) { return _captured.ToList(); } }
+ }
+
+ protected override void OnEventSourceCreated(EventSource eventSource)
+ {
+ if (string.Equals(
+ eventSource?.Name,
+ EntraCredentialSelectionListener.AzureIdentitySourceName,
+ StringComparison.Ordinal))
+ {
+ EnableEvents(eventSource!, EventLevel.Informational);
+ }
+ }
+
+ protected override void OnEventWritten(EventWrittenEventArgs eventData)
+ {
+ var names = eventData.PayloadNames is null
+ ? Array.Empty()
+ : eventData.PayloadNames.ToArray();
+ var payload = eventData.Payload is null
+ ? Array.Empty()
+ : eventData.Payload.Select(o => o?.ToString() ?? string.Empty).ToArray();
+
+ lock (_captured)
+ {
+ _captured.Add((eventData.EventId, names, payload));
+ }
+ }
+ }
+
private sealed class RecordingListener : EventListener
{
private readonly ConcurrentBag _seen = new();
From f86bbfd6dea973f4da76dc7f1c3dd2285fd5d37d Mon Sep 17 00:00:00 2001
From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com>
Date: Wed, 9 Sep 2026 12:23:22 -0400
Subject: [PATCH 05/14] Say what the payload-name count assertion actually does
It is not a control. An empty PayloadNames makes names[0] throw rather than
pass, so the pin cannot go vacuous that way - measured by discarding the names
and deleting the assertion, which still failed. The assertion earns its place
by naming the cause instead of leaving an index-out-of-range to diagnose.
---
Lite.Tests/EntraCredentialSelectionTests.cs | 7 +++++--
1 file changed, 5 insertions(+), 2 deletions(-)
diff --git a/Lite.Tests/EntraCredentialSelectionTests.cs b/Lite.Tests/EntraCredentialSelectionTests.cs
index 28ebff860..b8fbf5ba3 100644
--- a/Lite.Tests/EntraCredentialSelectionTests.cs
+++ b/Lite.Tests/EntraCredentialSelectionTests.cs
@@ -128,8 +128,11 @@ public void TheUpstreamEventContract_IsWhatTheListenerAssumes()
///
/// Read off rather than the reflected
/// method signature, because the name the runtime reports is what actually accompanies the
- /// payload — and the two can disagree. The count assertion is the control: an empty
- /// PayloadNames (which some event formats produce) would make an index check vacuous.
+ /// payload — and the two can disagree. Some event formats carry no payload names at all, so the
+ /// count is asserted first for the MESSAGE rather than as a control: an empty list makes
+ /// names[0] throw, which fails either way — measured, by discarding the names and
+ /// deleting the count assertion — so this says "this pin needs rewriting" instead of leaving
+ /// someone an index-out-of-range to diagnose.
///
[Fact]
public void PayloadSlotZero_IsTheCredentialType_OnARealWrittenEvent()
From 19a336c992db66e985231618cec5b86e9bf02de1 Mon Sep 17 00:00:00 2001
From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com>
Date: Wed, 9 Sep 2026 12:25:19 -0400
Subject: [PATCH 06/14] Correct the IsEnabled guard count and put every count
beside its version
The cost note said seven IsEnabled guards; at 1.18.0 there are 21, plus 10
when-IsEnabled switch arms. And a count in a comment is a partial list with a
numeral welded on, so each one now names the version it was measured at, and
the event histogram says it was counted twice off things that fail differently
- the [Event] attributes at the tag, and reflection over the shipped assembly.
---
Lite/Helpers/EntraCredentialSelection.cs | 16 ++++++++++------
1 file changed, 10 insertions(+), 6 deletions(-)
diff --git a/Lite/Helpers/EntraCredentialSelection.cs b/Lite/Helpers/EntraCredentialSelection.cs
index 0f09ae639..56ba9e0e1 100644
--- a/Lite/Helpers/EntraCredentialSelection.cs
+++ b/Lite/Helpers/EntraCredentialSelection.cs
@@ -34,10 +34,12 @@ namespace PerformanceMonitorLite.Helpers;
/// An ALLOWLIST on the event id, because the level cannot narrow this and keywords do not
/// exist here. Event 13 is itself Informational, so Informational is the LOWEST
/// level that delivers it — and
-/// admits every event at or above the requested severity, which in this source is 28 of its 29
-/// events: 20 at Informational (19 besides this one), plus 4 Warning, 2
-/// Error, 1 Critical and 1 LogAlways. Only the single Verbose event is
-/// excluded. Several of the 28 carry exactly what an application log must never hold:
+/// admits every event at or above the requested severity — which at 1.18.0 is 28 of this
+/// source's 29 events: 20 at Informational (19 besides this one), plus 4
+/// Warning, 2 Error, 1 Critical and 1 LogAlways. Only the single
+/// Verbose event is excluded. Counted twice, off two things that fail differently: the
+/// [Event] attributes in the source at tag Azure.Identity_1.18.0, and reflection over
+/// the shipped Azure.Identity.dll. Several of the 28 carry exactly what an application log must never hold:
/// TenantIdDiscoveredAndUsed / TenantIdDiscoveredAndNotUsed carry tenant ids,
/// AuthenticatedAccountDetails carries account details, GetTokenFailed carries a
/// formatted Exception, the six MsalLog* events carry MSAL's own log lines, and
@@ -153,8 +155,10 @@ exception message each fail the shape check on a character a type name cannot co
///
/// The listener's window is one connection open, not the process. While the source is
/// enabled, AzureIdentityEventSource.IsEnabled(EventLevel.Informational, …) answers true
-/// inside Azure.Identity and it performs the formatting work its seven
-/// if (IsEnabled(…)) guards otherwise skip — formatting scope arrays, rendering exceptions.
+/// inside Azure.Identity and it performs the formatting work that the
+/// IsEnabled(…) guard on almost every one of its event methods otherwise skips — formatting
+/// scope arrays, rendering exceptions. At 1.18.0 that is 21 such guards plus 10
+/// when IsEnabled(…) switch arms.
/// A monitoring tool that left it on would pay that on every token operation for the life of the
/// process, forever, to learn a fact that can only be reported once. So is
/// called immediately before the open and the listener is disposed immediately after it; an
From 2eaec8c8a3e2143d65e22e6ccefd71bfb30619a9 Mon Sep 17 00:00:00 2001
From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com>
Date: Wed, 9 Sep 2026 12:29:07 -0400
Subject: [PATCH 07/14] Report the selection on a failed open too, and claim
the dedupe atomically
Azure.Identity raises event 13 when it ACQUIRES a token, before SQL Server has
accepted or rejected the identity that token names. So the case this feature
exists for - DefaultAzureCredential picked the wrong ambient identity - arrives
as a login failure with the selection already captured, and reporting only
after a successful open dropped it. It is also the only chance to see it: the
driver caches the credential the moment a token exists, so a retry raises
nothing. Both call sites now report from a finally, and the call-site pin
requires that finally between the attach and the report - Begin-before-open
with Report-after-open was satisfied by the success tail.
And the dedupe is claimed with Interlocked.Exchange rather than read then
written. One raised event reaches every attached listener, and connection
checks run concurrently, so two concurrent first connections capture the same
name and a read-then-write pair lets both decide they are first.
---
Lite.Tests/EntraCredentialSelectionTests.cs | 52 +++++++++++++++++++--
Lite/Helpers/EntraCredentialSelection.cs | 29 ++++++++++--
Lite/Services/ServerManager.cs | 19 ++++++--
Lite/Windows/AddServerDialog.xaml.cs | 18 +++++--
4 files changed, 105 insertions(+), 13 deletions(-)
diff --git a/Lite.Tests/EntraCredentialSelectionTests.cs b/Lite.Tests/EntraCredentialSelectionTests.cs
index b8fbf5ba3..fc6ff0178 100644
--- a/Lite.Tests/EntraCredentialSelectionTests.cs
+++ b/Lite.Tests/EntraCredentialSelectionTests.cs
@@ -480,6 +480,25 @@ public void Begin_ReturnsNull_ForABuilderWithNoAuthenticationKeywordAndForNull()
public void Report_DoesNothingForANullListener() =>
EntraCredentialSelectionLog.Report(null);
+ ///
+ /// A captured selection is reported whether or not the connection then succeeded.
+ /// takes nothing but the listener — it has no
+ /// success argument to get wrong — and this pins that, because the call sites now invoke it from
+ /// a finally and the reason they do is that the failing case is the one that matters. The
+ /// call sites' own half is ;
+ /// this is the half that shows there is nothing here for a failure to suppress.
+ ///
+ [Fact]
+ public void Report_TakesNoSuccessArgument_SoAFailedOpenCannotSuppressIt()
+ {
+ var parameters = typeof(EntraCredentialSelectionLog)
+ .GetMethod(nameof(EntraCredentialSelectionLog.Report), BindingFlags.NonPublic | BindingFlags.Static)!
+ .GetParameters();
+
+ Assert.Single(parameters);
+ Assert.Equal(typeof(EntraCredentialSelectionListener), parameters[0].ParameterType);
+ }
+
// ---- What gets written, and at which level -------------------------------------------
///
@@ -644,14 +663,25 @@ after the first actually looks like. */
/// file would be satisfied by the comment alone and would pass with every call deleted. #3201's
/// equivalent pin did exactly that until the strip was added.
///
- /// The ORDER is the discriminating assertion. Both names being present is satisfied
- /// by a Report placed before the open, which would capture nothing on every connection
- /// forever while reading as fully instrumented.
+ /// Three discriminating assertions, and the third is the one review found missing.
+ /// Both names being present is satisfied by a Report placed before the open, which would
+ /// capture nothing on every connection forever while reading as fully instrumented — so the
+ /// order is asserted. And Begin before the open with Report after it is satisfied
+ /// by a Report on the SUCCESS TAIL, which drops the selection on every failed open. That
+ /// is not a corner: Azure.Identity raises the event when it acquires a token, before SQL
+ /// Server has accepted or rejected the identity that token names, so the wrong-ambient-identity
+ /// case this feature exists for arrives as a login failure with the selection already captured —
+ /// and the driver caches the credential as soon as the token exists, so there is no second
+ /// chance. Hence the third assertion: a finally between the attach and the report.
+ ///
+ /// Asserted on the WINDOW between Begin and Report rather than on the body,
+ /// so an unrelated finally elsewhere in the method cannot satisfy it —
+ /// RunConnectionTestAsync already had one, for re-enabling its buttons.
///
[Theory]
[InlineData("Lite/Services/ServerManager.cs", "CheckConnectionAsync(string serverId")]
[InlineData("Lite/Windows/AddServerDialog.xaml.cs", "RunConnectionTestAsync()")]
- public void EveryConnectionOpenSite_BeginsBeforeTheOpenAndReportsAfterIt(
+ public void EveryConnectionOpenSite_AttachesBeforeTheOpenAndReportsOnBothPaths(
string relativePath, string methodAnchor)
{
var source = CSharpSourceWalker.StripCommentsAndStrings(ReadRepoFile(relativePath));
@@ -672,6 +702,15 @@ public void EveryConnectionOpenSite_BeginsBeforeTheOpenAndReportsAfterIt(
Assert.True(begin >= 0, $"{relativePath} must attach the credential-selection listener in {methodAnchor}");
Assert.True(report >= 0, $"{relativePath} must report the credential selection in {methodAnchor}");
+ /* One of each, so a duplicate call cannot make the window assertions read off the wrong
+ pair of indices. */
+ Assert.Equal(
+ begin,
+ body.LastIndexOf("EntraCredentialSelectionLog.Begin", StringComparison.Ordinal));
+ Assert.Equal(
+ report,
+ body.LastIndexOf("EntraCredentialSelectionLog.Report", StringComparison.Ordinal));
+
Assert.True(
begin < open,
"the listener must be attached BEFORE the open, or Azure.Identity raises event 13 with "
@@ -680,6 +719,11 @@ public void EveryConnectionOpenSite_BeginsBeforeTheOpenAndReportsAfterIt(
open < report,
"the selection must be reported AFTER the open, or it is read before the event that "
+ "produces it and every connection reports nothing");
+
+ var window = body[begin..report];
+
+ Assert.Contains("finally", window, StringComparison.Ordinal);
+ Assert.Contains("try", window, StringComparison.Ordinal);
}
// ---- Helpers -------------------------------------------------------------------------
diff --git a/Lite/Helpers/EntraCredentialSelection.cs b/Lite/Helpers/EntraCredentialSelection.cs
index 56ba9e0e1..ae5b954d7 100644
--- a/Lite/Helpers/EntraCredentialSelection.cs
+++ b/Lite/Helpers/EntraCredentialSelection.cs
@@ -8,6 +8,7 @@
using System;
using System.Diagnostics.Tracing;
+using System.Threading;
using Microsoft.Data.SqlClient;
using Microsoft.Extensions.Logging;
using PerformanceMonitorLite.Services;
@@ -206,7 +207,9 @@ internal static class EntraCredentialSelectionLog
///
internal const int MaxCredentialTypeNameLength = 256;
- private static volatile string? s_lastReported;
+ /* Not volatile: every read goes through Volatile.Read and every write through
+ Interlocked.Exchange, and a volatile field cannot be passed by ref to either. */
+ private static string? s_lastReported;
///
/// A listener for a connection about to be opened with
@@ -313,6 +316,12 @@ not depend on which of two types applied the rule. The declined value is NOT in
///
/// Reads the listener and writes the line. A no-op for a null listener, which is every
/// mode but this one.
+ ///
+ /// Called from a finally at both call sites, so it must not throw. Nothing
+ /// here does I/O: is pure string work and enqueues
+ /// into a buffer. There is deliberately no blanket catch — a finally that swallows
+ /// everything would make this feature silently dead at exactly the moment it broke, and would
+ /// hide the defect rather than the symptom.
///
internal static void Report(EntraCredentialSelectionListener? listener)
{
@@ -321,11 +330,25 @@ internal static void Report(EntraCredentialSelectionListener? listener)
return;
}
- var line = Decide(listener.SelectedCredentialType, listener.RejectedPayload, s_lastReported);
+ var captured = listener.SelectedCredentialType;
+ var line = Decide(captured, listener.RejectedPayload, Volatile.Read(ref s_lastReported));
if (line.Reported is not null)
{
- s_lastReported = line.Reported;
+ /* Claimed atomically, because a read-then-write pair does not dedupe here.
+ CheckAllConnectionsAsync checks servers concurrently, and one raised event is delivered
+ to EVERY attached listener - so two concurrent first connections capture the SAME name
+ and, reading before either writes, both decide they are the first and both log at
+ Information. Interlocked.Exchange hands exactly one caller a previous value different
+ from what it is claiming; anyone else gets its own name back and re-decides against it.
+ The correctness of that rests on the exchange being atomic, not on a test: the race is
+ not reproducible on demand, and the sequential form of the same path is pinned. */
+ var previous = Interlocked.Exchange(ref s_lastReported, line.Reported);
+
+ if (string.Equals(previous, line.Reported, StringComparison.Ordinal))
+ {
+ line = Decide(captured, listener.RejectedPayload, previous);
+ }
}
if (line.Level == LogLevel.Information)
diff --git a/Lite/Services/ServerManager.cs b/Lite/Services/ServerManager.cs
index e15695a86..434a15e13 100644
--- a/Lite/Services/ServerManager.cs
+++ b/Lite/Services/ServerManager.cs
@@ -449,11 +449,24 @@ public async Task CheckConnectionAsync(string serverId,
mode disposes nothing and logs nothing, and the window is exactly this open rather than
the life of the process - see EntraCredentialSelectionLog for both lifetime decisions.
This is also the site that usually gets there first: the sweep runs on a timer, and the
- driver's static credential cache means the event fires at most once per process. */
+ driver's static credential cache means the event fires at most once per process.
+
+ Reported in a FINALLY, so a failed open reports too. Azure.Identity raises the event
+ when it ACQUIRES a token, before SQL Server has accepted or rejected the identity that
+ token names - so "DefaultAzureCredential picked the wrong ambient identity" arrives as
+ a login failure with the selection already captured, and that is the case this whole
+ feature exists for. It is also the only chance to see it: the driver caches the
+ credential the moment a token is acquired, so a retry raises nothing. */
using (var credentialSelection = EntraCredentialSelectionLog.Begin(builder))
{
- await connection.OpenAsync();
- EntraCredentialSelectionLog.Report(credentialSelection);
+ try
+ {
+ await connection.OpenAsync();
+ }
+ finally
+ {
+ EntraCredentialSelectionLog.Report(credentialSelection);
+ }
}
// Connection succeeded — server is reachable regardless of DMV permissions below.
diff --git a/Lite/Windows/AddServerDialog.xaml.cs b/Lite/Windows/AddServerDialog.xaml.cs
index 3543ba23e..aaa381fda 100644
--- a/Lite/Windows/AddServerDialog.xaml.cs
+++ b/Lite/Windows/AddServerDialog.xaml.cs
@@ -380,11 +380,23 @@ private SqlConnectionStringBuilder BuildConnectionBuilder()
- see EntraCredentialSelectionLog. Instrumented here AS WELL AS in ServerManager, not
instead of it: the event fires at most once per process, so whichever site opens the
first EntraDefaultCredential connection is the only one that can observe it, and which
- one that is depends on whether a server was already saved when the sweep ran. */
+ one that is depends on whether a server was already saved when the sweep ran.
+
+ Reported in a FINALLY for the same reason as the sibling site: the token is acquired,
+ and the event raised, BEFORE SQL Server accepts or rejects the identity it names. A
+ user pressing Test because they suspect the wrong Azure identity is being used gets a
+ failed open, and the selection has to survive it - which is also the only chance,
+ since the driver caches the credential as soon as a token exists. */
using (var credentialSelection = EntraCredentialSelectionLog.Begin(builder))
{
- await connection.OpenAsync();
- EntraCredentialSelectionLog.Report(credentialSelection);
+ try
+ {
+ await connection.OpenAsync();
+ }
+ finally
+ {
+ EntraCredentialSelectionLog.Report(credentialSelection);
+ }
}
using var cmd = new SqlCommand("SELECT @@VERSION", connection);
From 390940efcffdb395c808d93aa95b2dcc69950c69 Mon Sep 17 00:00:00 2001
From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com>
Date: Wed, 9 Sep 2026 12:33:13 -0400
Subject: [PATCH 08/14] Pin the ordering the narrow listener window rests on
Lite has around twenty SqlConnection sites reaching a monitored server and only
two are instrumented, which is sound only because the connectivity check gets
there first: CollectionBackgroundService.ExecuteAsync runs
CheckAllConnectionsAsync before RunDueCollectorsAsync on every cycle including
the first, and that loop is the only thing driving collection.
A reorder there breaks observability with nothing to notice - the app connects
normally, the listener attaches normally, and the event has already been raised
and discarded by a collector, permanently, because it fires at most once per
process. The remaining sites and what they cost are stated in the helper's
remarks rather than implied.
---
.../EntraCredentialSelectionOrderingTests.cs | 100 ++++++++++++++++++
Lite/Helpers/EntraCredentialSelection.cs | 21 +++-
2 files changed, 119 insertions(+), 2 deletions(-)
create mode 100644 Lite.Tests/EntraCredentialSelectionOrderingTests.cs
diff --git a/Lite.Tests/EntraCredentialSelectionOrderingTests.cs b/Lite.Tests/EntraCredentialSelectionOrderingTests.cs
new file mode 100644
index 000000000..177399a71
--- /dev/null
+++ b/Lite.Tests/EntraCredentialSelectionOrderingTests.cs
@@ -0,0 +1,100 @@
+/*
+ * 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.IO;
+using System.Linq;
+using System.Runtime.CompilerServices;
+using Darling.Tests;
+using Xunit;
+
+namespace PerformanceMonitorLite.Tests;
+
+///
+/// The ordering the credential-selection listener's NARROW WINDOW depends on, pinned because it
+/// lives in a file this feature does not otherwise touch.
+///
+/// Azure.Identity reports the selected credential once per credential instance and
+/// SqlClient caches that instance in a process-wide static, so the event fires at most once per
+/// process — whichever code opens the first ActiveDirectoryDefault connection is the only
+/// one that can ever observe it. The listener is attached at
+/// ServerManager.CheckConnectionAsync and at the connection dialog's Test button, and NOT at
+/// the other SqlConnection sites in Lite, which is only sound because
+/// CollectionBackgroundService checks connections before it collects.
+///
+/// A reorder there breaks observability silently — the app would connect identically,
+/// the listener would attach identically, and the event would already have been raised and
+/// discarded by the collector. There is no failure to notice, which is the definition of something
+/// that needs a pin rather than a comment.
+///
+public class EntraCredentialSelectionOrderingTests
+{
+ [Fact]
+ public void TheCollectionLoop_ChecksConnections_BeforeItCollects()
+ {
+ /* Stripped, so the several comments in that method discussing the connection check and the
+ collectors cannot satisfy the ordering below on their own. */
+ var source = CSharpSourceWalker.StripCommentsAndStrings(
+ ReadRepoFile("Lite/Services/CollectionBackgroundService.cs"));
+
+ var signature = source.IndexOf("ExecuteAsync(CancellationToken stoppingToken)", StringComparison.Ordinal);
+ Assert.True(signature >= 0, "CollectionBackgroundService.ExecuteAsync is the collection loop and must exist");
+
+ var brace = source.IndexOf('{', signature);
+ Assert.True(brace > signature, "ExecuteAsync must have a body");
+
+ var body = CSharpSourceWalker.BraceBalanced(source, brace);
+
+ var check = body.IndexOf("CheckAllConnectionsAsync", StringComparison.Ordinal);
+ var collect = body.IndexOf("RunDueCollectorsAsync", StringComparison.Ordinal);
+
+ Assert.True(check >= 0, "ExecuteAsync must run the connection check; the credential-selection listener is attached inside it");
+ Assert.True(collect >= 0, "ExecuteAsync must run the due collectors, or this ordering assertion has nothing to order");
+
+ Assert.True(
+ check < collect,
+ "CollectionBackgroundService must check connections BEFORE running collectors. The "
+ + "credential-selection listener (EntraCredentialSelectionLog) is attached only at the "
+ + "connectivity check and the connection dialog, on the grounds that the check gets "
+ + "there first — and Azure.Identity raises the selected-credential event at most once "
+ + "per process, so if a collector opens the first ActiveDirectoryDefault connection "
+ + "the selection is never observable again. Nothing about that failure is visible at "
+ + "runtime: the app connects normally and the log is simply missing a line.");
+ }
+
+ ///
+ /// And that the connectivity check is still the thing the listener is attached inside — so the
+ /// ordering above is about the right method. A rename of the attach site with this pin left
+ /// alone would otherwise order two things that no longer matter to each other.
+ ///
+ [Fact]
+ public void TheConnectivityCheck_IsWhereTheListenerIsAttached()
+ {
+ var manager = CSharpSourceWalker.StripCommentsAndStrings(
+ ReadRepoFile("Lite/Services/ServerManager.cs"));
+
+ var sweep = manager.IndexOf("CheckAllConnectionsAsync", StringComparison.Ordinal);
+ Assert.True(sweep >= 0, "ServerManager must still expose the sweep the collection loop calls");
+
+ Assert.Contains("CheckConnectionAsync", manager, StringComparison.Ordinal);
+ Assert.Contains("EntraCredentialSelectionLog.Begin", manager, StringComparison.Ordinal);
+ }
+
+ private static string ReadRepoFile(string relativePath, [CallerFilePath] string thisFile = "")
+ {
+ var dir = Path.GetDirectoryName(thisFile)!;
+ var parts = relativePath.Split('/');
+ while (dir is not null && !File.Exists(Path.Combine(new[] { dir }.Concat(parts).ToArray())))
+ {
+ dir = Path.GetDirectoryName(dir);
+ }
+
+ Assert.NotNull(dir);
+ return File.ReadAllText(Path.Combine(new[] { dir! }.Concat(parts).ToArray()));
+ }
+}
diff --git a/Lite/Helpers/EntraCredentialSelection.cs b/Lite/Helpers/EntraCredentialSelection.cs
index ae5b954d7..f246e4f58 100644
--- a/Lite/Helpers/EntraCredentialSelection.cs
+++ b/Lite/Helpers/EntraCredentialSelection.cs
@@ -164,8 +164,25 @@ exception message each fail the shape check on a character a type name cannot co
/// process, forever, to learn a fact that can only be reported once. So is
/// called immediately before the open and the listener is disposed immediately after it; an
/// undisposed keeps receiving events, so the disposal is the window.
-/// The cost of the narrow window is that an event raised outside it is missed, which is why BOTH of
-/// Lite's connection-open sites carry one rather than only the dialog a user is looking at.
+/// The cost of the narrow window is that an event raised outside it is missed — so the two
+/// instrumented sites had to be the ones that get there FIRST, and they are, by construction rather
+/// than by luck: CollectionBackgroundService.ExecuteAsync runs
+/// ServerManager.CheckAllConnectionsAsync BEFORE
+/// RemoteCollectorService.RunDueCollectorsAsync on every cycle including the first, and that
+/// loop is the only thing that drives collection — so the connectivity check is the first code in
+/// the process to open a connection to a monitored server. The interactive first-touch paths are
+/// the connection dialog's Test button and MainWindow's explicit retry, and both go through
+/// an instrumented open. EntraCredentialSelectionOrderingTests pins that ordering, because it
+/// lives in another file and a reorder there would silently blind this.
+///
+/// What that still does not cover, stated rather than implied. Lite has other
+/// SqlConnection sites that reach a monitored server — the bulk-add dialog, the excluded-
+/// databases dialog, a server tab's ad-hoc reads, the plan fetcher. Each needs an already-configured
+/// server, so in practice the sweep has run first; but a user who drives one of them inside the
+/// background service's five-second startup delay could acquire the first token there. The
+/// consequence is a "not observed" line rather than a wrong one, which
+/// is the direction to fail in — and it is why that line names the caching as the expected cause
+/// instead of asserting it.
///
/// Only the last REPORTED name outlives the attempt. Nothing else is retained: the
/// listener is gone and the captured value is read once. exists so an
From c849c9f134c34e84826c15ac24f2d6b85b033dab Mon Sep 17 00:00:00 2001
From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com>
Date: Wed, 9 Sep 2026 12:34:31 -0400
Subject: [PATCH 09/14] Record why the ordering pin strips, since nothing in
that file needs it today
No comment there names either identifier now, so the strip is invisible - until
someone reordering the loop writes a one-line ordering note, which is the case
the pin exists for. Measured: inverted loop plus such a comment passes on raw
text and fails on stripped text.
---
Lite.Tests/EntraCredentialSelectionOrderingTests.cs | 8 ++++++--
1 file changed, 6 insertions(+), 2 deletions(-)
diff --git a/Lite.Tests/EntraCredentialSelectionOrderingTests.cs b/Lite.Tests/EntraCredentialSelectionOrderingTests.cs
index 177399a71..15cbb348b 100644
--- a/Lite.Tests/EntraCredentialSelectionOrderingTests.cs
+++ b/Lite.Tests/EntraCredentialSelectionOrderingTests.cs
@@ -37,8 +37,12 @@ public class EntraCredentialSelectionOrderingTests
[Fact]
public void TheCollectionLoop_ChecksConnections_BeforeItCollects()
{
- /* Stripped, so the several comments in that method discussing the connection check and the
- collectors cannot satisfy the ordering below on their own. */
+ /* Stripped, and the strip is load-bearing rather than habit. No comment in that file names
+ either identifier TODAY, so the strip changes nothing today - but measured: with the loop
+ genuinely inverted AND one comment added naming CheckAllConnectionsAsync ahead of the
+ collector call, this pin passes on raw text and fails on stripped text. A one-line
+ ordering note of exactly that shape is the most natural thing for someone to write while
+ reordering this loop, which is the case where the pin has to still work. */
var source = CSharpSourceWalker.StripCommentsAndStrings(
ReadRepoFile("Lite/Services/CollectionBackgroundService.cs"));
From 257b7f0effd96a81bb5262f630c7bd96ceead904 Mon Sep 17 00:00:00 2001
From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com>
Date: Wed, 9 Sep 2026 12:47:21 -0400
Subject: [PATCH 10/14] Move RecordingListener's doc block back onto
RecordingListener
Two later insertions anchored on the class declaration rather than above its
doc comment, so each one pushed that comment further up and left the class
undocumented behind two stacked summaries. XML docs take the LAST one, so
tooling read correctly and only the file was misleading; Darling's whole-tree
DocCommentHygieneTests caught it, on Lite source, from the Darling suite.
The block is moved, not deleted - it documents a different member, which is
what that guard's own message warns about.
---
Lite.Tests/EntraCredentialSelectionTests.cs | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
diff --git a/Lite.Tests/EntraCredentialSelectionTests.cs b/Lite.Tests/EntraCredentialSelectionTests.cs
index fc6ff0178..7cbc563f2 100644
--- a/Lite.Tests/EntraCredentialSelectionTests.cs
+++ b/Lite.Tests/EntraCredentialSelectionTests.cs
@@ -789,15 +789,6 @@ private static string ReadRepoFile(string relativePath, [CallerFilePath] string
return File.ReadAllText(Path.Combine(new[] { dir! }.Concat(parts).ToArray()));
}
- ///
- /// An independent instrument: records every event id the enabled level actually delivers, so the
- /// production listener's filtering can be told apart from a level that delivered nothing.
- ///
- /// The bag is a FIELD INITIALISER, which in C# runs BEFORE the base constructor — the same
- /// hazard the production listener avoids with consts. Assigned in a constructor body it would be
- /// null when fires for an already-existing source, which is
- /// exactly the case every test here sets up.
- ///
///
/// An unrelated event source, existing only so "the listener enabled nothing else" has something
/// to be false about. Named outside the Azure- family on purpose.
@@ -848,6 +839,15 @@ protected override void OnEventWritten(EventWrittenEventArgs eventData)
}
}
+ ///
+ /// An independent instrument: records every event id the enabled level actually delivers, so the
+ /// production listener's filtering can be told apart from a level that delivered nothing.
+ ///
+ /// The bag is a FIELD INITIALISER, which in C# runs BEFORE the base constructor — the same
+ /// hazard the production listener avoids with consts. Assigned in a constructor body it would be
+ /// null when fires for an already-existing source, which is
+ /// exactly the case every test here sets up.
+ ///
private sealed class RecordingListener : EventListener
{
private readonly ConcurrentBag _seen = new();
From ffee046f0ba878c2cf8876b4b9f2b93064de4b95 Mon Sep 17 00:00:00 2001
From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com>
Date: Wed, 9 Sep 2026 12:59:28 -0400
Subject: [PATCH 11/14] Join app-logger-statics, and retire the invariant that
said nobody had to
Two pins here drain AppLogger's process-wide buffer and assert on what came
out, and one moves the process-wide minimum. LiteLogLevelGateTests created
app-logger-statics for exactly that and named the condition that would make the
hazard bite - a second class reading the log. This is that class, so it joins,
and the name now serialises two classes rather than nothing.
Its own remarks said this class is the only reader of the sink anywhere in the
suite, which this PR made false. Corrected there rather than left as a claim
someone would reason from: a class that only WRITES through the adapter still
does not need to join, and adding it would serialise the suite for nothing.
Two failure modes, both intermittent: DrainBufferedLines is destructive, so a
concurrent sweep dequeues the line this class is about to look for and tag
filtering cannot recover it; and those sweeps set the minimum as far as None,
so the Information line this class expects admitted is never enqueued.
---
Lite.Tests/EntraCredentialSelectionTests.cs | 19 ++++++++++++++++++
Lite.Tests/LiteLogLevelGateTests.cs | 22 ++++++++++++---------
2 files changed, 32 insertions(+), 9 deletions(-)
diff --git a/Lite.Tests/EntraCredentialSelectionTests.cs b/Lite.Tests/EntraCredentialSelectionTests.cs
index 7cbc563f2..64eb58cc3 100644
--- a/Lite.Tests/EntraCredentialSelectionTests.cs
+++ b/Lite.Tests/EntraCredentialSelectionTests.cs
@@ -46,6 +46,25 @@ namespace PerformanceMonitorLite.Tests;
/// az login session through this path — still unverified, still needs a tenant and a Windows
/// host. This file answers only for what the listener does with the event once it is raised.
///
+///
+/// In app-logger-statics because this class READS the sink. Two pins here drain
+/// 's process-wide buffer and assert on what came out, and one of them moves
+/// the process-wide minimum. LiteLogLevelGateTests created that collection for exactly this
+/// case and named the condition that would make the hazard bite: a second class reading the log.
+/// This is that class.
+///
+/// Two ways it would fail otherwise, both intermittent and neither reproducible on demand.
+/// DrainBufferedLines is destructive, so a concurrent sweep there dequeues the line this
+/// class is about to look for — and tag-filtering on the log source cannot recover a line another
+/// reader already took. And those sweeps set the minimum as far as , so
+/// the line this class expects to be admitted is never enqueued
+/// at all. Sharing the collection name serialises the two classes, which is the whole mechanism.
+///
+/// EntraCredentialSelectionModeGateTests and EntraCredentialSelectionOrderingTests
+/// deliberately do NOT join: neither touches , and a collection name that
+/// serialises classes which cannot race with each other costs suite time for nothing.
+///
+[Collection("app-logger-statics")]
public sealed class EntraCredentialSelectionTests : IDisposable
{
/* Azure.Identity 1.18.0, AzureIdentityEventSource.cs — every id and signature reflected below is
diff --git a/Lite.Tests/LiteLogLevelGateTests.cs b/Lite.Tests/LiteLogLevelGateTests.cs
index 07ff730bd..e9c254251 100644
--- a/Lite.Tests/LiteLogLevelGateTests.cs
+++ b/Lite.Tests/LiteLogLevelGateTests.cs
@@ -53,15 +53,19 @@ namespace PerformanceMonitorLite.Tests;
/// line that was never enqueued. That is the #1965 shape, which app-alert-statics was created
/// for.
///
-/// The interaction is real and currently harmless, which are two separate facts. Five other
-/// classes hand an to a service that logs —
-/// AnalysisNotificationTests, MuteRuleServiceTests, PagerDutyWebhookTests,
-/// RemainingEmptyReadsToolTests and WebhookCooldownSeedTests, against 28 log sites across
-/// four services — and each sits in its own default collection, so they run in parallel with the sweeps
-/// here and their lines really can be dropped. Nothing fails, because this class is the only reader of
-/// the sink anywhere in the suite: no other test asserts on log output, so a dropped line changes no
-/// assertion. The condition that would make it bite is any of those five reading the log, and then they
-/// join app-logger-statics.
+/// The interaction is real, and it now has a second party. Five other classes hand an
+/// to a service that logs — AnalysisNotificationTests,
+/// MuteRuleServiceTests, PagerDutyWebhookTests, RemainingEmptyReadsToolTests and
+/// WebhookCooldownSeedTests, against 28 log sites across four services — and each sits in its own
+/// default collection, so they run in parallel with the sweeps here and their lines really can be
+/// dropped. Nothing fails for those five, because none of them READS the sink: a dropped line changes no
+/// assertion of theirs.
+///
+/// This class is no longer the only reader, which is what the condition below was waiting
+/// for. EntraCredentialSelectionTests drains the buffer and asserts on what came out, so it
+/// joins app-logger-statics — which means the name now serialises two classes rather than
+/// nothing. Any further class that reads the log joins it too; a class that merely WRITES through the
+/// adapter still does not need to, and adding it would serialise the suite for no benefit.
///
/// Why a separate name rather than joining app-alert-statics. Its five members touch
/// App's alert-settings statics and make no AppLogger calls at all, so joining them would
From a0cf52bf4f30a4b45f3f375d7b0902a50cdfc8e8 Mon Sep 17 00:00:00 2001
From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com>
Date: Wed, 9 Sep 2026 13:15:03 -0400
Subject: [PATCH 12/14] The mode-gate class shares the event source, so it
shares the collection
Begin constructs a real EventListener over Azure.Identity's process-wide event
source, and the sibling class raises real events on that same source - a raised
event reaches every listener attached to it anywhere in the process. It changes
no assertion there today, because those pins only ask whether Begin returned a
listener and never read what one captured; the condition that would make it
bite is reading SelectedCredentialType or calling Report.
Joined to app-logger-statics rather than a new name: a class can be in only one
collection, so a separate event-source name would have left the one pairing
that matters unserialised. That widening is recorded in the collection's home
file too, so the name is not the only thing describing what it covers.
OrderingTests stays out and is the case that shows the rule is about REACH, not
subject matter - it is entirely about this feature and reads source files only.
---
.../EntraCredentialSelectionModeGateTests.cs | 15 ++++++++++
Lite.Tests/EntraCredentialSelectionTests.cs | 28 +++++++++++++++++--
Lite.Tests/LiteLogLevelGateTests.cs | 8 ++++++
3 files changed, 48 insertions(+), 3 deletions(-)
diff --git a/Lite.Tests/EntraCredentialSelectionModeGateTests.cs b/Lite.Tests/EntraCredentialSelectionModeGateTests.cs
index a43ec645f..a919aab96 100644
--- a/Lite.Tests/EntraCredentialSelectionModeGateTests.cs
+++ b/Lite.Tests/EntraCredentialSelectionModeGateTests.cs
@@ -33,6 +33,21 @@ namespace PerformanceMonitorLite.Tests;
/// Windows-only credential storage; the listener pins themselves need none of that and run in a
/// plain net10.0 harness on any platform.
///
+///
+/// In app-logger-statics for the event source, not for the log. Nothing here
+/// touches — but both pins call
+/// Begin, which constructs a real over
+/// Azure.Identity's process-wide event source, and EntraCredentialSelectionTests raises
+/// real events on that same source. A raised event reaches every listener attached to it anywhere in
+/// the process.
+///
+/// It changes no assertion here today, because these pins only ask whether Begin returned
+/// a listener and never read what one captured. Joined anyway, and joined to THAT name rather than a
+/// new one: a class can only be in one collection, so a separate event-source collection would leave
+/// the one pairing that matters — this class against the class that raises the events —
+/// unserialised.
+///
+[Collection("app-logger-statics")]
public class EntraCredentialSelectionModeGateTests
{
///
diff --git a/Lite.Tests/EntraCredentialSelectionTests.cs b/Lite.Tests/EntraCredentialSelectionTests.cs
index 64eb58cc3..43df47be8 100644
--- a/Lite.Tests/EntraCredentialSelectionTests.cs
+++ b/Lite.Tests/EntraCredentialSelectionTests.cs
@@ -60,9 +60,31 @@ namespace PerformanceMonitorLite.Tests;
/// the line this class expects to be admitted is never enqueued
/// at all. Sharing the collection name serialises the two classes, which is the whole mechanism.
///
-/// EntraCredentialSelectionModeGateTests and EntraCredentialSelectionOrderingTests
-/// deliberately do NOT join: neither touches , and a collection name that
-/// serialises classes which cannot race with each other costs suite time for nothing.
+/// And a SECOND process-wide static, which is why that collection name now covers more than
+/// its name says. This class raises real events on Azure.Identity's
+/// AzureIdentityEventSource singleton and constructs s over it, and
+/// both are process-wide: enabling an is a global effect, and a raised event
+/// is delivered to EVERY listener attached to that source anywhere in the process.
+/// EntraCredentialSelectionModeGateTests calls Begin, so it constructs real listeners on
+/// the same source — it therefore joins the same collection. Not because it touches
+/// , which it does not, but because a class cannot be in two collections: a
+/// separate name for the event-source hazard would fail to serialise it against THIS class, which is
+/// the one pairing that matters.
+///
+/// Harmless today and that is a separate fact from being safe: the mode-gate class never reads a
+/// listener's captured state and never calls Report, so a synthetic event landing in its
+/// listener changes no assertion of its own. The condition that would make it bite is that class — or
+/// any future one — reading SelectedCredentialType, RejectedPayload, or calling
+/// Report. Serialising now costs two small classes' parallelism and removes the question.
+///
+/// EntraCredentialSelectionOrderingTests stays out, and is the case that shows the rule is
+/// about REACH rather than about subject matter: it is entirely about this feature, and it reads source
+/// files only — no , no listener, no event source, nothing process-wide to
+/// share.
+///
+/// These are the only subclasses and the only Azure-Identity
+/// consumers anywhere in the repository, measured rather than assumed, so no third party can observe
+/// the events raised here today.
///
[Collection("app-logger-statics")]
public sealed class EntraCredentialSelectionTests : IDisposable
diff --git a/Lite.Tests/LiteLogLevelGateTests.cs b/Lite.Tests/LiteLogLevelGateTests.cs
index e9c254251..b584c1e64 100644
--- a/Lite.Tests/LiteLogLevelGateTests.cs
+++ b/Lite.Tests/LiteLogLevelGateTests.cs
@@ -67,6 +67,14 @@ namespace PerformanceMonitorLite.Tests;
/// nothing. Any further class that reads the log joins it too; a class that merely WRITES through the
/// adapter still does not need to, and adding it would serialise the suite for no benefit.
///
+/// The name now covers two unrelated process-wide statics, which is a consequence of xUnit's
+/// model rather than a taxonomy choice. EntraCredentialSelectionTests and
+/// EntraCredentialSelectionModeGateTests also share Azure.Identity's process-wide event
+/// source — one raises events on it, the other attaches listeners to it. A class can belong to only one
+/// collection, so a second name for that hazard would have failed to serialise those two against each
+/// other. One name, two reasons, and the reasons are recorded at each member rather than inferred from
+/// the name.
+///
/// Why a separate name rather than joining app-alert-statics. Its five members touch
/// App's alert-settings statics and make no AppLogger calls at all, so joining them would
/// serialise this class against classes that cannot race with it while still not serialising the five
From 49d54e36cc44f0df0fa320a8ff38ef78dcda8ae1 Mon Sep 17 00:00:00 2001
From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com>
Date: Wed, 9 Sep 2026 13:17:00 -0400
Subject: [PATCH 13/14] Pin that no observed selection writes nothing on a
default install
The acceptance criterion for the case where the chain found nothing, or where
the driver's cache means this connection raised no event: it must not record a
selection that did not happen - not an empty type name, not unknown, not a line
at all at the default minimum. Only the repeat case was pinned at that level;
the never-observed case was pinned only at a lowered one, so the property that
matters on a shipped install was untested.
The Debug explanation still exists and is still asserted at a lowered minimum,
because an operator who goes looking should find the reason rather than
silence. Those are two questions and only one of them is about the artifact.
---
Lite.Tests/EntraCredentialSelectionTests.cs | 44 +++++++++++++++++++++
1 file changed, 44 insertions(+)
diff --git a/Lite.Tests/EntraCredentialSelectionTests.cs b/Lite.Tests/EntraCredentialSelectionTests.cs
index 43df47be8..3036661c5 100644
--- a/Lite.Tests/EntraCredentialSelectionTests.cs
+++ b/Lite.Tests/EntraCredentialSelectionTests.cs
@@ -650,6 +650,50 @@ public void Report_WritesTheFirstObservationAtTheDefaultLevel_AndNotTheRepeat()
Assert.Empty(Ours());
}
+ ///
+ /// A listener that observed no selection writes NOTHING at the default configuration.
+ /// This is the acceptance criterion for the case where the credential chain found nothing, or where
+ /// the driver's cache means this connection raised no event: it must not record a selection that
+ /// did not happen — not an empty type name, not "unknown", not a line at all on a default
+ /// install.
+ ///
+ /// The explanation exists and is asserted separately below, at a
+ /// lowered minimum, because an operator who goes looking should find "the event fires once per
+ /// process" rather than silence. Two different questions, and only this one is about what a shipped
+ /// install writes.
+ ///
+ /// The positive control is the first assertion: a first observation on the same listener type
+ /// at the same level does reach the log, so the emptiness below is a decision about having nothing
+ /// to say rather than a sink that was never wired up.
+ ///
+ [Fact]
+ public void Report_WritesNothingAtTheDefaultLevel_WhenNoSelectionWasObserved()
+ {
+ Singleton();
+ EntraCredentialSelectionLog.ResetForTests();
+ Assert.Equal(LogLevel.Information, AppLogger.MinimumLevel);
+ AppLogger.DrainBufferedLines();
+
+ using (var observed = new EntraCredentialSelectionListener())
+ {
+ Raise(SelectedMethod, CliCredential);
+ EntraCredentialSelectionLog.Report(observed);
+ }
+
+ Assert.Single(Ours());
+
+ EntraCredentialSelectionLog.ResetForTests();
+
+ using (var silent = new EntraCredentialSelectionListener())
+ {
+ Assert.Null(silent.SelectedCredentialType);
+ Assert.False(silent.RejectedPayload);
+ EntraCredentialSelectionLog.Report(silent);
+ }
+
+ Assert.Empty(Ours());
+ }
+
///
/// The same repeat, with the minimum lowered — so "not written at the default" above is a level
/// decision rather than a that produced no line at all.
From 8845ac78dd402293555a02bdc4f249b9a7d6ffc0 Mon Sep 17 00:00:00 2001
From: Erik Darling <2136037+erikdarlingdata@users.noreply.github.com>
Date: Wed, 9 Sep 2026 13:37:08 -0400
Subject: [PATCH 14/14] Say process-wide about the log's granularity, not about
the selection
The dedupe was justified by calling the selection a process fact. The chain it
resolves from is process-wide, but the driver keys its credential cache on
authority, scope, audience and client id - so two servers in different tenants
get different DefaultAzureCredential instances and each raises its own event.
Collapsing them when they pick the same type is still right, for a different
reason: the type name is all that is recorded, so the second line would repeat
it. Telling them apart would need the tenant or the server on the line.
---
Lite/Helpers/EntraCredentialSelection.cs | 11 +++++++++--
1 file changed, 9 insertions(+), 2 deletions(-)
diff --git a/Lite/Helpers/EntraCredentialSelection.cs b/Lite/Helpers/EntraCredentialSelection.cs
index f246e4f58..bf9facb57 100644
--- a/Lite/Helpers/EntraCredentialSelection.cs
+++ b/Lite/Helpers/EntraCredentialSelection.cs
@@ -187,8 +187,15 @@ exception message each fail the shape check on a character a type name cannot co
/// Only the last REPORTED name outlives the attempt. Nothing else is retained: the
/// listener is gone and the captured value is read once. exists so an
/// unchanged repeat is a line instead of an
-/// one, because the selection is a process fact and a monitoring
-/// tool restating it once per server per sweep would be noise. A CHANGED name still reports at
+/// one, because a monitoring tool restating it once per server per
+/// sweep would be noise. Process-wide is the right granularity for what is logged, which is not
+/// quite the same as the selection being a process fact. The chain it resolves from — environment
+/// variables, an az login session, machine identity — is genuinely process-wide, but the
+/// driver keys its credential cache on authority, scope, audience and client id, so two servers in
+/// different tenants get different DefaultAzureCredential instances and each raises its own
+/// event. Both are collapsed here when they select the same TYPE, deliberately: the type name is all
+/// that is recorded, so a second line would repeat it and add nothing. Telling those two apart would
+/// need the tenant or the server on the line, and neither belongs there. A CHANGED name still reports at
/// — the driver clears its credential cache
/// (ActiveDirectoryAuthenticationProvider.cs:136-138), so a different source genuinely can
/// win later, and that is the one thing here worth interrupting someone with.