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 5 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 @@ -129,7 +129,8 @@ private async Task RefreshCacheForReadOperationsAsync()
requestScopes: _requestParams.Scope,
requestTenantId: _requestParams.AuthorityManager.OriginalAuthority.TenantId,
identityLogger: _requestParams.RequestContext.Logger.IdentityLogger,
piiLoggingEnabled: _requestParams.RequestContext.Logger.PiiLoggingEnabled);
piiLoggingEnabled: _requestParams.RequestContext.Logger.PiiLoggingEnabled,
telemetryDatapoints: RequestContext.TelemetryDatapoints);

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

await TokenCacheInternal.OnAfterAccessAsync(args).ConfigureAwait(false);
RequestContext.ApiEvent.DurationInCacheInMs += stopwatch.ElapsedMilliseconds;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
// Licensed under the MIT License.

using System;
using System.Collections.Generic;
trwalke marked this conversation as resolved.
Show resolved Hide resolved
using System.Threading;
using Microsoft.Identity.Client.Core;
using Microsoft.Identity.Client.Internal.Logger;
Expand All @@ -24,6 +25,8 @@ internal class RequestContext

public CancellationToken UserCancellationToken { get; }

public Dictionary<string, string> TelemetryDatapoints { get; } = new Dictionary<string, string>();

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,7 +96,7 @@ public async Task<AuthenticationResult> RunAsync(CancellationToken cancellationT
LogReturnedToken(authenticationResult);
UpdateTelemetry(sw, apiEvent, authenticationResult);
LogMetricsFromAuthResult(authenticationResult, AuthenticationRequestParameters.RequestContext.Logger);
LogSuccessfulTelemetryToClient(authenticationResult, telemetryEventDetails, telemetryClients);
LogSuccessfulTelemetryToClient(authenticationResult, AuthenticationRequestParameters.RequestContext.TelemetryDatapoints, telemetryEventDetails, telemetryClients);
trwalke marked this conversation as resolved.
Show resolved Hide resolved

return authenticationResult;
}
Expand Down Expand Up @@ -129,7 +129,7 @@ private void LogErrorTelemetryToClient(string errorCode, MsalTelemetryEventDetai
}
}

