Add raw STS error code to MsalFailure metric#5919
Open
ssmelov wants to merge 9 commits into
Open
Conversation
ssmelov
changed the title
Contributor
There was a problem hiding this comment.
Pull request overview
Adds an opt-in OpenTelemetry metric tag to surface the raw ESTS error_codes value on the MsalFailure counter, enabling deeper diagnostics while avoiding default high-cardinality metrics.
Changes:
- Introduces
WithRawStsErrorCodeTelemetry()/EnableRawStsErrorCodeTelemetryconfiguration to control emission of a newRawStsErrorCodemetric tag. - Extends OpenTelemetry failure logging plumbing (
IOtelInstrumentation+ implementations) to accept and emit the raw STS error code when enabled. - Adds unit tests validating tag inclusion/exclusion behavior across service vs. client exceptions.
Reviewed changes
Copilot reviewed 16 out of 16 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| tests/Microsoft.Identity.Test.Unit/TelemetryTests/OTelInstrumentationTests.cs | Adds tests verifying RawStsErrorCode tag behavior under opt-in and exception-type conditions. |
| src/client/Microsoft.Identity.Client/TelemetryCore/TelemetryConstants.cs | Defines the RawStsErrorCode tag key constant. |
| src/client/Microsoft.Identity.Client/TelemetryCore/OpenTelemetry/NullOtelInstrumentation.cs | Updates failure metric method signature to accept optional raw STS error code. |
| src/client/Microsoft.Identity.Client/TelemetryCore/OpenTelemetry/IOtelInstrumentation.cs | Extends failure metric API surface to carry optional raw STS error code. |
| src/client/Microsoft.Identity.Client/PublicApi/netstandard2.0/PublicAPI.Unshipped.txt | Declares newly added public APIs for API compatibility tracking. |
| src/client/Microsoft.Identity.Client/PublicApi/net8.0/PublicAPI.Unshipped.txt | Declares newly added public APIs for API compatibility tracking. |
| src/client/Microsoft.Identity.Client/PublicApi/net8.0-ios/PublicAPI.Unshipped.txt | Declares newly added public APIs for API compatibility tracking. |
| src/client/Microsoft.Identity.Client/PublicApi/net8.0-android/PublicAPI.Unshipped.txt | Declares newly added public APIs for API compatibility tracking. |
| src/client/Microsoft.Identity.Client/PublicApi/net472/PublicAPI.Unshipped.txt | Declares newly added public APIs for API compatibility tracking. |
| src/client/Microsoft.Identity.Client/PublicApi/net462/PublicAPI.Unshipped.txt | Declares newly added public APIs for API compatibility tracking. |
| src/client/Microsoft.Identity.Client/Platforms/Features/OpenTelemetry/OtelInstrumentation.cs | Emits RawStsErrorCode tag when provided and non-empty. |
| src/client/Microsoft.Identity.Client/Internal/Requests/SilentRequestHelper.cs | Passes raw STS error code into failure metrics during proactive refresh when enabled. |
| src/client/Microsoft.Identity.Client/Internal/Requests/RequestBase.cs | Passes raw STS error code into failure metrics on MsalException when enabled. |
| src/client/Microsoft.Identity.Client/AppConfig/IAppConfig.cs | Adds EnableRawStsErrorCodeTelemetry property to expose the opt-in setting. |
| src/client/Microsoft.Identity.Client/AppConfig/BaseAbstractApplicationBuilder.cs | Adds WithRawStsErrorCodeTelemetry() builder method to toggle the feature. |
| src/client/Microsoft.Identity.Client/AppConfig/ApplicationConfiguration.cs | Stores the EnableRawStsErrorCodeTelemetry configuration value. |
ssmelov
changed the title
bgavrilMS
approved these changes
Apr 24, 2026
neha-bhargava
approved these changes
Apr 28, 2026
…https://github.com/ssmelov/microsoft-authentication-library-for-dotnet into feature/add_sts_raw_error_code_to_msalfailure_metric
| // Copyright (c) Microsoft Corporation. All rights reserved. | ||
| // Licensed under the MIT License. | ||
| using System; | ||
| using System.Diagnostics; |
There was a problem hiding this comment.
using System.Diagnostics; appears unused in this file. Consider removing it to avoid unnecessary imports (and potential analyzer noise) since the code only uses types from System.Diagnostics.Metrics.
Suggested change
| using System.Diagnostics; |
Comment on lines
+434
to
+445
| Assert.IsNotNull(ex.ErrorCodes, "ErrorCodes should be populated from IDP response."); | ||
|
|
||
| s_meterProvider.ForceFlush(); | ||
|
|
||
| var failureMetric = _exportedMetrics.First(m => m.Name == "MsalFailure"); | ||
| foreach (var metricPoint in failureMetric.GetMetricPoints()) | ||
| { | ||
| var tags = GetTagDictionary(metricPoint.Tags); | ||
| Assert.IsTrue(tags.ContainsKey(TelemetryConstants.RawStsErrorCode), | ||
| "RawStsErrorCode tag should be present when the IDP response contains error_codes."); | ||
| Assert.AreEqual(ex.ErrorCodes.FirstOrDefault(), tags[TelemetryConstants.RawStsErrorCode]); | ||
| } |
There was a problem hiding this comment.
The test asserts ex.ErrorCodes is non-null, but it can still be empty; in that case FirstOrDefault() returns null and the subsequent assertions will fail in a less direct way. Consider asserting ex.ErrorCodes.Length > 0 (or that ex.ErrorCodes.FirstOrDefault() is non-empty) before validating the metric tag value.
|
|
||
| s_meterProvider.ForceFlush(); | ||
|
|
||
| var failureMetric = _exportedMetrics.First(m => m.Name == "MsalFailure"); |
There was a problem hiding this comment.
Same as above: _exportedMetrics.First(m => m.Name == "MsalFailure") will throw if the metric isn't present, making the test failure harder to interpret. Consider FirstOrDefault + an explicit assertion.
Suggested change
| var failureMetric = _exportedMetrics.First(m => m.Name == "MsalFailure"); | |
| var failureMetric = _exportedMetrics.FirstOrDefault(m => m.Name == "MsalFailure"); | |
| Assert.IsNotNull(failureMetric, "Expected 'MsalFailure' metric to be exported."); |
Member
|
@neha-bhargava - should this get merged now? |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Extended the MsalFailure OpenTelemetry counter with RawStsErrorCode tag that surfaces the raw ESTS error code returned in the error_codes array of the identity provider response.