diff --git a/src/client/Microsoft.Identity.Client/Internal/Requests/RequestBase.cs b/src/client/Microsoft.Identity.Client/Internal/Requests/RequestBase.cs index c8269bb7cd..5f27a8063c 100644 --- a/src/client/Microsoft.Identity.Client/Internal/Requests/RequestBase.cs +++ b/src/client/Microsoft.Identity.Client/Internal/Requests/RequestBase.cs @@ -106,7 +106,9 @@ public async Task RunAsync(CancellationToken cancellationT } AuthenticationRequestParameters.RequestContext.Logger.ErrorPii(ex); - LogFailureTelemetryToOtel(ex.ErrorCode, apiEvent, apiEvent.CacheInfo); + LogFailureTelemetryToOtel( + ex.ErrorCode, apiEvent, apiEvent.CacheInfo, + (ex as MsalServiceException)?.ErrorCodes?.FirstOrDefault()); throw; } catch (Exception ex) @@ -133,7 +135,7 @@ private void LogSuccessTelemetryToOtel(AuthenticationResult authenticationResult AuthenticationRequestParameters.RequestContext.Logger); } - private void LogFailureTelemetryToOtel(string errorCodeToLog, ApiEvent apiEvent, CacheRefreshReason cacheRefreshReason) + private void LogFailureTelemetryToOtel(string errorCodeToLog, ApiEvent apiEvent, CacheRefreshReason cacheRefreshReason, string rawStsErrorCode = null) { // Log metrics ServiceBundle.PlatformProxy.OtelInstrumentation.LogFailureMetrics( @@ -143,7 +145,8 @@ private void LogFailureTelemetryToOtel(string errorCodeToLog, ApiEvent apiEvent, apiEvent.CallerSdkApiId, apiEvent.CallerSdkVersion, cacheRefreshReason, - apiEvent.TokenType); + apiEvent.TokenType, + rawStsErrorCode); } private Tuple ParseScopesForTelemetry() diff --git a/src/client/Microsoft.Identity.Client/Internal/Requests/SilentRequestHelper.cs b/src/client/Microsoft.Identity.Client/Internal/Requests/SilentRequestHelper.cs index b18eb75a29..a5f43bf85a 100644 --- a/src/client/Microsoft.Identity.Client/Internal/Requests/SilentRequestHelper.cs +++ b/src/client/Microsoft.Identity.Client/Internal/Requests/SilentRequestHelper.cs @@ -3,6 +3,7 @@ using System; using System.Collections.Generic; +using System.Linq; using System.Threading; using System.Threading.Tasks; using Microsoft.Identity.Client.Cache.Items; @@ -124,7 +125,8 @@ internal static void ProcessFetchInBackground( callerSdkId, callerSdkVersion, CacheRefreshReason.ProactivelyRefreshed, - apiEvent.TokenType); + apiEvent.TokenType, + ex.ErrorCodes?.FirstOrDefault()); } catch (OperationCanceledException ex) { diff --git a/src/client/Microsoft.Identity.Client/Platforms/Features/OpenTelemetry/OtelInstrumentation.cs b/src/client/Microsoft.Identity.Client/Platforms/Features/OpenTelemetry/OtelInstrumentation.cs index 51f94757e6..ac4a68e2ee 100644 --- a/src/client/Microsoft.Identity.Client/Platforms/Features/OpenTelemetry/OtelInstrumentation.cs +++ b/src/client/Microsoft.Identity.Client/Platforms/Features/OpenTelemetry/OtelInstrumentation.cs @@ -1,6 +1,7 @@ // Copyright (c) Microsoft Corporation. All rights reserved. // Licensed under the MIT License. using System; +using System.Diagnostics; using System.Diagnostics.Metrics; using Microsoft.Identity.Client.Cache; using Microsoft.Identity.Client.Core; @@ -204,19 +205,27 @@ public void LogFailureMetrics(string platform, string callerSdkId, string callerSdkVersion, CacheRefreshReason cacheRefreshReason, - int tokenType) + int tokenType, + string rawStsErrorCode = null) { - if (s_failureCounter.Value.Enabled) + if (!s_failureCounter.Value.Enabled) + return; + + var tags = new TagList { - s_failureCounter.Value.Add(1, - new(TelemetryConstants.MsalVersion, MsalIdHelper.GetMsalVersion()), - new(TelemetryConstants.Platform, platform), - new(TelemetryConstants.ErrorCode, errorCode), - new(TelemetryConstants.ApiId, apiId), - new(TelemetryConstants.CallerSdkId, callerSdkId ?? string.Empty + "," + callerSdkVersion ?? string.Empty), - new(TelemetryConstants.CacheRefreshReason, cacheRefreshReason), - new(TelemetryConstants.TokenType, tokenType)); - } + { TelemetryConstants.MsalVersion, MsalIdHelper.GetMsalVersion() }, + { TelemetryConstants.Platform, platform }, + { TelemetryConstants.ErrorCode, errorCode }, + { TelemetryConstants.ApiId, apiId }, + { TelemetryConstants.CallerSdkId, callerSdkId ?? string.Empty + "," + callerSdkVersion ?? string.Empty }, + { TelemetryConstants.CacheRefreshReason, cacheRefreshReason }, + { TelemetryConstants.TokenType, tokenType } + }; + + if (!string.IsNullOrEmpty(rawStsErrorCode)) + tags.Add(TelemetryConstants.RawStsErrorCode, rawStsErrorCode); + + s_failureCounter.Value.Add(1, in tags); } } } diff --git a/src/client/Microsoft.Identity.Client/TelemetryCore/OpenTelemetry/IOtelInstrumentation.cs b/src/client/Microsoft.Identity.Client/TelemetryCore/OpenTelemetry/IOtelInstrumentation.cs index 7802ab4211..b4885d57f5 100644 --- a/src/client/Microsoft.Identity.Client/TelemetryCore/OpenTelemetry/IOtelInstrumentation.cs +++ b/src/client/Microsoft.Identity.Client/TelemetryCore/OpenTelemetry/IOtelInstrumentation.cs @@ -31,12 +31,13 @@ internal void IncrementSuccessCounter(string platform, ILoggerAdapter logger, int TokenType); - internal void LogFailureMetrics(string platform, - string errorCode, + internal void LogFailureMetrics(string platform, + string errorCode, ApiEvent.ApiIds apiId, string callerSdkId, string callerSdkVersion, CacheRefreshReason cacheRefreshReason, - int tokenType); + int tokenType, + string rawStsErrorCode = null); } } diff --git a/src/client/Microsoft.Identity.Client/TelemetryCore/OpenTelemetry/NullOtelInstrumentation.cs b/src/client/Microsoft.Identity.Client/TelemetryCore/OpenTelemetry/NullOtelInstrumentation.cs index 7fcdbbcf9c..eb23813f3c 100644 --- a/src/client/Microsoft.Identity.Client/TelemetryCore/OpenTelemetry/NullOtelInstrumentation.cs +++ b/src/client/Microsoft.Identity.Client/TelemetryCore/OpenTelemetry/NullOtelInstrumentation.cs @@ -27,13 +27,14 @@ public void LogSuccessMetrics( // No op } - public void LogFailureMetrics(string platform, - string errorCode, - ApiEvent.ApiIds apiId, + public void LogFailureMetrics(string platform, + string errorCode, + ApiEvent.ApiIds apiId, string callerSdkId, string callerSdkVersion, CacheRefreshReason cacheRefreshReason, - int tokenType) + int tokenType, + string rawStsErrorCode = null) { // No op } diff --git a/src/client/Microsoft.Identity.Client/TelemetryCore/TelemetryConstants.cs b/src/client/Microsoft.Identity.Client/TelemetryCore/TelemetryConstants.cs index bed6e7db55..444f53b09d 100644 --- a/src/client/Microsoft.Identity.Client/TelemetryCore/TelemetryConstants.cs +++ b/src/client/Microsoft.Identity.Client/TelemetryCore/TelemetryConstants.cs @@ -26,7 +26,7 @@ internal static class TelemetryConstants public const string CacheInfoTelemetry = "CacheInfoTelemetry"; public const string CacheRefreshReason = "CacheRefreshReason"; public const string ErrorCode = "ErrorCode"; - public const string StsErrorCode = "StsErrorCode"; + public const string RawStsErrorCode = "RawStsErrorCode"; public const string ErrorMessage = "ErrorMessage"; public const string Duration = "Duration"; public const string DurationInUs = "DurationInUs"; diff --git a/tests/Microsoft.Identity.Test.Unit/TelemetryTests/OTelInstrumentationTests.cs b/tests/Microsoft.Identity.Test.Unit/TelemetryTests/OTelInstrumentationTests.cs index 131869ac44..f074df9f80 100644 --- a/tests/Microsoft.Identity.Test.Unit/TelemetryTests/OTelInstrumentationTests.cs +++ b/tests/Microsoft.Identity.Test.Unit/TelemetryTests/OTelInstrumentationTests.cs @@ -415,7 +415,74 @@ private void CreateApplication() .BuildConcrete(); } - private void VerifyMetrics(int expectedMetricCount, List exportedMetrics, + [TestMethod] + public async Task MsalFailure_ServiceException_RawStsErrorCodeTag_IncludedAsync() + { + using (_harness = CreateTestHarness()) + { + CreateApplication(); + + _harness.HttpManager.AddInstanceDiscoveryMockHandler(); + _harness.HttpManager.AddTokenResponse(TokenResponseType.InvalidClient); + + MsalServiceException ex = await AssertException.TaskThrowsAsync( + () => _cca.AcquireTokenForClient(TestConstants.s_scopeForAnotherResource) + .WithExtraQueryParameters(extraQueryParams) + .WithTenantId(TestConstants.Utid) + .ExecuteAsync(CancellationToken.None)).ConfigureAwait(false); + + 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]); + } + } + } + + [TestMethod] + public async Task MsalFailure_ClientException_RawStsErrorCodeTag_NotIncludedAsync() + { + using (_harness = CreateTestHarness()) + { + CreateApplication(); + + // Null scope triggers MsalClientException before any HTTP call — no ErrorCodes + MsalClientException ex = await AssertException.TaskThrowsAsync( + () => _cca.AcquireTokenForClient(null) + .WithExtraQueryParameters(extraQueryParams) + .WithTenantId(TestConstants.Utid) + .ExecuteAsync(CancellationToken.None)).ConfigureAwait(false); + + Assert.IsNotNull(ex); + + s_meterProvider.ForceFlush(); + + var failureMetric = _exportedMetrics.First(m => m.Name == "MsalFailure"); + foreach (var metricPoint in failureMetric.GetMetricPoints()) + { + var tags = GetTagDictionary(metricPoint.Tags); + Assert.IsFalse(tags.ContainsKey(TelemetryConstants.RawStsErrorCode), + "RawStsErrorCode tag should not be present for non-service exceptions."); + } + } + } + + private static IDictionary GetTagDictionary(ReadOnlyTagCollection tags) + { + var dict = new Dictionary(); + foreach (var tag in tags) + dict[tag.Key] = tag.Value; + return dict; + } + + private void VerifyMetrics(int expectedMetricCount, List exportedMetrics, long expectedSuccessfulRequests, long expectedFailedRequests) { Assert.HasCount(expectedMetricCount, exportedMetrics, "Count of metrics recorded is not as expected."); @@ -467,7 +534,10 @@ private void VerifyMetrics(int expectedMetricCount, List exportedMetrics foreach (var metricPoint in exportedItem.GetMetricPoints()) { totalFailedRequests += metricPoint.GetSumLong(); - AssertTags(metricPoint.Tags, expectedTags, true); + var pointExpectedTags = new List(expectedTags); + if (GetTagDictionary(metricPoint.Tags).ContainsKey(TelemetryConstants.RawStsErrorCode)) + pointExpectedTags.Add(TelemetryConstants.RawStsErrorCode); + AssertTags(metricPoint.Tags, pointExpectedTags, true); } Assert.AreEqual(expectedFailedRequests, totalFailedRequests);