Skip to content
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 9 additions & 9 deletions src/Identity/Core/src/SignInManagerMetrics.cs
Original file line number Diff line number Diff line change
Expand Up @@ -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<long> _authenticateCounter;
Expand All @@ -29,12 +29,12 @@ public SignInManagerMetrics(IMeterFactory meterFactory)
{
_meter = meterFactory.Create(MeterName);

_authenticateCounter = _meter.CreateCounter<long>(AuthenticateCounterName, "count", "The number of authenticate attempts. The authenticate counter is incremented by sign in methods such as PasswordSignInAsync and TwoFactorSignInAsync.");
_rememberTwoFactorClientCounter = _meter.CreateCounter<long>(RememberTwoFactorCounterName, "count", "The number of two factor clients remembered.");
_forgetTwoFactorCounter = _meter.CreateCounter<long>(ForgetTwoFactorCounterName, "count", "The number of two factor clients forgotten.");
_checkPasswordCounter = _meter.CreateCounter<long>(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<long>(SignInUserPrincipalCounterName, "count", "The number of calls to sign in user principals.");
_signOutUserPrincipalCounter = _meter.CreateCounter<long>(SignOutUserPrincipalCounterName, "count", "The number of calls to sign out user principals.");
_authenticateCounter = _meter.CreateCounter<long>(AuthenticateCounterName, "{count}", "The number of authenticate attempts. The authenticate counter is incremented by sign in methods such as PasswordSignInAsync and TwoFactorSignInAsync.");
_rememberTwoFactorClientCounter = _meter.CreateCounter<long>(RememberTwoFactorCounterName, "{count}", "The number of two factor clients remembered.");
_forgetTwoFactorCounter = _meter.CreateCounter<long>(ForgetTwoFactorCounterName, "{count}", "The number of two factor clients forgotten.");
_checkPasswordCounter = _meter.CreateCounter<long>(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<long>(SignInUserPrincipalCounterName, "{sign_in}", "The number of calls to sign in user principals.");
_signOutUserPrincipalCounter = _meter.CreateCounter<long>(SignOutUserPrincipalCounterName, "{sign_out}", "The number of calls to sign out user principals.");
}

internal void CheckPasswordSignIn(string userType, SignInResult? result, Exception? exception = null)
Expand Down Expand Up @@ -182,7 +182,7 @@ private static string GetSignInType(SignInType signInType)
SignInType.TwoFactor => "two_factor",
SignInType.External => "external",
SignInType.Passkey => "passkey",
_ => "_UNKNOWN"
_ => "_OTHER"
};
}

Expand Down
35 changes: 18 additions & 17 deletions src/Identity/Extensions.Core/src/UserManagerMetrics.cs
Original file line number Diff line number Diff line change
Expand Up @@ -32,12 +32,12 @@ internal sealed class UserManagerMetrics : IDisposable
public UserManagerMetrics(IMeterFactory meterFactory)
{
_meter = meterFactory.Create(MeterName);
_createCounter = _meter.CreateCounter<long>(CreateCounterName, "count", "The number of users created.");
_updateCounter = _meter.CreateCounter<long>(UpdateCounterName, "count", "The number of user updates.");
_deleteCounter = _meter.CreateCounter<long>(DeleteCounterName, "count", "The number of users deleted.");
_checkPasswordCounter = _meter.CreateCounter<long>(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<long>(VerifyTokenCounterName, "count", "The number of token verification attempts.");
_generateTokenCounter = _meter.CreateCounter<long>(GenerateTokenCounterName, "count", "The number of token generation attempts.");
_createCounter = _meter.CreateCounter<long>(CreateCounterName, "{create}", "The number of users created.");

Choose a reason for hiding this comment

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

it counts users, so should it be

Suggested change
_createCounter = _meter.CreateCounter<long>(CreateCounterName, "{create}", "The number of users created.");
_createCounter = _meter.CreateCounter<long>(CreateCounterName, "{user}", "The number of users created.");

?

Copy link
Member Author

Choose a reason for hiding this comment

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

You'd like create, update and delete metrics to have a unit of {user}?

Copy link

@lmolkova lmolkova Jul 17, 2025

Choose a reason for hiding this comment

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

the number of users create/deleted is the number of users.
The number of updates is not the number of users, so I'd prefer {update} for it. I don't see a strong reason for all three to have the same unit.

It's not a super-strong opinion, but I'd prefer the unit to align with what's documented in the description.

_updateCounter = _meter.CreateCounter<long>(UpdateCounterName, "{update}", "The number of user updates.");
_deleteCounter = _meter.CreateCounter<long>(DeleteCounterName, "{delete}", "The number of users deleted.");
_checkPasswordCounter = _meter.CreateCounter<long>(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<long>(VerifyTokenCounterName, "{count}", "The number of token verification attempts.");
_generateTokenCounter = _meter.CreateCounter<long>(GenerateTokenCounterName, "{count}", "The number of token generation attempts.");
}

internal void CreateUser(string userType, IdentityResult? result, Exception? exception = null)
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);
}
Expand All @@ -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);