private void LogSuccessfulTelemetryToClient(AuthenticationResult authenticationResult, MsalTelemetryEventDetails telemetryEventDetails, ITelemetryClient[] telemetryClients)
private void LogSuccessfulTelemetryToClient(AuthenticationResult authenticationResult, Dictionary<string, string> telemetryDatapoints, MsalTelemetryEventDetails telemetryEventDetails, ITelemetryClient[] telemetryClients)
{
if (telemetryClients.HasEnabledClients(TelemetryConstants.AcquireTokenEventName))
{
Expand All @@ -142,6 +142,40 @@ private void LogSuccessfulTelemetryToClient(AuthenticationResult authenticationR
telemetryEventDetails.SetProperty(TelemetryConstants.PopToken, authenticationResult.TokenType.Equals(Constants.PoPTokenType));
telemetryEventDetails.SetProperty(TelemetryConstants.RemainingLifetime, (authenticationResult.ExpiresOn - DateTime.Now).TotalMilliseconds);
telemetryEventDetails.SetProperty(TelemetryConstants.ActivityId, authenticationResult.CorrelationId);
telemetryEventDetails.SetProperty(TelemetryConstants.RefreshOn,
trwalke marked this conversation as resolved.
Show resolved Hide resolved
authenticationResult.AuthenticationResultMetadata.RefreshOn.HasValue ?
DateTimeHelpers.DateTimeToUnixTimestampMilliseconds(authenticationResult.AuthenticationResultMetadata.RefreshOn.Value)
: 0);
trwalke marked this conversation as resolved.
Show resolved Hide resolved

LogDatapointsToTelemetryClient(telemetryDatapoints, telemetryEventDetails);
}
}

private void LogDatapointsToTelemetryClient(Dictionary<string, string> telemetryDatapoints, MsalTelemetryEventDetails telemetryEventDetails)
{
if ((telemetryDatapoints == null) || (telemetryDatapoints.Count() == 0)) return;

string value;

if (telemetryDatapoints.TryGetValue(TelemetryCacheConstants.CacheUsed, out value))
neha-bhargava marked this conversation as resolved.
Show resolved Hide resolved
{
int cacheUsed;
int.TryParse(value, out cacheUsed);
telemetryEventDetails.SetProperty(TelemetryCacheConstants.CacheUsed, cacheUsed);
}

if (telemetryDatapoints.TryGetValue(TelemetryCacheConstants.L1Latency, out value))
{
long latency;
long.TryParse(value, out latency);
telemetryEventDetails.SetProperty(TelemetryCacheConstants.L1Latency, latency);
}

if (telemetryDatapoints.TryGetValue(TelemetryCacheConstants.L2Latency, out value))
{
long latency;
long.TryParse(value, out latency);
telemetryEventDetails.SetProperty(TelemetryCacheConstants.L2Latency, latency);
}
}

Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright (c) Microsoft Corporation. All rights reserved.
// Licensed under the MIT License.

using Microsoft.IdentityModel.Abstractions;

namespace Microsoft.Identity.Client.TelemetryCore.TelemetryClient
{
/// <summary>
/// Contains the parameters of cache to log to telemetry.
/// </summary>
public class TelemetryCacheConstants
{
/// <summary>
/// Constant to use as key to Cache Details dictionary.
/// </summary>
public const string CacheUsed = "CacheUsed";

/// <summary>
/// Constant to use as key to Cache Details dictionary.
/// </summary>
public const string L1Latency = "L1Latency";

/// <summary>
/// Constant to use as key to Cache Details dictionary.
/// </summary>
public const string L2Latency = "L2Latency";
}

/// <summary>
/// Indicates the cache which was used to get the token.
/// </summary>
public enum CacheUsed
{
/// <summary>
/// Indicates that the token was not cached
/// </summary>
None = 0,
neha-bhargava marked this conversation as resolved.
Show resolved Hide resolved

/// <summary>
/// Token was obtained from Memory Cache
/// </summary>
MemoryCache = 1,

/// <summary>
/// Token was obtained from Distributed Cache
/// </summary>
DistributedCache = 2
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,7 @@ internal static class TelemetryConstants
public const string DurationInHttp = "DurationInHttp";
public const string ActivityId = "ActivityId";
public const string Resource = "Resource";
public const string RefreshOn = "RefreshOn";

#endregion
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,7 +177,8 @@ async Task<Tuple<MsalAccessTokenCacheItem, MsalIdTokenCacheItem, Account>> IToke
requestScopes: requestParams.Scope,
requestTenantId: requestParams.AuthorityManager.OriginalAuthority.TenantId,
identityLogger: requestParams.RequestContext.Logger.IdentityLogger,
piiLoggingEnabled: requestParams.RequestContext.Logger.PiiLoggingEnabled);
piiLoggingEnabled: requestParams.RequestContext.Logger.PiiLoggingEnabled,
telemetryDatapoints: requestParams.RequestContext.TelemetryDatapoints);

Stopwatch sw = Stopwatch.StartNew();

Expand Down Expand Up @@ -249,7 +250,8 @@ async Task<Tuple<MsalAccessTokenCacheItem, MsalIdTokenCacheItem, Account>> IToke
requestScopes: requestParams.Scope,
requestTenantId: requestParams.AuthorityManager.OriginalAuthority.TenantId,
identityLogger: requestParams.RequestContext.Logger.IdentityLogger,
piiLoggingEnabled: requestParams.RequestContext.Logger.PiiLoggingEnabled);
piiLoggingEnabled: requestParams.RequestContext.Logger.PiiLoggingEnabled,
telemetryDatapoints: requestParams.RequestContext.TelemetryDatapoints);

