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

tnorling · 2020-07-09T23:50:45Z

This PR builds on PR #1917 to enable sending Telemetry data to eSTS via 2 extra headers on all /token calls

Responsibilities of each library are:

msal-common:

Provides TelemetryManager class
Add Telemetry headers to each /token request
Clear telemetry related cache items on successful network call (Does not matter if the auth request itself is successful or not, just that the server acknowledged the request)
SilentFlowClient increments cache hit counter each time tokens are returned from the cache

msal-browser, msal-node, etc:

Instantiate instance of TelemetryManager with apiId, correlationId, cache implementation, forceRefresh value (silent calls only)
Pass Telemetry instance to authCodeClient and silentFlowClient
Cache errors

lib/msal-node/src/utils/Constants.ts

…AD/microsoft-authentication-library-for-js into msal-node-MSER-Telemetry

sangonzal

Overall looks good. You might be able to avoid some code duplication by creating an acquire token common API that takes in a BaseClient and ClientConfiguration and calls acquireToken, logs, and cachesFailedRequests

lib/msal-node/src/utils/Constants.ts

lib/msal-node/src/client/ClientApplication.ts

…AD/microsoft-authentication-library-for-js into msal-node-MSER-Telemetry

tnorling

Still need to add the new telemetry CacheSchemaType to the Node Storage class

…AD/microsoft-authentication-library-for-js into msal-node-MSER-Telemetry

coveralls · 2020-08-04T00:00:08Z

Coverage decreased (-0.008%) to 83.168% when pulling 0475b8d on msal-node-MSER-Telemetry into 126b83b on dev.

…tion-library-for-js into msal-node-MSER-Telemetry

…/microsoft-authentication-library-for-js into msal-node-MSER-Telemetry

…zureAD/microsoft-authentication-library-for-js into msal-node-MSER-Telemetry

…ft-authentication-library-for-js into msal-node-MSER-Telemetry

…osoft-authentication-library-for-js into msal-node-MSER-Telemetry

…zureAD/microsoft-authentication-library-for-js into msal-node-MSER-Telemetry

…/microsoft-authentication-library-for-js into msal-node-MSER-Telemetry

sameerag · 2020-09-03T22:41:14Z

lib/msal-node/src/client/ClientApplication.ts

+                refreshTokenClientConfig
+            );
+            return refreshTokenClient.acquireToken(validRequest);
+        } catch (e) {


Are we sure the catch is always telemetry initialization error?

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

sameerag · 2020-09-03T22:42:46Z

lib/msal-node/src/utils/Constants.ts

+    acquireTokenByCode = 871,
+    acquireTokenByRefreshToken = 872,
+    acquireTokenByDeviceCode = 671
+};


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

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

sameerag

two comments, approved.

…tion-library-for-js into msal-node-MSER-Telemetry

tnorling added 2 commits July 9, 2020 16:48

Implement Telemetry in Node

ebbe74f

Add forceRefresh to acquireTokenSilent Telemetry

eb28059

github-actions bot added the msal-node Related to msal-node package label Jul 9, 2020

tnorling commented Jul 10, 2020

View reviewed changes

lib/msal-node/src/utils/Constants.ts Outdated Show resolved Hide resolved

tnorling linked an issue Jul 13, 2020 that may be closed by this pull request

[msal-node] Support MSER Telemetry #1481

Closed

3 tasks

tnorling added 5 commits July 13, 2020 10:22

Add comment to API codes

7dc2daf

Null assert correaltionId

da6bede

Fix function name

d2b1807

Merge branch 'msal-common-MSER-Telemetry' of https://github.com/Azure…

19549ee

…AD/microsoft-authentication-library-for-js into msal-node-MSER-Telemetry

Fix import

75cb78e

tnorling marked this pull request as ready for review July 15, 2020 00:57

tnorling requested review from DarylThayil, jasonnutter, jmckennon, jo-arroyo, pkanher617, sameerag and hectormmg as code owners July 15, 2020 00:57

sangonzal approved these changes Jul 16, 2020

View reviewed changes

lib/msal-node/src/utils/Constants.ts Show resolved Hide resolved

lib/msal-node/src/client/ClientApplication.ts Show resolved Hide resolved

tnorling added 2 commits July 16, 2020 17:07

Update to ServerTelemetryManager

561bdfe

Merge branch 'msal-common-MSER-Telemetry' of https://github.com/Azure…

58bc4af

…AD/microsoft-authentication-library-for-js into msal-node-MSER-Telemetry

tnorling commented Jul 17, 2020

View reviewed changes

tnorling added 3 commits July 22, 2020 17:14

Merge branch 'msal-common-MSER-Telemetry' of https://github.com/Azure…

4cf4985

…AD/microsoft-authentication-library-for-js into msal-node-MSER-Telemetry

Add telemetryManager to authCodeClient

8888eb2