Expand Down Expand Up @@ -169,7 +169,7 @@ private static string GetTokenPurpose(string purpose)
"EmailConfirmation" => "email_confirmation",
"ChangeEmail" => "change_email",
"TwoFactor" => "two_factor",
_ => "_UNKNOWN"
_ => "_OTHER"
};
}

Expand All @@ -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);
}
}

Expand All @@ -204,7 +205,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"
};
}

Expand Down Expand Up @@ -244,7 +245,7 @@ private static string GetUpdateType(UserUpdateType updateType)
UserUpdateType.RedeemTwoFactorRecoveryCode => "redeem_two_factor_recovery_code",
UserUpdateType.SetPasskey => "set_passkey",
UserUpdateType.RemovePasskey => "remove_passkey",
_ => "_UNKNOWN"
_ => "_OTHER"
};
}

Expand Down
10 changes: 5 additions & 5 deletions src/Identity/test/Identity.Test/UserManagerTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,7 @@ public async Task DeleteCallsStore_Failure()
[
KeyValuePair.Create<string, object>("aspnetcore.identity.user_type", "Microsoft.AspNetCore.Identity.Test.PocoUser"),
KeyValuePair.Create<string, object>("aspnetcore.identity.result", "failure"),
KeyValuePair.Create<string, object>("aspnetcore.identity.result_error_code", "ConcurrencyFailure")
KeyValuePair.Create<string, object>("aspnetcore.identity.error_code", "ConcurrencyFailure")
]));
}

Expand Down Expand Up @@ -664,7 +664,7 @@ public async Task CheckPasswordWillRehashPasswordWhenNeeded()
Assert.Collection(checkPassword.GetMeasurementSnapshot(),
m => MetricsHelpers.AssertContainsTags(m.Tags,
[
KeyValuePair.Create<string, object>("aspnetcore.identity.user.password_result", "success_rehash_needed")
KeyValuePair.Create<string, object>("aspnetcore.identity.password_check_result", "success_rehash_needed")
]));
}

Expand Down Expand Up @@ -871,7 +871,7 @@ public async Task CheckPasswordWithNullUserReturnsFalse()
Assert.Collection(checkPassword.GetMeasurementSnapshot(),
m => MetricsHelpers.AssertContainsTags(m.Tags,
[
KeyValuePair.Create<string, object>("aspnetcore.identity.user.password_result", "user_missing")
KeyValuePair.Create<string, object>("aspnetcore.identity.password_check_result", "user_missing")
]));
}

Expand Down Expand Up @@ -952,13 +952,13 @@ await Assert.ThrowsAsync<NotSupportedException>(
Assert.Collection(generateToken.GetMeasurementSnapshot(),
m => MetricsHelpers.AssertContainsTags(m.Tags,
[
KeyValuePair.Create<string, object>("aspnetcore.identity.token_purpose", "_UNKNOWN"),
KeyValuePair.Create<string, object>("aspnetcore.identity.token_purpose", "_OTHER"),
KeyValuePair.Create<string, object>("error.type", "System.NotSupportedException"),
]));
Assert.Collection(verifyToken.GetMeasurementSnapshot(),
m => MetricsHelpers.AssertContainsTags(m.Tags,
[
KeyValuePair.Create<string, object>("aspnetcore.identity.token_purpose", "_UNKNOWN"),
KeyValuePair.Create<string, object>("aspnetcore.identity.token_purpose", "_OTHER"),
KeyValuePair.Create<string, object>("error.type", "System.NotSupportedException"),
]));
}
Expand Down
Loading