-
Notifications
You must be signed in to change notification settings - Fork 403
[Proposal] Add opt‑in MSAL.NET metrics enrichment #5877
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -204,7 +204,29 @@ public static AbstractAcquireTokenParameterBuilder<T> WithFmiPathForClientAssert | |
| } | ||
|
|
||
| /// <summary> | ||
| /// Specifies extra claims to be included in the client assertion. | ||
| /// Registers a callback that is invoked just before OpenTelemetry metrics are recorded for this | ||
| /// token acquisition call. Use this to append tags to every metric emitted for the request. | ||
| /// </summary> | ||
| /// <typeparam name="T"></typeparam> | ||
| /// <param name="builder">The builder to chain options to.</param> | ||
| /// <param name="otelTagsEnricher"> | ||
| /// A delegate that receives the <see cref="TokenAcquisitionResult"/> (carrying either the | ||
| /// successful <see cref="AuthenticationResult"/> or the failure <see cref="Exception"/>) | ||
| /// and a mutable tag list. Add <see cref="KeyValuePair{String, Object}"/> entries to the list | ||
| /// to include them as dimensions on all metrics emitted for this call. | ||
| /// </param> | ||
| /// <returns>The builder to chain other options to.</returns> | ||
| public static AbstractAcquireTokenParameterBuilder<T> WithOtelTagsEnricher<T>( | ||
| this AbstractAcquireTokenParameterBuilder<T> builder, | ||
| Action<TokenAcquisitionResult, IList<KeyValuePair<string, object>>> otelTagsEnricher) | ||
| where T : AbstractAcquireTokenParameterBuilder<T> | ||
| { | ||
| builder.CommonParameters.OtelTagsEnricher = otelTagsEnricher; | ||
| return builder; | ||
| } | ||
|
Comment on lines
+219
to
+226
|
||
|
|
||
| /// <summary> | ||
| /// Specifies extra claims to be included in the client assertion. | ||
| /// These claims will be merged with default claims when the client assertion is generated. | ||
| /// This lets higher level APIs like Microsoft.Identity.Web provide additional claims for the client assertion. | ||
| /// Important: tokens are associated with the extra client assertion claims, which impacts cache lookups. | ||
|
|
||
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
|
|
@@ -83,27 +83,32 @@ internal static bool NeedsRefresh(MsalAccessTokenCacheItem oldAccessToken, out D | |||||||||||||||||||||||||||||||
| internal static void ProcessFetchInBackground( | ||||||||||||||||||||||||||||||||
| MsalAccessTokenCacheItem oldAccessToken, | ||||||||||||||||||||||||||||||||
| Func<Task<AuthenticationResult>> fetchAction, | ||||||||||||||||||||||||||||||||
| ILoggerAdapter logger, | ||||||||||||||||||||||||||||||||
| IServiceBundle serviceBundle, | ||||||||||||||||||||||||||||||||
| ApiEvent apiEvent, | ||||||||||||||||||||||||||||||||
| string callerSdkId, | ||||||||||||||||||||||||||||||||
| string callerSdkVersion) | ||||||||||||||||||||||||||||||||
| ILoggerAdapter logger, | ||||||||||||||||||||||||||||||||
| IServiceBundle serviceBundle, | ||||||||||||||||||||||||||||||||
| ApiEvent apiEvent, | ||||||||||||||||||||||||||||||||
| string callerSdkId, | ||||||||||||||||||||||||||||||||
| string callerSdkVersion, | ||||||||||||||||||||||||||||||||
| Action<TokenAcquisitionResult, IList<KeyValuePair<string, object>>> tagsEnricher = null) | ||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
| _ = Task.Run(async () => | ||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
| try | ||||||||||||||||||||||||||||||||
| { | ||||||||||||||||||||||||||||||||
| var authResult = await fetchAction().ConfigureAwait(false); | ||||||||||||||||||||||||||||||||
| var context = new TokenAcquisitionResult { AuthenticationResult = authResult }; | ||||||||||||||||||||||||||||||||
| var extraTags = new List<KeyValuePair<string, object>>(); | ||||||||||||||||||||||||||||||||
| tagsEnricher?.Invoke(context, extraTags); | ||||||||||||||||||||||||||||||||
|
Comment on lines
+99
to
+100
|
||||||||||||||||||||||||||||||||
| var extraTags = new List<KeyValuePair<string, object>>(); | |
| tagsEnricher?.Invoke(context, extraTags); | |
| IList<KeyValuePair<string, object>> extraTags = null; | |
| if (tagsEnricher != null) | |
| { | |
| extraTags = new List<KeyValuePair<string, object>>(); | |
| try | |
| { | |
| tagsEnricher(context, extraTags); | |
| } | |
| catch (Exception ex) | |
| { | |
| logger.ErrorPiiWithPrefix(ex, "Proactive refresh tags enricher callback failed."); | |
| } | |
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The XML docs for this enricher don’t call out important constraints: avoid adding secrets/PII (e.g., access tokens) to metric tags, keep tag cardinality low, and ensure the delegate is fast and non-throwing (or clarify that MSAL will ignore exceptions). Adding these warnings would help prevent accidental data exposure and high-cardinality metric explosions.