Merge branch 'msal-common-MSER-Telemetry' of https://github.com/Azure…

c970122

…AD/microsoft-authentication-library-for-js into msal-node-MSER-Telemetry

pkanher617 approved these changes Jul 30, 2020

View reviewed changes

tnorling changed the base branch from msal-common-MSER-Telemetry to dev August 3, 2020 23:45

Merge branch 'dev' into msal-node-MSER-Telemetry

078d413

Merge branch 'dev' of https://github.com/AzureAD/microsoft-authentica…

f1cf9a0

…tion-library-for-js into msal-node-MSER-Telemetry

tnorling added 2 commits August 6, 2020 10:17

Track suberror if it is present

24ed043

Merge branch 'msal-node-MSER-Telemetry' of https://github.com/AzureAD…

c5245aa

…/microsoft-authentication-library-for-js into msal-node-MSER-Telemetry

github-actions bot added the msal-common Related to msal-common package label Aug 6, 2020

tnorling added 2 commits August 6, 2020 10:32

Fix lint

f3c6baa

Merge branch 'msal-node-storage-optimization' of https://github.com/A…

4f37e2e

…zureAD/microsoft-authentication-library-for-js into msal-node-MSER-Telemetry

tnorling changed the base branch from dev to msal-node-storage-optimization August 6, 2020 17:47

github-actions bot added the samples Related to the samples apps for the library. label Aug 6, 2020

Merge branch 'msal-node-storage-optimization' of https://github.com/A…

9de6d65

…zureAD/microsoft-authentication-library-for-js into msal-node-MSER-Telemetry

tnorling added this to the @azure/msal-node@1.0.0-alpha.5 milestone Aug 11, 2020

tnorling added 2 commits August 11, 2020 13:08

Merge branch 'tnorling-patch-2' of https://github.com/AzureAD/microso…

f3badab

…ft-authentication-library-for-js into msal-node-MSER-Telemetry

Merge branch 'update-headers-type' of https://github.com/AzureAD/micr…

fbe5af9

…osoft-authentication-library-for-js into msal-node-MSER-Telemetry

tnorling removed the samples Related to the samples apps for the library. label Aug 12, 2020

tnorling added 2 commits August 18, 2020 12:47

Merge branch 'msal-node-storage-optimization' of https://github.com/A…

31dfac1

…zureAD/microsoft-authentication-library-for-js into msal-node-MSER-Telemetry

Merge branch 'msal-node-storage-optimization' of https://github.com/A…

2d40274

…zureAD/microsoft-authentication-library-for-js into msal-node-MSER-Telemetry

tnorling modified the milestones: @azure/msal-node@1.0.0-alpha.5, @azure/msal-node@1.0.0-alpha.6 Aug 25, 2020

tnorling changed the base branch from msal-node-storage-optimization to dev September 2, 2020 19:33

tnorling and others added 4 commits September 2, 2020 12:34

Merge branch 'dev' into msal-node-MSER-Telemetry

3b48482

Merge branch 'msal-node-MSER-Telemetry' of https://github.com/AzureAD…

f0ba824

…/microsoft-authentication-library-for-js into msal-node-MSER-Telemetry

Fix build

7d760ac

Change files

1872900

sameerag reviewed Sep 3, 2020

View reviewed changes

sameerag approved these changes Sep 3, 2020

View reviewed changes

tnorling added 3 commits September 3, 2020 16:09

Add Client Credential flow

01897d5

Merge branch 'dev' of https://github.com/AzureAD/microsoft-authentica…

1afb052

…tion-library-for-js into msal-node-MSER-Telemetry

Fix lint

0475b8d

tnorling merged commit aa74343 into dev Sep 10, 2020

tnorling deleted the msal-node-MSER-Telemetry branch September 29, 2020 20:36

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

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

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

tnorling commented Jul 9, 2020 •

edited

Loading

sangonzal left a comment

tnorling left a comment

coveralls commented Aug 4, 2020 •

edited

Loading

sameerag Sep 3, 2020

tnorling Sep 3, 2020

sameerag Sep 3, 2020

tnorling Sep 3, 2020

sameerag left a comment

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

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

Conversation

tnorling commented Jul 9, 2020 • edited Loading

sangonzal left a comment

Choose a reason for hiding this comment

tnorling left a comment

Choose a reason for hiding this comment

coveralls commented Aug 4, 2020 • edited Loading

sameerag Sep 3, 2020

Choose a reason for hiding this comment

tnorling Sep 3, 2020

Choose a reason for hiding this comment

sameerag Sep 3, 2020

Choose a reason for hiding this comment

tnorling Sep 3, 2020

Choose a reason for hiding this comment

sameerag left a comment

Choose a reason for hiding this comment

tnorling commented Jul 9, 2020 •

edited

Loading

coveralls commented Aug 4, 2020 •

edited

Loading