Skip to content
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

Update Telemetry client with data outlined in spec #3737

Merged
merged 38 commits into from
May 23, 2023
Merged
Show file tree
Hide file tree
Changes from 10 commits
Commits
Show all changes
38 commits
Select commit Hold shift + click to select a range
3190435
Add cache details to token cache notification args
neha-bhargava Oct 6, 2022
2b2d6fd
Change code to make it more readable.
neha-bhargava Oct 7, 2022
4fe6bd7
Address comments
neha-bhargava Oct 7, 2022
169d95c
Address comments
neha-bhargava Oct 7, 2022
969867c
Remove unused imports
neha-bhargava Oct 11, 2022
c233b72
Change from dictionary to class
neha-bhargava Oct 12, 2022
09d2d35
Update comments and CacheUsed enum values
neha-bhargava Oct 12, 2022
c537f87
Merge branch 'main' into nebharg/AddCacheDetailsTelemetry
neha-bhargava Oct 12, 2022
fd6dd23
Removing the enum as this can be inferred from Latencies
neha-bhargava Oct 15, 2022
5449a7d
Merge branch 'nebharg/AddCacheDetailsTelemetry' of https://github.com…
neha-bhargava Oct 15, 2022
bd32bba
Merge remote-tracking branch 'origin/pmaytak/1' into nebharg/AddCache…
Mar 7, 2023
39ed3f0
Adding api event for credential type.
Mar 9, 2023
02dd568
Updating telemetry Data
Mar 10, 2023
e9e9ceb
Adding tests for new telemetry datapoints.
Mar 15, 2023
1b9644f
Adding telemetry cache test.
Mar 15, 2023
057e407
Merge branch 'main' into nebharg/AddCacheDetailsTelemetry
trwalke Mar 15, 2023
8477fc4
Merge remote-tracking branch 'origin/pmaytak/1' into nebharg/AddCache…
Mar 29, 2023
2df0d13
Refactoring
Mar 29, 2023
43ad450
Merge branch 'main' into nebharg/AddCacheDetailsTelemetry
Apr 20, 2023
18f153b
Refactoring
Apr 20, 2023
df4da2f
Adding scopes, resources and error message to telemetry
Apr 21, 2023
d7c655f
Apply suggestions from code review
trwalke Apr 25, 2023
2d1eb42
Merge remote-tracking branch 'origin/main' into nebharg/AddCacheDetai…
Apr 25, 2023
35b5c73
Updating scope parsing logic.
Apr 27, 2023
8a66797
Refactoring
Apr 28, 2023
1a88f21
Updating telemetry data points
May 3, 2023
1823be2
Merge branch 'main' into nebharg/AddCacheDetailsTelemetry
trwalke May 3, 2023
40d62f8
Test update
May 3, 2023
15896d2
Fix typo
May 3, 2023
d64f181
Test Fixes
May 3, 2023
9346d15
Merge branch 'main' into nebharg/AddCacheDetailsTelemetry
trwalke May 3, 2023
24d2bc3
Apply suggestions from code review
trwalke May 16, 2023
4e1bf52
Adding additional test
May 17, 2023
657a8cc
Merge branch 'main' into nebharg/AddCacheDetailsTelemetry
May 23, 2023
40a22d0
test fix
May 23, 2023
0be7992
Test Fix
May 23, 2023
cb4667f
Test Update
May 23, 2023
059b6e4
Merge branch 'main' into nebharg/AddCacheDetailsTelemetry
gladjohn May 23, 2023
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
Original file line number Diff line number Diff line change
Expand Up @@ -341,8 +341,6 @@ public ConfidentialClientApplicationBuilder WithGenericAuthority(string authorit
/// <returns>The builder to chain the .With methods</returns>
public ConfidentialClientApplicationBuilder WithTelemetryClient(params ITelemetryClient[] telemetryClients)
{
ValidateUseOfExperimentalFeature("ITelemetryClient");
trwalke marked this conversation as resolved.
Show resolved Hide resolved

if (telemetryClients == null)
{
throw new ArgumentNullException(nameof(telemetryClients));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,21 +10,26 @@
namespace Microsoft.Identity.Client.Cache
{
/// <summary>
/// Identifies the type of cache used when accessing the cache.
/// Identifies the type of cache used when accessing the cache. Cache implementations must provide this.
trwalke marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
public enum CacheTypeUsed
public enum CacheLevel
pmaytak marked this conversation as resolved.
Show resolved Hide resolved
{
/// <summary>
/// Specifies that no cache type is used
/// Specifies that the cache level used is None.
/// Token was retrieved from ESTS
/// </summary>
None = 0,
/// <summary>
/// Specifies if the L1 cache is used
/// Specifies that the cache level used is unknown.
/// The custom token cache which doesn't relay the info about which cache was hit back to MSAL.
trwalke marked this conversation as resolved.
Show resolved Hide resolved
/// </summary>
L1Cache = 1,
Unknown = 1,
/// <summary>
/// Specifies if the L2 cache is used
/// Specifies if the L1 cache is used.
/// </summary>
L2Cache = 2
L1Cache = 2,
/// <summary>
/// Specifies if the L2 cache is used.
L2Cache = 3
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
using Microsoft.Identity.Client.Internal;
using Microsoft.Identity.Client.Internal.Requests;
using Microsoft.Identity.Client.OAuth2;
using Microsoft.Identity.Client.TelemetryCore.TelemetryClient;

namespace Microsoft.Identity.Client.Cache
{
Expand Down Expand Up @@ -106,6 +107,7 @@ private async Task RefreshCacheForReadOperationsAsync()
await TokenCacheInternal.Semaphore.WaitAsync(_requestParams.RequestContext.UserCancellationToken).ConfigureAwait(false);
_requestParams.RequestContext.Logger.Verbose(()=>"[Cache Session Manager] Entered cache semaphore");

TelemetryData telemetryData = new TelemetryData();
Stopwatch stopwatch = new Stopwatch();
try
{
Expand All @@ -130,7 +132,7 @@ private async Task RefreshCacheForReadOperationsAsync()
requestTenantId: _requestParams.AuthorityManager.OriginalAuthority.TenantId,
identityLogger: _requestParams.RequestContext.Logger.IdentityLogger,
piiLoggingEnabled: _requestParams.RequestContext.Logger.PiiLoggingEnabled,
telemetryDatapoints: RequestContext.TelemetryDatapoints);
telemetryData: telemetryData);

stopwatch.Start();
await TokenCacheInternal.OnBeforeAccessAsync(args).ConfigureAwait(false);
Expand All @@ -157,7 +159,7 @@ private async Task RefreshCacheForReadOperationsAsync()
requestTenantId: _requestParams.AuthorityManager.OriginalAuthority.TenantId,
identityLogger: _requestParams.RequestContext.Logger.IdentityLogger,
piiLoggingEnabled: _requestParams.RequestContext.Logger.PiiLoggingEnabled,
telemetryDatapoints: RequestContext.TelemetryDatapoints);
telemetryData: telemetryData);

await TokenCacheInternal.OnAfterAccessAsync(args).ConfigureAwait(false);
RequestContext.ApiEvent.DurationInCacheInMs += stopwatch.ElapsedMilliseconds;
Expand All @@ -172,6 +174,7 @@ private async Task RefreshCacheForReadOperationsAsync()
{
TokenCacheInternal.Semaphore.Release();
_requestParams.RequestContext.Logger.Verbose(()=>"[Cache Session Manager] Released cache semaphore");
RequestContext.ApiEvent.CacheLevel = telemetryData.CacheLevel;
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ internal class CertificateAndClaimsClientCredential : IClientCredential
private readonly string _base64EncodedThumbprint; // x5t
public X509Certificate2 Certificate { get; }

public AssertionType TelemetryAssertionType => AssertionType.CertificateWithoutSni;
public AssertionType AssertionType => AssertionType.CertificateWithoutSni;

public CertificateAndClaimsClientCredential(X509Certificate2 certificate, IDictionary<string, string> claimsToSign, bool appendDefaultClaims)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ namespace Microsoft.Identity.Client.Internal.ClientCredential
{
internal interface IClientCredential
{
AssertionType TelemetryAssertionType { get; }
AssertionType AssertionType { get; }
trwalke marked this conversation as resolved.
Show resolved Hide resolved

Task AddConfidentialClientParametersAsync(
OAuth2Client oAuth2Client,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ internal class SecretStringClientCredential : IClientCredential
{
internal string Secret { get; }

public AssertionType TelemetryAssertionType => AssertionType.Secret;
public AssertionType AssertionType => AssertionType.Secret;

public SecretStringClientCredential(string secret)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ internal class SignedAssertionClientCredential : IClientCredential
{
private readonly string _signedAssertion;

public AssertionType TelemetryAssertionType => AssertionType.ClientAssertion;
public AssertionType AssertionType => AssertionType.ClientAssertion;

public SignedAssertionClientCredential(string signedAssertion)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,7 @@ internal class SignedAssertionDelegateClientCredential : IClientCredential
{
internal Func<CancellationToken, Task<string>> _signedAssertionDelegate { get; }
internal Func<AssertionRequestOptions, Task<string>> _signedAssertionWithInfoDelegate { get; }
public AssertionType TelemetryAssertionType => AssertionType.ClientAssertion;
public AssertionType AssertionType => AssertionType.ClientAssertion;

[System.ComponentModel.EditorBrowsable(System.ComponentModel.EditorBrowsableState.Never)]
public SignedAssertionDelegateClientCredential(Func<CancellationToken, Task<string>> signedAssertionDelegate)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,6 @@ internal class RequestContext

public CancellationToken UserCancellationToken { get; }

public TelemetryDatapoints TelemetryDatapoints { get; } = new TelemetryDatapoints();

public RequestContext(IServiceBundle serviceBundle, Guid correlationId, CancellationToken cancellationToken = default)
{
ServiceBundle = serviceBundle ?? throw new ArgumentNullException(nameof(serviceBundle));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,21 +96,22 @@ public async Task<AuthenticationResult> RunAsync(CancellationToken cancellationT
LogReturnedToken(authenticationResult);
UpdateTelemetry(sw, apiEvent, authenticationResult);
LogMetricsFromAuthResult(authenticationResult, AuthenticationRequestParameters.RequestContext.Logger);
LogSuccessfulTelemetryToClient(authenticationResult, AuthenticationRequestParameters.RequestContext.TelemetryDatapoints, telemetryEventDetails, telemetryClients);
LogSuccessfulTelemetryToClient(authenticationResult, telemetryEventDetails, telemetryClients);

return authenticationResult;
}
catch (MsalException ex)
{
apiEvent.ApiErrorCode = ex.ErrorCode;
AuthenticationRequestParameters.RequestContext.Logger.ErrorPii(ex);
LogErrorTelemetryToClient(ex.ErrorCode, telemetryEventDetails, telemetryClients);
LogMsalErrorTelemetryToClient(ex, telemetryEventDetails, telemetryClients);
trwalke marked this conversation as resolved.
Show resolved Hide resolved
throw;
}
catch (Exception ex)
{
apiEvent.ApiErrorCode = ex.GetType().Name;
AuthenticationRequestParameters.RequestContext.Logger.ErrorPii(ex);
LogErrorTelemetryToClient(ex, telemetryEventDetails, telemetryClients);
throw;
}
finally
Expand All @@ -120,16 +121,38 @@ public async Task<AuthenticationResult> RunAsync(CancellationToken cancellationT
}
}

private void LogErrorTelemetryToClient(string errorCode, MsalTelemetryEventDetails telemetryEventDetails, ITelemetryClient[] telemetryClients)
private void LogMsalErrorTelemetryToClient(MsalException ex, MsalTelemetryEventDetails telemetryEventDetails, ITelemetryClient[] telemetryClients)
trwalke marked this conversation as resolved.
Show resolved Hide resolved
{
if (telemetryClients.HasEnabledClients(TelemetryConstants.AcquireTokenEventName))
{
if (ex is MsalClientException clientException)
trwalke marked this conversation as resolved.
Show resolved Hide resolved
{
telemetryEventDetails.SetProperty(TelemetryConstants.Succeeded, false);
telemetryEventDetails.SetProperty(TelemetryConstants.ErrorCode, clientException.ErrorCode);
telemetryEventDetails.SetProperty(TelemetryConstants.ErrorMessage, clientException.Message);
}

if (ex is MsalServiceException serviceException)
{
telemetryEventDetails.SetProperty(TelemetryConstants.Succeeded, false);
telemetryEventDetails.SetProperty(TelemetryConstants.ErrorCode, serviceException.ErrorCode);
telemetryEventDetails.SetProperty(TelemetryConstants.ErrorMessage, serviceException.Message);
telemetryEventDetails.SetProperty(TelemetryConstants.StsErrorCode, serviceException.ErrorCodes.AsSingleString());
trwalke marked this conversation as resolved.
Show resolved Hide resolved
}
}
}

private void LogErrorTelemetryToClient(Exception ex, MsalTelemetryEventDetails telemetryEventDetails, ITelemetryClient[] telemetryClients)
{
if (telemetryClients.HasEnabledClients(TelemetryConstants.AcquireTokenEventName))
{
telemetryEventDetails.SetProperty(TelemetryConstants.Succeeded, false);
telemetryEventDetails.SetProperty(TelemetryConstants.ErrorCode, errorCode);
telemetryEventDetails.SetProperty(TelemetryConstants.ErrorCode, ex.GetType().ToString());
telemetryEventDetails.SetProperty(TelemetryConstants.ErrorMessage, ex.Message);
}
}

private void LogSuccessfulTelemetryToClient(AuthenticationResult authenticationResult, TelemetryDatapoints telemetryDatapoints, MsalTelemetryEventDetails telemetryEventDetails, ITelemetryClient[] telemetryClients)
private void LogSuccessfulTelemetryToClient(AuthenticationResult authenticationResult, MsalTelemetryEventDetails telemetryEventDetails, ITelemetryClient[] telemetryClients)
{
if (telemetryClients.HasEnabledClients(TelemetryConstants.AcquireTokenEventName))
{
Expand All @@ -142,13 +165,14 @@ private void LogSuccessfulTelemetryToClient(AuthenticationResult authenticationR
telemetryEventDetails.SetProperty(TelemetryConstants.TokenType, (int)AuthenticationRequestParameters.RequestContext.ApiEvent.TokenType);
telemetryEventDetails.SetProperty(TelemetryConstants.RemainingLifetime, (authenticationResult.ExpiresOn - DateTime.Now).TotalMilliseconds);
telemetryEventDetails.SetProperty(TelemetryConstants.ActivityId, authenticationResult.CorrelationId);
telemetryEventDetails.SetProperty(TelemetryConstants.RefreshOn,
authenticationResult.AuthenticationResultMetadata.RefreshOn.HasValue ?
DateTimeHelpers.DateTimeToUnixTimestampMilliseconds(authenticationResult.AuthenticationResultMetadata.RefreshOn.Value)
: 0);

if (authenticationResult.AuthenticationResultMetadata.RefreshOn.HasValue)
{
telemetryEventDetails.SetProperty(TelemetryConstants.RefreshOn, DateTimeHelpers.DateTimeToUnixTimestampMilliseconds(authenticationResult.AuthenticationResultMetadata.RefreshOn.Value));
}
telemetryEventDetails.SetProperty(TelemetryConstants.AssertionType, (int)AuthenticationRequestParameters.RequestContext.ApiEvent.AssertionType);
telemetryEventDetails.SetProperty(TelemetryConstants.Endpoint, AuthenticationRequestParameters.Authority.AuthorityInfo.CanonicalAuthority.ToString());
telemetryEventDetails.SetProperty(TelemetryConstants.CacheUsed, (int)telemetryDatapoints.CacheTypeUsed);
telemetryEventDetails.SetProperty(TelemetryConstants.CacheLevel, (int)GetCacheLevel(authenticationResult));
ParseScopesForTelemetry(telemetryEventDetails);
}
}
Expand All @@ -157,19 +181,45 @@ private void ParseScopesForTelemetry(MsalTelemetryEventDetails telemetryEventDet
{
if (AuthenticationRequestParameters.Scope.Count > 0)
trwalke marked this conversation as resolved.
Show resolved Hide resolved
{
var firstScope = AuthenticationRequestParameters.Scope.First().ToString();
string firstScope = AuthenticationRequestParameters.Scope.First();

if (Uri.IsWellFormedUriString(firstScope, UriKind.Absolute))
trwalke marked this conversation as resolved.
Show resolved Hide resolved
{
telemetryEventDetails.SetProperty(TelemetryConstants.Resource, firstScope);
Uri firstScopeAsUri = new Uri(firstScope);
telemetryEventDetails.SetProperty(TelemetryConstants.Resource, $"{firstScopeAsUri.Scheme}://{firstScopeAsUri.Host}");

StringBuilder stringBuilder = new StringBuilder();

foreach (string scope in AuthenticationRequestParameters.Scope)
{
stringBuilder.Append(scope.Split(new[] { firstScopeAsUri.Host }, StringSplitOptions.None)[1].Replace("/", string.Empty) + " ");
trwalke marked this conversation as resolved.
Show resolved Hide resolved
}

telemetryEventDetails.SetProperty(TelemetryConstants.Scopes, stringBuilder.ToString().TrimEnd(' '));
}
else
{
telemetryEventDetails.SetProperty(TelemetryConstants.Scopes, JsonHelper.SerializeToJson(AuthenticationRequestParameters.Scope));
telemetryEventDetails.SetProperty(TelemetryConstants.Scopes, AuthenticationRequestParameters.Scope.AsSingleString());
}

}
}

private CacheLevel GetCacheLevel(AuthenticationResult authenticationResult)
{
if (authenticationResult.AuthenticationResultMetadata.TokenSource == TokenSource.Cache) //Check if token scource is cache
trwalke marked this conversation as resolved.
Show resolved Hide resolved
{
if (AuthenticationRequestParameters.RequestContext.ApiEvent.CacheLevel > CacheLevel.Unknown) //Check if cache has indicated which level was used
{
return AuthenticationRequestParameters.RequestContext.ApiEvent.CacheLevel;
}

//If no level was used, set to unknown
return CacheLevel.Unknown;
}

return CacheLevel.None;
}

private static void LogMetricsFromAuthResult(AuthenticationResult authenticationResult, ILoggerAdapter logger)
{
if (logger.IsLoggingEnabled(LogLevel.Always))
Expand Down Expand Up @@ -230,14 +280,15 @@ private AssertionType GetAssertionType()
{
if (!string.IsNullOrWhiteSpace(ServiceBundle.Config.ManagedIdentityUserAssignedClientId) ||
trwalke marked this conversation as resolved.
Show resolved Hide resolved
!string.IsNullOrWhiteSpace(ServiceBundle.Config.ManagedIdentityUserAssignedResourceId) ||
(!string.IsNullOrWhiteSpace(ServiceBundle.Config.TenantId) && ServiceBundle.Config.TenantId.Equals(Constants.ManagedIdentityDefaultTenant)) ||
ServiceBundle.Config.AppTokenProvider != null)
trwalke marked this conversation as resolved.
Show resolved Hide resolved
{
return AssertionType.Msi;
}

if (ServiceBundle.Config.ClientCredential != null)
{
if (ServiceBundle.Config.ClientCredential.TelemetryAssertionType == AssertionType.CertificateWithoutSni)
if (ServiceBundle.Config.ClientCredential.AssertionType == AssertionType.CertificateWithoutSni)
{
if (ServiceBundle.Config.SendX5C)
{
Expand All @@ -247,7 +298,7 @@ private AssertionType GetAssertionType()
return AssertionType.CertificateWithoutSni;
}

return ServiceBundle.Config.ClientCredential.TelemetryAssertionType;
return ServiceBundle.Config.ClientCredential.AssertionType;
}

return AssertionType.None;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -221,6 +221,11 @@ public HttpResponseHeaders Headers
/// </remarks>
internal string SubError { get; set; }

/// <summary>
/// A list of STS-specific error codes that can help in diagnostics.
/// </summary>
internal string[] ErrorCodes { get; set; }

/// <summary>
/// As per discussion with Evo, AAD
/// </summary>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ internal static MsalServiceException FromHttpResponse(
ex.Claims = oAuth2Response?.Claims;
ex.CorrelationId = oAuth2Response?.CorrelationId;
ex.SubError = oAuth2Response?.SubError;
ex.ErrorCodes = oAuth2Response?.ErrorCodes;

return ex;
}
Expand Down
Original file line number Diff line number Diff line change
@@ -1,12 +1,6 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using System;
using System.Collections.Generic;
using System.Linq;
using System.Text;
using System.Threading.Tasks;

namespace Microsoft.Identity.Client.TelemetryCore
{
internal enum AssertionType
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@

using System;
using Microsoft.Identity.Client.AuthScheme;
using Microsoft.Identity.Client.Cache;
using Microsoft.Identity.Client.Region;

namespace Microsoft.Identity.Client.TelemetryCore.Internal.Events
Expand Down Expand Up @@ -128,5 +129,11 @@ public string TokenTypeString
}

public AssertionType AssertionType { get; set; }

public CacheLevel CacheLevel { get; set; }

public static bool IsLongRunningObo(ApiIds apiId) => apiId == ApiIds.InitiateLongRunningObo || apiId == ApiIds.AcquireTokenInLongRunningObo;

public static bool IsOnBehalfOfRequest(ApiIds apiId) => apiId == ApiIds.AcquireTokenOnBehalfOf || IsLongRunningObo(apiId);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,11 @@ namespace Microsoft.Identity.Client.TelemetryCore.TelemetryClient
/// <summary>
/// Stores details to log to the <see cref="ITelemetryClient"/>.
/// </summary>
public class TelemetryDatapoints
public class TelemetryData
trwalke marked this conversation as resolved.
Show resolved Hide resolved
{
/// <summary>
/// Type of cache used. This data is captured from MSAL or Microsoft.Identity.Web to log to telemetry.
/// </summary>
public CacheTypeUsed CacheTypeUsed { get; set; } = CacheTypeUsed.None;
public CacheLevel CacheLevel { get; set; } = CacheLevel.None;
}
}
Loading