Skip to content

Commit 8f6c61c

Browse files
tnorlingtnorling
authored
Upgrade/Rollback Telemetry (#7738)
Adds tracking for upgrades and downgrades
1 parent 533ccad commit 8f6c61c

11 files changed

Lines changed: 209 additions & 60 deletions

File tree

‎change/@azure-msal-browser-f198cf62-fc18-4c1b-99d3-2317f048ed7c.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "Upgrade/rollback telemetry #7738",
4+
"packageName": "@azure/msal-browser",
5+
"email": "thomas.norling@microsoft.com",
6+
"dependentChangeType": "patch"
7+
}

‎change/@azure-msal-common-5480aa2a-a988-4371-a778-2c3c77a7a30a.json‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,7 @@
1+
{
2+
"type": "patch",
3+
"comment": "Upgrade/rollback telemetry #7738",
4+
"packageName": "@azure/msal-common",
5+
"email": "thomas.norling@microsoft.com",
6+
"dependentChangeType": "patch"
7+
}

‎lib/msal-browser/src/cache/BrowserCacheManager.ts‎

Lines changed: 24 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,7 @@ import { getAccountKeys, getTokenKeys } from "./CacheHelpers.js";
6666
import { EventType } from "../event/EventType.js";
6767
import { EventHandler } from "../event/EventHandler.js";
6868
import { clearHash } from "../utils/BrowserUtils.js";
69+
import { version } from "../packageMetadata.js";
6970

7071
/**
7172
* This class implements the cache storage interface for MSAL through browser local or session storage.
@@ -123,6 +124,29 @@ export class BrowserCacheManager extends CacheManager {
123124

124125
async initialize(correlationId: string): Promise<void> {
125126
await this.browserStorage.initialize(correlationId);
127+
this.trackVersionChanges(correlationId);
128+
}
129+
130+
/**
131+
* Tracks upgrades and downgrades for telemetry and debugging purposes
132+
*/
133+
private trackVersionChanges(correlationId: string): void {
134+
const previousVersion = this.browserStorage.getItem(
135+
StaticCacheKeys.VERSION
136+
);
137+
if (previousVersion) {
138+
this.logger.info(
139+
`MSAL.js was last initialized by version: ${previousVersion}`
140+
);
141+
this.performanceClient.addFields(
142+
{ previousLibraryVersion: previousVersion },
143+
correlationId
144+
);
145+
}
146+
147+
if (previousVersion !== version) {
148+
this.browserStorage.setItem(StaticCacheKeys.VERSION, version);
149+
}
126150
}
127151