Stopwatch sw = Stopwatch.StartNew();
await tokenCacheInternal.OnAfterAccessAsync(args).ConfigureAwait(false);
Expand Down Expand Up @@ -741,7 +743,8 @@ internal async Task ExpireAllAccessTokensForTestAsync()
requestScopes: null,
requestTenantId: null,
identityLogger: null,
piiLoggingEnabled: false);
piiLoggingEnabled: false,
telemetryDatapoints: new Dictionary<string, string>());

await tokenCacheInternal.OnAfterAccessAsync(args).ConfigureAwait(false);
}
Expand Down Expand Up @@ -1162,7 +1165,8 @@ async Task ITokenCacheInternal.RemoveAccountAsync(IAccount account, Authenticati
requestScopes: requestParameters.Scope,
requestTenantId: requestParameters.AuthorityManager.OriginalAuthority.TenantId,
identityLogger: requestParameters.RequestContext.Logger.IdentityLogger,
piiLoggingEnabled: requestParameters.RequestContext.Logger.PiiLoggingEnabled);
piiLoggingEnabled: requestParameters.RequestContext.Logger.PiiLoggingEnabled,
telemetryDatapoints: requestParameters.RequestContext.TelemetryDatapoints);


await tokenCacheInternal.OnBeforeAccessAsync(args).ConfigureAwait(false);
Expand Down Expand Up @@ -1198,7 +1202,8 @@ async Task ITokenCacheInternal.RemoveAccountAsync(IAccount account, Authenticati
requestScopes: requestParameters.Scope,
requestTenantId: requestParameters.AuthorityManager.OriginalAuthority.TenantId,
identityLogger: requestParameters.RequestContext.Logger.IdentityLogger,
piiLoggingEnabled: requestParameters.RequestContext.Logger.PiiLoggingEnabled);
piiLoggingEnabled: requestParameters.RequestContext.Logger.PiiLoggingEnabled,
telemetryDatapoints: requestParameters.RequestContext.TelemetryDatapoints);


await tokenCacheInternal.OnAfterAccessAsync(args).ConfigureAwait(false);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -146,6 +146,44 @@ public TokenCacheNotificationArgs( // only use this constructor in product co
PiiLoggingEnabled = piiLoggingEnabled;
}

/// <summary>
/// This constructor is for test purposes only. It allows apps to unit test their MSAL token cache implementation code.
/// </summary>
public TokenCacheNotificationArgs( // only use this constructor in product code
trwalke marked this conversation as resolved.
Show resolved Hide resolved
trwalke marked this conversation as resolved.
Show resolved Hide resolved
ITokenCacheSerializer tokenCache,
string clientId,
IAccount account,
bool hasStateChanged,
bool isApplicationCache,
string suggestedCacheKey,
bool hasTokens,
DateTimeOffset? suggestedCacheExpiry,
CancellationToken cancellationToken,
Guid correlationId,
IEnumerable<string> requestScopes,
string requestTenantId,
IIdentityLogger identityLogger,
bool piiLoggingEnabled,
Dictionary<string, string> telemetryDatapoints)

{
TokenCache = tokenCache;
ClientId = clientId;
Account = account;
HasStateChanged = hasStateChanged;
IsApplicationCache = isApplicationCache;
SuggestedCacheKey = suggestedCacheKey;
HasTokens = hasTokens;
CancellationToken = cancellationToken;
CorrelationId = correlationId;
RequestScopes = requestScopes;
RequestTenantId = requestTenantId;
SuggestedCacheExpiry = suggestedCacheExpiry;
IdentityLogger = identityLogger;
PiiLoggingEnabled = piiLoggingEnabled;
TelemetryDatapoints = telemetryDatapoints;
}

/// <summary>
/// Gets the <see cref="ITokenCacheSerializer"/> involved in the transaction
/// </summary>
Expand Down Expand Up @@ -248,5 +286,10 @@ public TokenCacheNotificationArgs( // only use this constructor in product co
/// Boolean used to determine if Personally Identifiable Information (PII) logging is enabled.
/// </summary>
public bool PiiLoggingEnabled { get; }

/// <summary>
/// Cache Details contains the details of L1/ L2 cache for telemetry logging.
/// </summary>
public Dictionary<string, string> TelemetryDatapoints { get; }
}
}