Skip to content

Commit 6c69508

Browse files
MackinnonBuckMackinnonBuck
committed
PR feedback for authentication metrics
1 parent b08e436 commit 6c69508

File tree

9 files changed

+438
-108
lines changed

9 files changed

+438
-108
lines changed

‎src/Http/Authentication.Core/src/AuthenticationCoreServiceCollectionExtensions.cs‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ public static IServiceCollection AddAuthenticationCore(this IServiceCollection s
2121
ArgumentNullException.ThrowIfNull(services);
2222

2323
services.AddMetrics();
24-
services.TryAddScoped<IAuthenticationService, AuthenticationService>();
24+
services.TryAddScoped<IAuthenticationService, AuthenticationServiceImpl>();
2525
services.TryAddSingleton<IClaimsTransformation, NoopClaimsTransformation>(); // Can be replaced with scoped ones that use DbContext
2626
services.TryAddScoped<IAuthenticationHandlerProvider, AuthenticationHandlerProvider>();
2727
services.TryAddSingleton<IAuthenticationSchemeProvider, AuthenticationSchemeProvider>();

‎src/Http/Authentication.Core/src/AuthenticationMetrics.cs‎

Lines changed: 174 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,8 @@
33

44
using System.Diagnostics;
55
using System.Diagnostics.Metrics;
6+
using System.Runtime.CompilerServices;
7+
using Microsoft.AspNetCore.Http;
68

79
namespace Microsoft.AspNetCore.Authentication;
810

@@ -11,7 +13,7 @@ internal sealed class AuthenticationMetrics : IDisposable
1113
public const string MeterName = "Microsoft.AspNetCore.Authentication";
1214

1315
private readonly Meter _meter;
14-
private readonly Counter<long> _authenticatedRequestCount;
16+
private readonly Histogram<double> _authenticatedRequestDuration;
1517
private readonly Counter<long> _challengeCount;
1618
private readonly Counter<long> _forbidCount;
1719
private readonly Counter<long> _signInCount;
@@ -21,10 +23,11 @@ public AuthenticationMetrics(IMeterFactory meterFactory)
2123
{
2224
_meter = meterFactory.Create(MeterName);
2325

24-
_authenticatedRequestCount = _meter.CreateCounter<long>(
25-
"aspnetcore.authentication.authenticated_requests",
26+
_authenticatedRequestDuration = _meter.CreateHistogram<double>(
27+
"aspnetcore.authentication.request.duration",
2628
unit: "{request}",
27-
description: "The total number of authenticated requests");
29+
description: "The total number of requests for which authentication was attempted",
30+
advice: new() { HistogramBucketBoundaries = MetricsConstants.ShortSecondsBucketBoundaries });
2831

2932
_challengeCount = _meter.CreateCounter<long>(
3033
"aspnetcore.authentication.challenges",
@@ -47,57 +50,202 @@ public AuthenticationMetrics(IMeterFactory meterFactory)
4750
description: "The total number of times a scheme is signed out");
4851
}
4952

50-
public void AuthenticatedRequest(string scheme, AuthenticateResult result)
53+
public void AuthenticatedRequestSucceeded(string? scheme, AuthenticateResult result, long startTimestamp, long currentTimestamp)
5154
{
52-
if (_authenticatedRequestCount.Enabled)
55+
if (_authenticatedRequestDuration.Enabled)
5356
{
54-
var resultTagValue = result switch
55-
{
56-
{ Succeeded: true } => "success",
57-
{ Failure: not null } => "failure",
58-
{ None: true } => "none",
59-
_ => throw new UnreachableException($"Could not determine the result state of the {nameof(AuthenticateResult)}"),
60-
};
61-
62-
_authenticatedRequestCount.Add(1, [
63-
new("aspnetcore.authentication.scheme", scheme),
64-
new("aspnetcore.authentication.result", resultTagValue),
65-
]);
57+
AuthenticatedRequestSucceededCore(scheme, result, startTimestamp, currentTimestamp);
6658
}
6759
}
6860

69-
public void Challenge(string scheme)
61+
[MethodImpl(MethodImplOptions.NoInlining)]
62+
private void AuthenticatedRequestSucceededCore(string? scheme, AuthenticateResult result, long startTimestamp, long currentTimestamp)
63+
{
64+
var tags = new TagList();
65+
TryAddSchemeTag(ref tags, scheme);
66+
67+
var resultTagValue = result switch
68+
{
69+
{ None: true } => "none",
70+
{ Succeeded: true } => "success",
71+
{ Failure: not null } => "failure",
72+
_ => "_OTHER", // _OTHER is commonly used fallback for an extra or unexpected value. Shouldn't reach here.
73+
};
74+
tags.Add("aspnetcore.authentication.result", resultTagValue);
75+
76+
var duration = Stopwatch.GetElapsedTime(startTimestamp, currentTimestamp);
77+
_authenticatedRequestDuration.Record(duration.TotalSeconds, tags);
78+
}
79+
80+
public void AuthenticatedRequestFailed(string? scheme, Exception exception, long startTimestamp, long currentTimestamp)
81+
{
82+
if (_authenticatedRequestDuration.Enabled)
83+
{
84+
AuthenticatedRequestFailedCore(scheme, exception, startTimestamp, currentTimestamp);
85+
}
86+
}
87+
88+
[MethodImpl(MethodImplOptions.NoInlining)]
89+
private void AuthenticatedRequestFailedCore(string? scheme, Exception exception, long startTimestamp, long currentTimestamp)
90+
{
91+
var tags = new TagList();
92+
TryAddSchemeTag(ref tags, scheme);
93+
AddErrorTag(ref tags, exception);
94+
95+
var duration = Stopwatch.GetElapsedTime(startTimestamp, currentTimestamp);
96+
_authenticatedRequestDuration.Record(duration.TotalSeconds, tags);
97+
}
98+
99+
public void ChallengeSucceeded(string? scheme)
100+
{
101+
if (_challengeCount.Enabled)
102+
{
103+
ChallengeSucceededCore(scheme);
104+
}
105+
}
106+
107+
[MethodImpl(MethodImplOptions.NoInlining)]
108+
private void ChallengeSucceededCore(string? scheme)
109+
{
110+
var tags = new TagList();
111+
TryAddSchemeTag(ref tags, scheme);
112+
113+
_challengeCount.Add(1, tags);
114+
}
115+
116+
public void ChallengeFailed(string? scheme, Exception exception)
70117
{
71118
if (_challengeCount.Enabled)
72119
{
73-
_challengeCount.Add(1, [new("aspnetcore.authentication.scheme", scheme)]);
120+
ChallengeFailedCore(scheme, exception);
74121
}
75122
}
76123

77-
public void Forbid(string scheme)
124+
[MethodImpl(MethodImplOptions.NoInlining)]
125+
private void ChallengeFailedCore(string? scheme, Exception exception)
126+
{
127+
var tags = new TagList();
128+
TryAddSchemeTag(ref tags, scheme);
129+
AddErrorTag(ref tags, exception);
130+
131+
_challengeCount.Add(1, tags);
132+
}
133+
134+
public void ForbidSucceeded(string? scheme)
78135
{
79136
if (_forbidCount.Enabled)
80137
{
81-
_forbidCount.Add(1, [new("aspnetcore.authentication.scheme", scheme)]);
138+
ForbidSucceededCore(scheme);
82139
}
83140
}
84141

85-
public void SignIn(string scheme)
142+
[MethodImpl(MethodImplOptions.NoInlining)]
143+
private void ForbidSucceededCore(string? scheme)
144+
{
145+
var tags = new TagList();
146+
TryAddSchemeTag(ref tags, scheme);
147+
_forbidCount.Add(1, tags);
148+
}
149+
150+
public void ForbidFailed(string? scheme, Exception exception)
151+
{
152+
if (_forbidCount.Enabled)
153+
{
154+
ForbidFailedCore(scheme, exception);
155+
}
156+
}
157+
158+
[MethodImpl(MethodImplOptions.NoInlining)]
159+
private void ForbidFailedCore(string? scheme, Exception exception)
160+
{
161+
var tags = new TagList();
162+
TryAddSchemeTag(ref tags, scheme);
163+
AddErrorTag(ref tags, exception);
164+
165+
_forbidCount.Add(1, tags);
166+
}
167+
168+
public void SignInSucceeded(string? scheme)
86169
{
87170
if (_signInCount.Enabled)
88171
{
89-
_signInCount.Add(1, [new("aspnetcore.authentication.scheme", scheme)]);
172+
SignInSucceededCore(scheme);
90173
}
91174
}
92175

93-
public void SignOut(string scheme)
176+
[MethodImpl(MethodImplOptions.NoInlining)]
177+
private void SignInSucceededCore(string? scheme)
178+
{
179+
var tags = new TagList();
180+
TryAddSchemeTag(ref tags, scheme);
181+
_signInCount.Add(1, tags);
182+
}
183+
184+
public void SignInFailed(string? scheme, Exception exception)
185+
{
186+
if (_signInCount.Enabled)
187+
{
188+
SignInFailedCore(scheme, exception);
189+
}
190+
}
191+
192+
[MethodImpl(MethodImplOptions.NoInlining)]
193+
private void SignInFailedCore(string? scheme, Exception exception)
194+
{
195+
var tags = new TagList();
196+
TryAddSchemeTag(ref tags, scheme);
197+
AddErrorTag(ref tags, exception);
198+
199+
_signInCount.Add(1, tags);
200+
}
201+
202+
public void SignOutSucceeded(string? scheme)
94203
{
95204
if (_signOutCount.Enabled)
96205
{
97-
_signOutCount.Add(1, [new("aspnetcore.authentication.scheme", scheme)]);
206+
SignOutSucceededCore(scheme);
98207
}
99208
}
100209

210+
[MethodImpl(MethodImplOptions.NoInlining)]
211+
private void SignOutSucceededCore(string? scheme)
212+
{
213+
var tags = new TagList();
214+
TryAddSchemeTag(ref tags, scheme);
215+
_signOutCount.Add(1, tags);
216+
}
217+
218+
public void SignOutFailed(string? scheme, Exception exception)
219+
{
220+
if (_signOutCount.Enabled)
221+
{
222+
SignOutFailedCore(scheme, exception);
223+
}
224+
}
225+
226+
[MethodImpl(MethodImplOptions.NoInlining)]
227+
private void SignOutFailedCore(string? scheme, Exception exception)
228+
{
229+
var tags = new TagList();
230+
TryAddSchemeTag(ref tags, scheme);
231+
AddErrorTag(ref tags, exception);
232+
233+
_signOutCount.Add(1, tags);
234+
}
235+
236+
private static void TryAddSchemeTag(ref TagList tags, string? scheme)
237+
{
238+
if (scheme is not null)
239+
{
240+
tags.Add("aspnetcore.authentication.scheme", scheme);
241+
}
242+
}
243+
244+
private static void AddErrorTag(ref TagList tags, Exception exception)
245+
{
246+
tags.Add("error.type", exception.GetType().FullName);
247+
}
248+
101249
public void Dispose()
102250
{
103251
_meter.Dispose();

0 commit comments

Comments
 (0)