128152
/**

‎lib/msal-browser/src/utils/BrowserConstants.ts‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,7 @@ export type TemporaryCacheKeys =
103103
export const StaticCacheKeys = {
104104
ACCOUNT_KEYS: "msal.account.keys",
105105
TOKEN_KEYS: "msal.token.keys",
106+
VERSION: "msal.version",
106107
} as const;
107108
export type StaticCacheKeys =
108109
(typeof StaticCacheKeys)[keyof typeof StaticCacheKeys];

‎lib/msal-browser/test/app/PublicClientApplication.spec.ts‎

Lines changed: 20 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -30,7 +30,6 @@ import {
3030
CacheHelpers,
3131
CacheManager,
3232
ClientAuthErrorCodes,
33-
CommonAuthorizationCodeRequest,
3433
CommonAuthorizationUrlRequest,
3534
CommonSilentFlowRequest,
3635
Constants,
@@ -60,6 +59,7 @@ import {
6059
BrowserConstants,
6160
CacheLookupPolicy,
6261
InteractionType,
62+
StaticCacheKeys,
6363
PlatformAuthConstants,
6464
TemporaryCacheKeys,
6565
WrapperSKU,
@@ -76,7 +76,6 @@ import { NavigationOptions } from "../../src/navigation/NavigationOptions.js";
7676
import { EventMessage } from "../../src/event/EventMessage.js";
7777
import { EventHandler } from "../../src/event/EventHandler.js";
7878
import { SilentIframeClient } from "../../src/interaction_client/SilentIframeClient.js";
79-
import { base64Encode } from "../../src/encode/Base64Encode.js";
8079
import { FetchClient } from "../../src/network/FetchClient.js";
8180
import {
8281
BrowserAuthError,
@@ -89,10 +88,6 @@ import { RedirectClient } from "../../src/interaction_client/RedirectClient.js";
8988
import { PopupClient } from "../../src/interaction_client/PopupClient.js";
9089
import { SilentCacheClient } from "../../src/interaction_client/SilentCacheClient.js";
9190
import { SilentRefreshClient } from "../../src/interaction_client/SilentRefreshClient.js";
92-
import {
93-
AuthorizationCodeRequest,
94-
EndSessionRequest,
95-
} from "../../src/index.js";
9691
import { SilentAuthCodeClient } from "../../src/interaction_client/SilentAuthCodeClient.js";
9792
import { BrowserCacheManager } from "../../src/cache/BrowserCacheManager.js";
9893
import { PlatformAuthExtensionHandler } from "../../src/broker/nativeBroker/PlatformAuthExtensionHandler.js";
@@ -117,6 +112,9 @@ import {
117112
TestTimeUtils,
118113
} from "msal-test-utils";
119114
import { INTERACTION_TYPE } from "../../src/utils/BrowserConstants.js";
115+
import { version } from "../../src/packageMetadata.js";
116+
import { AuthorizationCodeRequest } from "../../src/request/AuthorizationCodeRequest.js";
117+
import { EndSessionRequest } from "../../src/request/EndSessionRequest.js";
120118
import { BaseOperatingContext } from "../../src/operatingcontext/BaseOperatingContext.js";
121119
import { PlatformAuthDOMHandler } from "../../src/broker/nativeBroker/PlatformAuthDOMHandler.js";
122120
import { config } from "process";
@@ -832,7 +830,10 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
832830
pca.handleRedirectPromise().catch((e) => {
833831
expect(e).toMatchObject(testError);
834832
expect(window.localStorage.length).toEqual(0);
835-
expect(window.sessionStorage.length).toEqual(0);
833+
expect(window.sessionStorage.length).toEqual(1);
834+
expect(
835+
window.sessionStorage.getItem(StaticCacheKeys.VERSION)
836+
).toEqual(version); // Validate that the one item in sessionStorage is what we expect
836837
done();
837838
});
838839
});
@@ -2104,7 +2105,10 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
21042105
expect(redirectClientSpy).toHaveBeenCalledTimes(1);
21052106
expect(browserStorage.isInteractionInProgress()).toBe(false);
21062107
expect(window.localStorage.length).toBe(0);
2107-
expect(window.sessionStorage.length).toBe(0);
2108+
expect(window.sessionStorage.length).toBe(1);
2109+
expect(
2110+
window.sessionStorage.getItem(StaticCacheKeys.VERSION)
2111+
).toEqual(version); // Validate that the one item in sessionStorage is what we expect
21082112
done();
21092113
});
21102114
});
@@ -5480,7 +5484,10 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
54805484
});
54815485
} catch (e) {
54825486
// Test that error was cached for telemetry purposes and then thrown
5483-
expect(window.sessionStorage).toHaveLength(1);
5487+
expect(window.sessionStorage).toHaveLength(2);
5488+
expect(
5489+
window.sessionStorage.getItem(StaticCacheKeys.VERSION)
5490+
).toEqual(version);
54845491
const failures = window.sessionStorage.getItem(
54855492
`server-telemetry-${TEST_CONFIG.MSAL_CLIENT_ID}`
54865493
);
@@ -5536,7 +5543,10 @@ describe("PublicClientApplication.ts Class Unit Tests", () => {
55365543
await silentRequest3.catch(() => {});
55375544
// Test that error was cached for telemetry purposes and then thrown
55385545
expect(atsSpy).toHaveBeenCalledTimes(1);
5539-
expect(window.sessionStorage).toHaveLength(1);
5546+
expect(window.sessionStorage).toHaveLength(2);
5547+
expect(
5548+
window.sessionStorage.getItem(StaticCacheKeys.VERSION)
5549+
).toEqual(version);
55405550
const failures = window.sessionStorage.getItem(
55415551
`server-telemetry-${TEST_CONFIG.MSAL_CLIENT_ID}`
55425552
);

‎lib/msal-browser/test/cache/BrowserCacheManager.spec.ts‎

Lines changed: 82 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -16,9 +16,7 @@ import {
1616
import { CacheOptions } from "../../src/config/Configuration.js";
1717
import {
1818
Constants,
19-
PersistentCacheKeys,
2019
CommonAuthorizationCodeRequest as AuthorizationCodeRequest,
21-
ProtocolUtils,
2220
Logger,
2321
LogLevel,
2422
AuthenticationScheme,
@@ -41,18 +39,18 @@ import {
4139
import {
4240
BrowserCacheLocation,
4341
INTERACTION_TYPE,
44-
InteractionType,
42+
StaticCacheKeys,
4543
TemporaryCacheKeys,
4644
} from "../../src/utils/BrowserConstants.js";
4745
import { CryptoOps } from "../../src/crypto/CryptoOps.js";
4846
import { DatabaseStorage } from "../../src/cache/DatabaseStorage.js";
4947
import { BrowserCacheManager } from "../../src/cache/BrowserCacheManager.js";
50-
import { BrowserStateObject } from "../../src/utils/BrowserProtocolUtils.js";
5148
import { base64Decode } from "../../src/encode/Base64Decode.js";
5249
import { getDefaultPerformanceClient } from "../utils/TelemetryUtils.js";
5350
import { BrowserPerformanceClient } from "../../src/telemetry/BrowserPerformanceClient.js";
5451
import { CookieStorage } from "../../src/cache/CookieStorage.js";
5552
import { EventHandler } from "../../src/event/EventHandler.js";
53+
import { version } from "../../src/packageMetadata.js";
5654

5755
describe("BrowserCacheManager tests", () => {
5856
let cacheConfig: Required<CacheOptions>;
@@ -140,6 +138,85 @@ describe("BrowserCacheManager tests", () => {
140138
});
141139
});
142140

141+
describe("initialize", () => {
142+
it("sets MSAL version in localStorage if not already set", async () => {
143+
const browserCacheManager = new BrowserCacheManager(
144+
TEST_CONFIG.MSAL_CLIENT_ID,
145+
{
146+
...cacheConfig,
147+
cacheLocation: BrowserCacheLocation.LocalStorage,
148+
},
149+
browserCrypto,
150+
logger,
151+
new StubPerformanceClient(),
152+
new EventHandler()
153+
);
154+
await browserCacheManager.initialize(TEST_CONFIG.CORRELATION_ID);
155+
expect(window.localStorage.getItem(StaticCacheKeys.VERSION)).toBe(
156+
version
157+
);
158+
});
159+
160+
it("sets MSAL version in localStorage if previous version doesn't match", async () => {
161+
window.localStorage.setItem(StaticCacheKeys.VERSION, "1.0.0");
162+
const browserCacheManager = new BrowserCacheManager(
163+
TEST_CONFIG.MSAL_CLIENT_ID,
164+
{
165+
...cacheConfig,
166+
cacheLocation: BrowserCacheLocation.LocalStorage,
167+
},
168+
browserCrypto,
169+
logger,
170+
new StubPerformanceClient(),
171+
new EventHandler()
172+
);
173+
await browserCacheManager.initialize(TEST_CONFIG.CORRELATION_ID);
174+
expect(window.localStorage.getItem(StaticCacheKeys.VERSION)).toBe(
175+
version
176+
);
177+
});
178+
179+
it("does not set MSAL version in localStorage if existing version already matches", async () => {
180+
// First make sure the version gets set
181+
const browserCacheManager1 = new BrowserCacheManager(
182+
TEST_CONFIG.MSAL_CLIENT_ID,
183+
{
184+
...cacheConfig,
185+
cacheLocation: BrowserCacheLocation.LocalStorage,
186+
},
187+
browserCrypto,
188+
logger,
189+
new StubPerformanceClient(),
190+
new EventHandler()
191+
);
192+
await browserCacheManager1.initialize(TEST_CONFIG.CORRELATION_ID);
193+
expect(window.localStorage.getItem(StaticCacheKeys.VERSION)).toBe(
194+
version
195+
);
196+
197+
const setSpy = jest.spyOn(Storage.prototype, "setItem");
198+
const browserCacheManager2 = new BrowserCacheManager(
199+
TEST_CONFIG.MSAL_CLIENT_ID,
200+
{
201+
...cacheConfig,
202+
cacheLocation: BrowserCacheLocation.LocalStorage,
203+
},
204+
browserCrypto,
205+
logger,
206+
new StubPerformanceClient(),
207+
new EventHandler()
208+
);
209+
await browserCacheManager2.initialize(TEST_CONFIG.CORRELATION_ID);
210+
expect(window.localStorage.getItem(StaticCacheKeys.VERSION)).toBe(
211+
version
212+
);
213+
expect(setSpy).not.toHaveBeenCalledWith(
214+
StaticCacheKeys.VERSION,
215+
expect.anything()
216+
);
217+
});
218+
});
219+
143220
describe("Interface functions", () => {
144221
let browserSessionStorage: BrowserCacheManager;
145222
let authority: Authority;
@@ -251,6 +328,7 @@ describe("BrowserCacheManager tests", () => {
251328
expect(browserLocalStorage.getKeys()).toEqual([
252329
"msal.account.keys",
253330
`msal.token.keys.${TEST_CONFIG.MSAL_CLIENT_ID}`,
331+
StaticCacheKeys.VERSION,
254332
msalCacheKey,
255333
msalCacheKey2,
256334
]);

‎lib/msal-browser/test/interaction_client/PopupClient.spec.ts‎

Lines changed: 6 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,6 +43,7 @@ import {
4343
TemporaryCacheKeys,
4444
ApiId,
4545
BrowserConstants,
46+
StaticCacheKeys,
4647
} from "../../src/utils/BrowserConstants.js";
4748
import * as BrowserCrypto from "../../src/crypto/BrowserCrypto.js";
4849
import * as PkceGenerator from "../../src/crypto/PkceGenerator.js";
@@ -66,6 +67,7 @@ import * as BrowserUtils from "../../src/utils/BrowserUtils.js";
6667
import { FetchClient } from "../../src/network/FetchClient.js";
6768
import { TestTimeUtils } from "msal-test-utils";
6869
import { PopupRequest } from "../../src/request/PopupRequest.js";
70+
import { version } from "../../src/packageMetadata.js";
6971

7072
const testPopupWondowDefaults = {
7173
height: BrowserConstants.POPUP_HEIGHT,
@@ -814,7 +816,10 @@ describe("PopupClient", () => {
814816
});
815817
} catch (e) {
816818
// Test that error was cached for telemetry purposes and then thrown
817-
expect(window.sessionStorage).toHaveLength(1);
819+
expect(window.sessionStorage).toHaveLength(2);
820+
expect(
821+
window.sessionStorage.getItem(StaticCacheKeys.VERSION)
822+
).toEqual(version);
818823
const failures = window.sessionStorage.getItem(
819824
`server-telemetry-${TEST_CONFIG.MSAL_CLIENT_ID}`
820825
);

‎lib/msal-browser/test/interaction_client/RedirectClient.spec.ts‎

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,7 @@ import {
5757
ApiId,
5858
BrowserCacheLocation,
5959
InteractionType,
60+
StaticCacheKeys,
6061
} from "../../src/utils/BrowserConstants.js";
6162
import { base64Encode } from "../../src/encode/Base64Encode.js";
6263
import { FetchClient } from "../../src/network/FetchClient.js";
@@ -87,6 +88,7 @@ import {
8788
TestTimeUtils,
8889
} from "msal-test-utils";
8990
import { BrowserPerformanceClient } from "../../src/telemetry/BrowserPerformanceClient.js";
91+
import { version } from "../../src/packageMetadata.js";
9092

9193
const cacheConfig = {
9294
cacheLocation: BrowserCacheLocation.SessionStorage,
@@ -205,7 +207,10 @@ describe("RedirectClient", () => {
205207
.then((response) => {
206208
expect(response).toBe(null);
207209
expect(window.localStorage.length).toEqual(0);
208-
expect(window.sessionStorage.length).toEqual(0);
210+
expect(window.sessionStorage.length).toEqual(1);
211+
expect(
212+
window.sessionStorage.getItem(StaticCacheKeys.VERSION)
213+
).toEqual(version); // Validate that the one item in sessionStorage is what we expect
209214
done();
210215
});
211216
});
@@ -223,7 +228,10 @@ describe("RedirectClient", () => {
223228
.then((response) => {
224229
expect(response).toBe(null);
225230
expect(window.localStorage.length).toEqual(0);
226-
expect(window.sessionStorage.length).toEqual(0);
231+
expect(window.sessionStorage.length).toEqual(1);
232+
expect(
233+
window.sessionStorage.getItem(StaticCacheKeys.VERSION)
234+
).toEqual(version); // Validate that the one item in sessionStorage is what we expect
227235
done();
228236
});
229237
});
@@ -241,7 +249,10 @@ describe("RedirectClient", () => {
241249
.then((response) => {
242250
expect(response).toBe(null);
243251
expect(window.localStorage.length).toEqual(0);
244-
expect(window.sessionStorage.length).toEqual(0);
252+
expect(window.sessionStorage.length).toEqual(1);
253+
expect(
254+
window.sessionStorage.getItem(StaticCacheKeys.VERSION)
255+
).toEqual(version); // Validate that the one item in sessionStorage is what we expect
245256
expect(window.location.hash).toEqual(
246257
TEST_HASHES.TEST_SUCCESS_CODE_HASH_POPUP
247258
);

0 commit comments

Comments
 (0)