From b2fe2cb1ba940e9f0ab664a1ef2b0a74a75b1a69 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Fri, 11 Jul 2025 15:10:19 +0800 Subject: [PATCH 1/5] Replace _UNKNOWN with _OTHER in identity metrics --- src/Identity/Extensions.Core/src/UserManagerMetrics.cs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Identity/Extensions.Core/src/UserManagerMetrics.cs b/src/Identity/Extensions.Core/src/UserManagerMetrics.cs index b534a0d38ce6..0f3c49f3e5d6 100644 --- a/src/Identity/Extensions.Core/src/UserManagerMetrics.cs +++ b/src/Identity/Extensions.Core/src/UserManagerMetrics.cs @@ -169,7 +169,7 @@ private static string GetTokenPurpose(string purpose) "EmailConfirmation" => "email_confirmation", "ChangeEmail" => "change_email", "TwoFactor" => "two_factor", - _ => "_UNKNOWN" + _ => "_OTHER" }; } @@ -204,7 +204,7 @@ private static string GetPasswordResult(PasswordVerificationResult? result, bool (PasswordVerificationResult.Failed, false, false) => "failure", (null, true, false) => "password_missing", (null, false, true) => "user_missing", - _ => "_UNKNOWN" + _ => "_OTHER" }; } @@ -244,7 +244,7 @@ private static string GetUpdateType(UserUpdateType updateType) UserUpdateType.RedeemTwoFactorRecoveryCode => "redeem_two_factor_recovery_code", UserUpdateType.SetPasskey => "set_passkey", UserUpdateType.RemovePasskey => "remove_passkey", - _ => "_UNKNOWN" + _ => "_OTHER" }; } From 58719a024c64372a15f6274bd660e650dffd17d8 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Sat, 12 Jul 2025 06:56:25 +0800 Subject: [PATCH 2/5] More --- src/Identity/Core/src/SignInManagerMetrics.cs | 2 +- src/Identity/test/Identity.Test/UserManagerTest.cs | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/src/Identity/Core/src/SignInManagerMetrics.cs b/src/Identity/Core/src/SignInManagerMetrics.cs index cfd81642dbfd..5eaca17dab4f 100644 --- a/src/Identity/Core/src/SignInManagerMetrics.cs +++ b/src/Identity/Core/src/SignInManagerMetrics.cs @@ -182,7 +182,7 @@ private static string GetSignInType(SignInType signInType) SignInType.TwoFactor => "two_factor", SignInType.External => "external", SignInType.Passkey => "passkey", - _ => "_UNKNOWN" + _ => "_OTHER" }; } diff --git a/src/Identity/test/Identity.Test/UserManagerTest.cs b/src/Identity/test/Identity.Test/UserManagerTest.cs index e82932e5e90a..3339c771c76d 100644 --- a/src/Identity/test/Identity.Test/UserManagerTest.cs +++ b/src/Identity/test/Identity.Test/UserManagerTest.cs @@ -952,13 +952,13 @@ await Assert.ThrowsAsync( Assert.Collection(generateToken.GetMeasurementSnapshot(), m => MetricsHelpers.AssertContainsTags(m.Tags, [ - KeyValuePair.Create("aspnetcore.identity.token_purpose", "_UNKNOWN"), + KeyValuePair.Create("aspnetcore.identity.token_purpose", "_OTHER"), KeyValuePair.Create("error.type", "System.NotSupportedException"), ])); Assert.Collection(verifyToken.GetMeasurementSnapshot(), m => MetricsHelpers.AssertContainsTags(m.Tags, [ - KeyValuePair.Create("aspnetcore.identity.token_purpose", "_UNKNOWN"), + KeyValuePair.Create("aspnetcore.identity.token_purpose", "_OTHER"), KeyValuePair.Create("error.type", "System.NotSupportedException"), ])); } From fc6ad96401124e691721cc51ee75fdd619c9dfd5 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Tue, 15 Jul 2025 13:43:46 +0800 Subject: [PATCH 3/5] Feedback from review --- src/Identity/Core/src/SignInManagerMetrics.cs | 16 +++++----- .../Extensions.Core/src/UserManagerMetrics.cs | 29 ++++++++++--------- .../test/Identity.Test/UserManagerTest.cs | 6 ++-- 3 files changed, 26 insertions(+), 25 deletions(-) diff --git a/src/Identity/Core/src/SignInManagerMetrics.cs b/src/Identity/Core/src/SignInManagerMetrics.cs index 5eaca17dab4f..7fb39262b61a 100644 --- a/src/Identity/Core/src/SignInManagerMetrics.cs +++ b/src/Identity/Core/src/SignInManagerMetrics.cs @@ -14,8 +14,8 @@ internal sealed class SignInManagerMetrics : IDisposable public const string RememberTwoFactorCounterName = "aspnetcore.identity.sign_in.remember_two_factor"; public const string ForgetTwoFactorCounterName = "aspnetcore.identity.sign_in.forget_two_factor"; public const string CheckPasswordCounterName = "aspnetcore.identity.sign_in.check_password"; - public const string SignInUserPrincipalCounterName = "aspnetcore.identity.sign_in.sign_in_principal"; - public const string SignOutUserPrincipalCounterName = "aspnetcore.identity.sign_in.sign_out_principal"; + public const string SignInUserPrincipalCounterName = "aspnetcore.identity.sign_in.sign_in"; + public const string SignOutUserPrincipalCounterName = "aspnetcore.identity.sign_in.sign_out"; private readonly Meter _meter; private readonly Counter _authenticateCounter; @@ -29,12 +29,12 @@ public SignInManagerMetrics(IMeterFactory meterFactory) { _meter = meterFactory.Create(MeterName); - _authenticateCounter = _meter.CreateCounter(AuthenticateCounterName, "count", "The number of authenticate attempts. The authenticate counter is incremented by sign in methods such as PasswordSignInAsync and TwoFactorSignInAsync."); - _rememberTwoFactorClientCounter = _meter.CreateCounter(RememberTwoFactorCounterName, "count", "The number of two factor clients remembered."); - _forgetTwoFactorCounter = _meter.CreateCounter(ForgetTwoFactorCounterName, "count", "The number of two factor clients forgotten."); - _checkPasswordCounter = _meter.CreateCounter(CheckPasswordCounterName, "count", "The number of check password attempts. Checks that the account is in a state that can log in and that the password is valid using the UserManager.CheckPasswordAsync method."); - _signInUserPrincipalCounter = _meter.CreateCounter(SignInUserPrincipalCounterName, "count", "The number of calls to sign in user principals."); - _signOutUserPrincipalCounter = _meter.CreateCounter(SignOutUserPrincipalCounterName, "count", "The number of calls to sign out user principals."); + _authenticateCounter = _meter.CreateCounter(AuthenticateCounterName, "{count}", "The number of authenticate attempts. The authenticate counter is incremented by sign in methods such as PasswordSignInAsync and TwoFactorSignInAsync."); + _rememberTwoFactorClientCounter = _meter.CreateCounter(RememberTwoFactorCounterName, "{count}", "The number of two factor clients remembered."); + _forgetTwoFactorCounter = _meter.CreateCounter(ForgetTwoFactorCounterName, "{count}", "The number of two factor clients forgotten."); + _checkPasswordCounter = _meter.CreateCounter(CheckPasswordCounterName, "{check}", "The number of check password attempts. Checks that the account is in a state that can log in and that the password is valid using the UserManager.CheckPasswordAsync method."); + _signInUserPrincipalCounter = _meter.CreateCounter(SignInUserPrincipalCounterName, "{sign_in}", "The number of calls to sign in user principals."); + _signOutUserPrincipalCounter = _meter.CreateCounter(SignOutUserPrincipalCounterName, "{sign_out}", "The number of calls to sign out user principals."); } internal void CheckPasswordSignIn(string userType, SignInResult? result, Exception? exception = null) diff --git a/src/Identity/Extensions.Core/src/UserManagerMetrics.cs b/src/Identity/Extensions.Core/src/UserManagerMetrics.cs index 0f3c49f3e5d6..b5b937fb2799 100644 --- a/src/Identity/Extensions.Core/src/UserManagerMetrics.cs +++ b/src/Identity/Extensions.Core/src/UserManagerMetrics.cs @@ -32,12 +32,12 @@ internal sealed class UserManagerMetrics : IDisposable public UserManagerMetrics(IMeterFactory meterFactory) { _meter = meterFactory.Create(MeterName); - _createCounter = _meter.CreateCounter(CreateCounterName, "count", "The number of users created."); - _updateCounter = _meter.CreateCounter(UpdateCounterName, "count", "The number of user updates."); - _deleteCounter = _meter.CreateCounter(DeleteCounterName, "count", "The number of users deleted."); - _checkPasswordCounter = _meter.CreateCounter(CheckPasswordCounterName, "count", "The number of check password attempts. Only checks whether the password is valid and not whether the user account is in a state that can log in."); - _verifyTokenCounter = _meter.CreateCounter(VerifyTokenCounterName, "count", "The number of token verification attempts."); - _generateTokenCounter = _meter.CreateCounter(GenerateTokenCounterName, "count", "The number of token generation attempts."); + _createCounter = _meter.CreateCounter(CreateCounterName, "{create}", "The number of users created."); + _updateCounter = _meter.CreateCounter(UpdateCounterName, "{update}", "The number of user updates."); + _deleteCounter = _meter.CreateCounter(DeleteCounterName, "{delete}", "The number of users deleted."); + _checkPasswordCounter = _meter.CreateCounter(CheckPasswordCounterName, "{check}", "The number of check password attempts. Only checks whether the password is valid and not whether the user account is in a state that can log in."); + _verifyTokenCounter = _meter.CreateCounter(VerifyTokenCounterName, "{count}", "The number of token verification attempts."); + _generateTokenCounter = _meter.CreateCounter(GenerateTokenCounterName, "{count}", "The number of token generation attempts."); } internal void CreateUser(string userType, IdentityResult? result, Exception? exception = null) @@ -52,7 +52,7 @@ internal void CreateUser(string userType, IdentityResult? result, Exception? exc { "aspnetcore.identity.user_type", userType } }; AddIdentityResultTags(ref tags, result); - AddExceptionTags(ref tags, exception); + AddExceptionTags(ref tags, exception, result: result); _createCounter.Add(1, tags); } @@ -70,7 +70,7 @@ internal void UpdateUser(string userType, IdentityResult? result, UserUpdateType { "aspnetcore.identity.user.update_type", GetUpdateType(updateType) }, }; AddIdentityResultTags(ref tags, result); - AddExceptionTags(ref tags, exception); + AddExceptionTags(ref tags, exception, result: result); _updateCounter.Add(1, tags); } @@ -87,7 +87,7 @@ internal void DeleteUser(string userType, IdentityResult? result, Exception? exc { "aspnetcore.identity.user_type", userType } }; AddIdentityResultTags(ref tags, result); - AddExceptionTags(ref tags, exception); + AddExceptionTags(ref tags, exception, result: result); _deleteCounter.Add(1, tags); } @@ -105,7 +105,7 @@ internal void CheckPassword(string userType, bool? userMissing, PasswordVerifica }; if (userMissing != null || result != null) { - tags.Add("aspnetcore.identity.user.password_result", GetPasswordResult(result, passwordMissing: null, userMissing)); + tags.Add("aspnetcore.identity.password_check_result", GetPasswordResult(result, passwordMissing: null, userMissing)); } AddExceptionTags(ref tags, exception); @@ -183,15 +183,16 @@ private static void AddIdentityResultTags(ref TagList tags, IdentityResult? resu tags.Add("aspnetcore.identity.result", result.Succeeded ? "success" : "failure"); if (!result.Succeeded && result.Errors.FirstOrDefault()?.Code is { Length: > 0 } code) { - tags.Add("aspnetcore.identity.result_error_code", code); + tags.Add("aspnetcore.identity.error_code", code); } } - private static void AddExceptionTags(ref TagList tags, Exception? exception) + private static void AddExceptionTags(ref TagList tags, Exception? exception, IdentityResult? result = null) { - if (exception != null) + var value = exception?.GetType().FullName ?? result?.Errors.FirstOrDefault()?.Code; + if (value != null) { - tags.Add("error.type", exception.GetType().FullName!); + tags.Add("error.type", value); } } diff --git a/src/Identity/test/Identity.Test/UserManagerTest.cs b/src/Identity/test/Identity.Test/UserManagerTest.cs index 3339c771c76d..69eac50cdb83 100644 --- a/src/Identity/test/Identity.Test/UserManagerTest.cs +++ b/src/Identity/test/Identity.Test/UserManagerTest.cs @@ -174,7 +174,7 @@ public async Task DeleteCallsStore_Failure() [ KeyValuePair.Create("aspnetcore.identity.user_type", "Microsoft.AspNetCore.Identity.Test.PocoUser"), KeyValuePair.Create("aspnetcore.identity.result", "failure"), - KeyValuePair.Create("aspnetcore.identity.result_error_code", "ConcurrencyFailure") + KeyValuePair.Create("aspnetcore.identity.error_code", "ConcurrencyFailure") ])); } @@ -664,7 +664,7 @@ public async Task CheckPasswordWillRehashPasswordWhenNeeded() Assert.Collection(checkPassword.GetMeasurementSnapshot(), m => MetricsHelpers.AssertContainsTags(m.Tags, [ - KeyValuePair.Create("aspnetcore.identity.user.password_result", "success_rehash_needed") + KeyValuePair.Create("aspnetcore.identity.password_check_result", "success_rehash_needed") ])); } @@ -871,7 +871,7 @@ public async Task CheckPasswordWithNullUserReturnsFalse() Assert.Collection(checkPassword.GetMeasurementSnapshot(), m => MetricsHelpers.AssertContainsTags(m.Tags, [ - KeyValuePair.Create("aspnetcore.identity.user.password_result", "user_missing") + KeyValuePair.Create("aspnetcore.identity.password_check_result", "user_missing") ])); } From 1725d2655cc6cb9e25947d8bd97332fcabc3fdd2 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Tue, 15 Jul 2025 15:22:33 +0800 Subject: [PATCH 4/5] PR feedback --- src/Identity/Core/src/SignInManagerMetrics.cs | 14 ++++++------- .../Extensions.Core/src/UserManagerMetrics.cs | 20 +++++++++---------- 2 files changed, 17 insertions(+), 17 deletions(-) diff --git a/src/Identity/Core/src/SignInManagerMetrics.cs b/src/Identity/Core/src/SignInManagerMetrics.cs index 7fb39262b61a..f14c37edf042 100644 --- a/src/Identity/Core/src/SignInManagerMetrics.cs +++ b/src/Identity/Core/src/SignInManagerMetrics.cs @@ -49,7 +49,7 @@ internal void CheckPasswordSignIn(string userType, SignInResult? result, Excepti { "aspnetcore.identity.user_type", userType }, }; AddSignInResult(ref tags, result); - AddExceptionTags(ref tags, exception); + AddErrorTag(ref tags, exception); _checkPasswordCounter.Add(1, tags); } @@ -69,7 +69,7 @@ internal void AuthenticateSignIn(string userType, string authenticationScheme, S }; AddIsPersistent(ref tags, isPersistent); AddSignInResult(ref tags, result); - AddExceptionTags(ref tags, exception); + AddErrorTag(ref tags, exception); _authenticateCounter.Add(1, tags); } @@ -87,7 +87,7 @@ internal void SignInUserPrincipal(string userType, string authenticationScheme, { "aspnetcore.identity.authentication_scheme", authenticationScheme }, }; AddIsPersistent(ref tags, isPersistent); - AddExceptionTags(ref tags, exception); + AddErrorTag(ref tags, exception); _signInUserPrincipalCounter.Add(1, tags); } @@ -104,7 +104,7 @@ internal void SignOutUserPrincipal(string userType, string authenticationScheme, { "aspnetcore.identity.user_type", userType }, { "aspnetcore.identity.authentication_scheme", authenticationScheme }, }; - AddExceptionTags(ref tags, exception); + AddErrorTag(ref tags, exception); _signOutUserPrincipalCounter.Add(1, tags); } @@ -121,7 +121,7 @@ internal void RememberTwoFactorClient(string userType, string authenticationSche { "aspnetcore.identity.user_type", userType }, { "aspnetcore.identity.authentication_scheme", authenticationScheme } }; - AddExceptionTags(ref tags, exception); + AddErrorTag(ref tags, exception); _rememberTwoFactorClientCounter.Add(1, tags); } @@ -138,7 +138,7 @@ internal void ForgetTwoFactorClient(string userType, string authenticationScheme { "aspnetcore.identity.user_type", userType }, { "aspnetcore.identity.authentication_scheme", authenticationScheme } }; - AddExceptionTags(ref tags, exception); + AddErrorTag(ref tags, exception); _forgetTwoFactorCounter.Add(1, tags); } @@ -164,7 +164,7 @@ private static void AddSignInResult(ref TagList tags, SignInResult? result) } } - private static void AddExceptionTags(ref TagList tags, Exception? exception) + private static void AddErrorTag(ref TagList tags, Exception? exception) { if (exception != null) { diff --git a/src/Identity/Extensions.Core/src/UserManagerMetrics.cs b/src/Identity/Extensions.Core/src/UserManagerMetrics.cs index b5b937fb2799..a969034b5579 100644 --- a/src/Identity/Extensions.Core/src/UserManagerMetrics.cs +++ b/src/Identity/Extensions.Core/src/UserManagerMetrics.cs @@ -32,9 +32,9 @@ internal sealed class UserManagerMetrics : IDisposable public UserManagerMetrics(IMeterFactory meterFactory) { _meter = meterFactory.Create(MeterName); - _createCounter = _meter.CreateCounter(CreateCounterName, "{create}", "The number of users created."); - _updateCounter = _meter.CreateCounter(UpdateCounterName, "{update}", "The number of user updates."); - _deleteCounter = _meter.CreateCounter(DeleteCounterName, "{delete}", "The number of users deleted."); + _createCounter = _meter.CreateCounter(CreateCounterName, "{user}", "The number of users created."); + _updateCounter = _meter.CreateCounter(UpdateCounterName, "{user}", "The number of users updated."); + _deleteCounter = _meter.CreateCounter(DeleteCounterName, "{user}", "The number of users deleted."); _checkPasswordCounter = _meter.CreateCounter(CheckPasswordCounterName, "{check}", "The number of check password attempts. Only checks whether the password is valid and not whether the user account is in a state that can log in."); _verifyTokenCounter = _meter.CreateCounter(VerifyTokenCounterName, "{count}", "The number of token verification attempts."); _generateTokenCounter = _meter.CreateCounter(GenerateTokenCounterName, "{count}", "The number of token generation attempts."); @@ -52,7 +52,7 @@ internal void CreateUser(string userType, IdentityResult? result, Exception? exc { "aspnetcore.identity.user_type", userType } }; AddIdentityResultTags(ref tags, result); - AddExceptionTags(ref tags, exception, result: result); + AddErrorTag(ref tags, exception, result: result); _createCounter.Add(1, tags); } @@ -70,7 +70,7 @@ internal void UpdateUser(string userType, IdentityResult? result, UserUpdateType { "aspnetcore.identity.user.update_type", GetUpdateType(updateType) }, }; AddIdentityResultTags(ref tags, result); - AddExceptionTags(ref tags, exception, result: result); + AddErrorTag(ref tags, exception, result: result); _updateCounter.Add(1, tags); } @@ -87,7 +87,7 @@ internal void DeleteUser(string userType, IdentityResult? result, Exception? exc { "aspnetcore.identity.user_type", userType } }; AddIdentityResultTags(ref tags, result); - AddExceptionTags(ref tags, exception, result: result); + AddErrorTag(ref tags, exception, result: result); _deleteCounter.Add(1, tags); } @@ -107,7 +107,7 @@ internal void CheckPassword(string userType, bool? userMissing, PasswordVerifica { tags.Add("aspnetcore.identity.password_check_result", GetPasswordResult(result, passwordMissing: null, userMissing)); } - AddExceptionTags(ref tags, exception); + AddErrorTag(ref tags, exception); _checkPasswordCounter.Add(1, tags); } @@ -128,7 +128,7 @@ internal void VerifyToken(string userType, bool? result, string purpose, Excepti { tags.Add("aspnetcore.identity.token_verified", result == true ? "success" : "failure"); } - AddExceptionTags(ref tags, exception); + AddErrorTag(ref tags, exception); _verifyTokenCounter.Add(1, tags); } @@ -145,7 +145,7 @@ internal void GenerateToken(string userType, string purpose, Exception? exceptio { "aspnetcore.identity.user_type", userType }, { "aspnetcore.identity.token_purpose", GetTokenPurpose(purpose) }, }; - AddExceptionTags(ref tags, exception); + AddErrorTag(ref tags, exception); _generateTokenCounter.Add(1, tags); } @@ -187,7 +187,7 @@ private static void AddIdentityResultTags(ref TagList tags, IdentityResult? resu } } - private static void AddExceptionTags(ref TagList tags, Exception? exception, IdentityResult? result = null) + private static void AddErrorTag(ref TagList tags, Exception? exception, IdentityResult? result = null) { var value = exception?.GetType().FullName ?? result?.Errors.FirstOrDefault()?.Code; if (value != null) From 5d2d90e7517251b6eb0244e69ae6f78229916920 Mon Sep 17 00:00:00 2001 From: James Newton-King Date: Tue, 15 Jul 2025 15:24:07 +0800 Subject: [PATCH 5/5] Comment --- src/Identity/Extensions.Core/src/UserManagerMetrics.cs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/Identity/Extensions.Core/src/UserManagerMetrics.cs b/src/Identity/Extensions.Core/src/UserManagerMetrics.cs index a969034b5579..e0f6794b8fc3 100644 --- a/src/Identity/Extensions.Core/src/UserManagerMetrics.cs +++ b/src/Identity/Extensions.Core/src/UserManagerMetrics.cs @@ -152,7 +152,7 @@ internal void GenerateToken(string userType, string purpose, Exception? exceptio private static string GetTokenPurpose(string purpose) { - // Purpose could be any value and can't be used as a tag value. However, there are known purposes + // Purpose could be any value and can't be used directly as a tag value. However, there are known purposes // on UserManager that we can detect and use as a tag value. Some could have a ':' in them followed by user data. // We need to trim them to content before ':' and then match to known values. ReadOnlySpan trimmedPurpose = purpose; @@ -161,7 +161,8 @@ private static string GetTokenPurpose(string purpose) { trimmedPurpose = purpose.AsSpan(0, colonIndex); } - + + // These are known purposes that are specified in ASP.NET Core Identity. return trimmedPurpose switch { "ResetPassword" => "reset_password",