Skip to content

Telemetry #2: Implement Telemetry in msal-browser #1918

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 32 commits into from
Aug 4, 2020
Merged

Telemetry #2: Implement Telemetry in msal-browser #1918

merged 32 commits into from
Aug 4, 2020

Conversation

tnorling
Copy link
Collaborator

@tnorling tnorling commented Jul 9, 2020 •
edited
Loading

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

@github-actions github-actions bot added the msal-browser Related to msal-browser package label Jul 9, 2020
@tnorling tnorling mentioned this pull request Jul 9, 2020
Merged
tnorling added 10 commits July 9, 2020 14:36
@tnorling
Merge branch 'msal-common-MSER-Telemetry' of https://github.com/Azure…
dc6518a 
…AD/microsoft-authentication-library-for-js into msal-browser-MSER-Telemetry
@tnorling
Merge branch 'msal-common-MSER-Telemetry' of https://github.com/Azure…
0838065 
…AD/microsoft-authentication-library-for-js into msal-browser-MSER-Telemetry
@tnorling
Merge branch 'msal-common-MSER-Telemetry' of https://github.com/Azure…
f53718a 
…AD/microsoft-authentication-library-for-js into msal-browser-MSER-Telemetry
@tnorling
Merge branch 'msal-common-MSER-Telemetry' of https://github.com/Azure…
e7c25e4 
…AD/microsoft-authentication-library-for-js into msal-browser-MSER-Telemetry
@tnorling
Merge branch 'msal-common-MSER-Telemetry' of https://github.com/Azure…
54dde19 
…AD/microsoft-authentication-library-for-js into msal-browser-MSER-Telemetry
@tnorling
Merge branch 'msal-common-MSER-Telemetry' of https://github.com/Azure…
666fa9c 
…AD/microsoft-authentication-library-for-js into msal-browser-MSER-Telemetry
@tnorling
Always generateCacheKey
b6598c6
@tnorling
Add comment to API codes
455f3e5
@tnorling
Merge branch 'msal-common-MSER-Telemetry' of https://github.com/Azure…
24846c7 
…AD/microsoft-authentication-library-for-js into msal-browser-MSER-Telemetry
@tnorling
Tests passing
f73ed7d
@tnorling tnorling marked this pull request as ready for review July 15, 2020 00:57
sangonzal
sangonzal approved these changes Jul 16, 2020
View reviewed changes
Copy link
Contributor

@sangonzal sangonzal left a comment

Choose a reason for hiding this comment

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

Looks good.

@github-actions github-actions bot added the msal-common Related to msal-common package label Jul 22, 2020
@tnorling tnorling changed the base branch from msal-common-MSER-Telemetry to dev August 3, 2020 23:45
@coveralls
Copy link

coveralls commented Aug 4, 2020 •
edited
Loading

Coverage Status

Coverage decreased (-0.1%) to 81.795% when pulling 0cfa1bb on msal-browser-MSER-Telemetry into c9603bd on dev.

@tnorling tnorling merged commit 48face9 into dev Aug 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pkanher617 pkanher617 pkanher617 approved these changes

@sangonzal sangonzal sangonzal approved these changes

@DarylThayil DarylThayil Awaiting requested review from DarylThayil

@jasonnutter jasonnutter Awaiting requested review from jasonnutter

@jmckennon jmckennon Awaiting requested review from jmckennon

@jo-arroyo jo-arroyo Awaiting requested review from jo-arroyo jo-arroyo is a code owner

@sameerag sameerag Awaiting requested review from sameerag sameerag is a code owner

@hectormmg hectormmg Awaiting requested review from hectormmg hectormmg is a code owner

Assignees
No one assigned
Labels
msal-browser Related to msal-browser package msal-common Related to msal-common package
Projects
None yet
Milestone
@azure/[email protected] Release
Development

Successfully merging this pull request may close these issues.
4 participants
@tnorling @coveralls @pkanher617 @sangonzal