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

Telemetry #3: Implement Telemetry in msal-node #1921

Merged
merged 30 commits into from
Sep 10, 2020
Merged
Show file tree
Hide file tree
Changes from 27 commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
ebbe74f
Implement Telemetry in Node
tnorling Jul 9, 2020
eb28059
Add forceRefresh to acquireTokenSilent Telemetry
tnorling Jul 9, 2020
7dc2daf
Add comment to API codes
tnorling Jul 13, 2020
da6bede
Null assert correaltionId
tnorling Jul 13, 2020
d2b1807
Fix function name
tnorling Jul 14, 2020
19549ee
Merge branch 'msal-common-MSER-Telemetry' of https://github.com/Azure…
tnorling Jul 14, 2020
75cb78e
Fix import
tnorling Jul 15, 2020
561bdfe
Update to ServerTelemetryManager
tnorling Jul 17, 2020
58bc4af
Merge branch 'msal-common-MSER-Telemetry' of https://github.com/Azure…
tnorling Jul 17, 2020
4cf4985
Merge branch 'msal-common-MSER-Telemetry' of https://github.com/Azure…
tnorling Jul 23, 2020
8888eb2
Add telemetryManager to authCodeClient
tnorling Jul 23, 2020
c970122
Merge branch 'msal-common-MSER-Telemetry' of https://github.com/Azure…
tnorling Jul 25, 2020
078d413
Merge branch 'dev' into msal-node-MSER-Telemetry
tnorling Aug 3, 2020
f1cf9a0
Merge branch 'dev' of https://github.com/AzureAD/microsoft-authentica…
tnorling Aug 6, 2020
24ed043
Track suberror if it is present
tnorling Aug 6, 2020
c5245aa
Merge branch 'msal-node-MSER-Telemetry' of https://github.com/AzureAD…
tnorling Aug 6, 2020
f3c6baa
Fix lint
tnorling Aug 6, 2020
4f37e2e
Merge branch 'msal-node-storage-optimization' of https://github.com/A…
tnorling Aug 6, 2020
9de6d65
Merge branch 'msal-node-storage-optimization' of https://github.com/A…
tnorling Aug 6, 2020
f3badab
Merge branch 'tnorling-patch-2' of https://github.com/AzureAD/microso…
tnorling Aug 11, 2020
fbe5af9
Merge branch 'update-headers-type' of https://github.com/AzureAD/micr…
tnorling Aug 11, 2020
31dfac1
Merge branch 'msal-node-storage-optimization' of https://github.com/A…
tnorling Aug 18, 2020
2d40274
Merge branch 'msal-node-storage-optimization' of https://github.com/A…
tnorling Aug 20, 2020
3b48482
Merge branch 'dev' into msal-node-MSER-Telemetry
tnorling Sep 2, 2020
f0ba824
Merge branch 'msal-node-MSER-Telemetry' of https://github.com/AzureAD…
tnorling Sep 2, 2020
7d760ac
Fix build
tnorling Sep 2, 2020
1872900
Change files
tnorling Sep 2, 2020
01897d5
Add Client Credential flow
tnorling Sep 3, 2020
1afb052
Merge branch 'dev' of https://github.com/AzureAD/microsoft-authentica…
tnorling Sep 10, 2020
0475b8d
Fix lint
tnorling Sep 10, 2020
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
@@ -0,0 +1,8 @@
{
"type": "patch",
"comment": "Track Suberrors in Telemetry (#1921)",
"packageName": "@azure/msal-common",
"email": "thomas.norling@microsoft.com",
"dependentChangeType": "patch",
"date": "2020-09-02T20:01:33.353Z"
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,8 @@
{
"type": "prerelease",
"comment": "Implement Telemetry in msal-node (#1921)",
"packageName": "@azure/msal-node",
"email": "thomas.norling@microsoft.com",
"dependentChangeType": "patch",
"date": "2020-09-02T20:01:47.198Z"
}
4 changes: 3 additions & 1 deletion lib/msal-common/src/error/AuthError.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,14 +22,16 @@ export class AuthError extends Error {
errorCode: string;
// Detailed description of error
errorMessage: string;
suberror: string;

constructor(errorCode: string, errorMessage?: string) {
constructor(errorCode: string, errorMessage?: string, suberror?: string) {
const errorString = errorMessage ? `${errorCode}: ${errorMessage}` : errorCode;
super(errorString);
Object.setPrototypeOf(this, AuthError.prototype);

this.errorCode = errorCode;
this.errorMessage = errorMessage;
this.suberror = suberror;
this.name = "AuthError";
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import { CacheManager } from "../../cache/CacheManager";
import { ServerTelemetryCacheValue } from "./ServerTelemetryCacheValue";
import { AuthError } from "../../error/AuthError";
import { ServerTelemetryRequest } from "./ServerTelemetryRequest";
import { StringUtils } from "../../utils/StringUtils";

export class ServerTelemetryManager {
private cacheManager: CacheManager;
Expand Down Expand Up @@ -49,7 +50,7 @@ export class ServerTelemetryManager {
cacheFailedRequest(error: AuthError): void {
const lastRequests = this.getLastRequests();
lastRequests.failedRequests.push(this.apiId, this.correlationId);
lastRequests.errors.push(error.errorCode);
lastRequests.errors.push(StringUtils.isEmpty(error.suberror)? error.errorCode: error.suberror);
lastRequests.errorCount += 1;

if (lastRequests.errors.length > SERVER_TELEM_CONSTANTS.FAILURE_LIMIT) {
Expand Down
101 changes: 70 additions & 31 deletions lib/msal-node/src/client/ClientApplication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,20 +19,22 @@ import {
BaseAuthRequest,
SilentFlowRequest,
SilentFlowClient,
Logger
Logger,
ServerTelemetryManager,
ServerTelemetryRequest
} from '@azure/msal-common';
import { Configuration, buildAppConfiguration } from '../config/Configuration';
import { CryptoProvider } from '../crypto/CryptoProvider';
import { Storage } from '../cache/Storage';
import { Constants as NodeConstants } from './../utils/Constants';
import { Constants as NodeConstants, ApiId } from './../utils/Constants';
import { TokenCache } from '../cache/TokenCache';
import { ClientAssertion } from "../client/ClientAssertion";
const pjson = require("../../package.json");

export abstract class ClientApplication {
private _authority: Authority;
private readonly cryptoProvider: CryptoProvider;
private storage: Storage;
protected storage: Storage;
private tokenCache: TokenCache;
protected logger: Logger;
protected config: Configuration;
Expand Down Expand Up @@ -74,7 +76,7 @@ export abstract class ClientApplication {
const authorizationCodeClient = new AuthorizationCodeClient(
authClientConfig
);
return authorizationCodeClient.getAuthCodeUrl(this.initializeRequestScopes(request) as AuthorizationUrlRequest);
return authorizationCodeClient.getAuthCodeUrl(this.initializeRequest(request) as AuthorizationUrlRequest);
}

/**
Expand All @@ -87,14 +89,22 @@ export abstract class ClientApplication {
*/
async acquireTokenByCode(request: AuthorizationCodeRequest): Promise<AuthenticationResult> {
this.logger.info("acquireTokenByCode called");
const authClientConfig = await this.buildOauthClientConfiguration(
request.authority
);
this.logger.verbose("Auth client config generated");
const authorizationCodeClient = new AuthorizationCodeClient(
authClientConfig
);
return authorizationCodeClient.acquireToken(this.initializeRequestScopes(request) as AuthorizationCodeRequest);
const validRequest = this.initializeRequest(request) as AuthorizationCodeRequest;
tnorling marked this conversation as resolved.
Show resolved Hide resolved
const serverTelemetryManager = this.initializeServerTelemetryManager(ApiId.acquireTokenByCode, validRequest.correlationId!);
try {
const authClientConfig = await this.buildOauthClientConfiguration(
request.authority,
serverTelemetryManager
);
this.logger.verbose("Auth client config generated");
const authorizationCodeClient = new AuthorizationCodeClient(
authClientConfig
);
return authorizationCodeClient.acquireToken(validRequest);
} catch (e) {
serverTelemetryManager.cacheFailedRequest(e);
throw e;
}
}

/**
Expand All @@ -106,14 +116,22 @@ export abstract class ClientApplication {
*/
async acquireTokenByRefreshToken(request: RefreshTokenRequest): Promise<AuthenticationResult> {
this.logger.info("acquireTokenByRefreshToken called");
const refreshTokenClientConfig = await this.buildOauthClientConfiguration(
request.authority
);
this.logger.verbose("Auth client config generated");
const refreshTokenClient = new RefreshTokenClient(
refreshTokenClientConfig
);
return refreshTokenClient.acquireToken(this.initializeRequestScopes(request) as RefreshTokenRequest);
const validRequest = this.initializeRequest(request) as RefreshTokenRequest;
const serverTelemetryManager = this.initializeServerTelemetryManager(ApiId.acquireTokenByRefreshToken, validRequest.correlationId!);
try {
const refreshTokenClientConfig = await this.buildOauthClientConfiguration(
request.authority,
serverTelemetryManager
);
this.logger.verbose("Auth client config generated");
const refreshTokenClient = new RefreshTokenClient(
refreshTokenClientConfig
);
return refreshTokenClient.acquireToken(validRequest);
} catch (e) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are we sure the catch is always telemetry initialization error?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not. The catch is for any error that occurs in the flow so that Telemetry can properly cache it

serverTelemetryManager.cacheFailedRequest(e);
throw e;
}
}

/**
Expand All @@ -125,13 +143,21 @@ export abstract class ClientApplication {
* and the guidance is for the user to call any interactive token acquisition API (eg: `acquireTokenByCode()`).
*/
async acquireTokenSilent(request: SilentFlowRequest): Promise<AuthenticationResult> {
const silentFlowClientConfig = await this.buildOauthClientConfiguration(
request.authority
);
const silentFlowClient = new SilentFlowClient(
silentFlowClientConfig
);
return silentFlowClient.acquireToken(this.initializeRequestScopes(request) as SilentFlowRequest);
const validRequest = this.initializeRequest(request) as SilentFlowRequest;
const serverTelemetryManager = this.initializeServerTelemetryManager(ApiId.acquireTokenSilent, validRequest.correlationId!, validRequest.forceRefresh);
try {
const silentFlowClientConfig = await this.buildOauthClientConfiguration(
request.authority,
serverTelemetryManager
);
const silentFlowClient = new SilentFlowClient(
silentFlowClientConfig
);
return silentFlowClient.acquireToken(validRequest);
} catch (e) {
serverTelemetryManager.cacheFailedRequest(e);
throw e;
}
}

/**
Expand All @@ -142,7 +168,7 @@ export abstract class ClientApplication {
return this.tokenCache;
}

protected async buildOauthClientConfiguration(authority?: string): Promise<ClientConfiguration> {
protected async buildOauthClientConfiguration(authority?: string, serverTelemetryManager?: ServerTelemetryManager): Promise<ClientConfiguration> {
this.logger.verbose("buildOauthClientConfiguration called");
// using null assertion operator as we ensure that all config values have default values in buildConfiguration()

Expand All @@ -163,6 +189,7 @@ export abstract class ClientApplication {
cryptoInterface: this.cryptoProvider,
networkInterface: this.config.system!.networkClient,
storageInterface: this.storage,
serverTelemetryManager: serverTelemetryManager,
clientCredentials: {
clientSecret: this.clientSecret,
clientAssertion: this.clientAssertion ? this.getClientAssertion() : undefined,
Expand All @@ -184,16 +211,28 @@ export abstract class ClientApplication {
}

/**
* Generates a request with the default scopes.
* Generates a request with the default scopes & generates a correlationId.
* @param authRequest
*/
protected initializeRequestScopes(authRequest: BaseAuthRequest): BaseAuthRequest {
protected initializeRequest(authRequest: BaseAuthRequest): BaseAuthRequest {
this.logger.verbose("initializeRequestScopes called");

return {
...authRequest,
scopes: [...((authRequest && authRequest.scopes) || []), Constants.OPENID_SCOPE, Constants.PROFILE_SCOPE, Constants.OFFLINE_ACCESS_SCOPE]
scopes: [...((authRequest && authRequest.scopes) || []), Constants.OPENID_SCOPE, Constants.PROFILE_SCOPE, Constants.OFFLINE_ACCESS_SCOPE],
correlationId: authRequest && authRequest.correlationId || this.cryptoProvider.createNewGuid()
};
}

protected initializeServerTelemetryManager(apiId: number, correlationId: string, forceRefresh?: boolean): ServerTelemetryManager {
const telemetryPayload: ServerTelemetryRequest = {
clientId: this.config.auth.clientId,
correlationId: correlationId,
apiId: apiId,
forceRefresh: forceRefresh || false
};

return new ServerTelemetryManager(telemetryPayload, this.storage);
}

/**
Expand Down
21 changes: 15 additions & 6 deletions lib/msal-node/src/client/PublicClientApplication.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import { DeviceCodeClient, DeviceCodeRequest, AuthenticationResult } from '@azure/msal-common';
import { Configuration } from '../config/Configuration';
import { ClientApplication } from './ClientApplication';
import { ApiId } from '../utils/Constants';

/**
* This class is to be used to acquire tokens for public client applications (desktop, mobile). Public client applications
Expand Down Expand Up @@ -44,11 +45,19 @@ export class PublicClientApplication extends ClientApplication {
*/
public async acquireTokenByDeviceCode(request: DeviceCodeRequest): Promise<AuthenticationResult> {
this.logger.info("acquireTokenByDeviceCode called");
const deviceCodeConfig = await this.buildOauthClientConfiguration(
request.authority
);
this.logger.verbose("Auth client config generated");
const deviceCodeClient = new DeviceCodeClient(deviceCodeConfig);
return deviceCodeClient.acquireToken(this.initializeRequestScopes(request) as DeviceCodeRequest);
const validRequest = this.initializeRequest(request) as DeviceCodeRequest;
const serverTelemetryManager = this.initializeServerTelemetryManager(ApiId.acquireTokenByDeviceCode, validRequest.correlationId!);
try {
const deviceCodeConfig = await this.buildOauthClientConfiguration(
request.authority,
serverTelemetryManager
);
this.logger.verbose("Auth client config generated");
const deviceCodeClient = new DeviceCodeClient(deviceCodeConfig);
return deviceCodeClient.acquireToken(validRequest);
} catch (e) {
serverTelemetryManager.cacheFailedRequest(e);
throw e;
}
}
}
14 changes: 14 additions & 0 deletions lib/msal-node/src/utils/Constants.ts
Original file line number Diff line number Diff line change
Expand Up @@ -47,6 +47,20 @@ export const Constants = {
JWT_BEARER_ASSERTION_TYPE: 'urn:ietf:params:oauth:client-assertion-type:jwt-bearer'
};

/**
* API Codes for Telemetry purposes.
* Before adding a new code you must claim it in the MSAL Telemetry tracker as these number spaces are shared across all MSALs
* 0-99 Silent Flow
* 600-699 Device Code Flow
* 800-899 Auth Code Flow
*/
export enum ApiId {
sangonzal marked this conversation as resolved.
Show resolved Hide resolved
acquireTokenSilent = 62,
acquireTokenByCode = 871,
acquireTokenByRefreshToken = 872,
acquireTokenByDeviceCode = 671
};
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Are they no API ids for confidential clients? Are we adding it later?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the callout. Confidential Client wasn't built out yet when this PR was first put up. Will add.


/**
* JWT constants
*/
Expand Down