Skip to content

Commit

Permalink
Fix for incorrect metric causing high cardinality
Browse files Browse the repository at this point in the history
  • Loading branch information
sioked committed Mar 14, 2024
1 parent 055b024 commit 63bfbe0
Show file tree
Hide file tree
Showing 2 changed files with 45 additions and 22 deletions.
27 changes: 6 additions & 21 deletions src/tracking.js
Original file line number Diff line number Diff line change
Expand Up @@ -171,26 +171,11 @@ export function setupLogger() {
);

if (loadTime) {
logger
// We can not send gauge metrics to our logger backend currently
// once we have that ability, we should uncomment this gauge metric
// .metricGauge({
// namespace: "sdk_client.init.gauge",
// event: "load_performance",
// value: sdkLoadTime,
// dimensions: {
// cacheType,
// version,
// components: getComponents().join(","),
// isPayPalDomain: isLoadedInFrame,
// token: getTokenType(),
// },
// })
.track({
[FPTI_KEY.TRANSITION]: "process_js_sdk_init_client",
[FPTI_KEY.SDK_LOAD_TIME]: sdkLoadTime,
[FPTI_KEY.SDK_CACHE]: cacheType,
});
logger.track({
[FPTI_KEY.TRANSITION]: "process_js_sdk_init_client",
[FPTI_KEY.SDK_LOAD_TIME]: sdkLoadTime,
[FPTI_KEY.SDK_CACHE]: cacheType,
});
}

if (isIEIntranet()) {
Expand All @@ -203,7 +188,7 @@ export function setupLogger() {
dimensions: {
components: getComponents().join(","),
integrationSource,
isPayPalDomain: isLoadedInFrame,
isPayPalDomain: Boolean(isLoadedInFrame),
jsSdkLibrary,
pageType,
token: getTokenType(),
Expand Down
40 changes: 39 additions & 1 deletion src/tracking.test.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
/* @flow */
import { describe, it, vi, expect } from "vitest";
/* eslint-disable promise/no-native, no-restricted-globals, compat/compat */
import { describe, it, vi, expect, beforeEach } from "vitest";

Check failure on line 3 in src/tracking.test.js

View workflow job for this annotation

GitHub Actions / main

'beforeEach' is defined but never used

import { makeMockScriptElement } from "../test/helpers";

Expand All @@ -8,13 +9,34 @@ import { getSDKInitTime, setupLogger } from "./tracking";
const clientId = "foobar123";
const mockScriptSrc = `https://test.paypal.com/sdk/js?client-id=${clientId}`;

let metricCounterSpy;
let windowReadyPromiseResolver;
vi.mock("./logger", async () => {
const actual = await vi.importActual("./logger");
return {
...actual,
getLogger: vi.fn(() => {
const logger = actual.getLogger();
metricCounterSpy = vi
.spyOn(logger, "metricCounter")
.mockImplementation(() => {}); // eslint-disable-line no-empty-function
return logger;
}),
};
});
vi.mock("@krakenjs/belter/src", async () => {
const actual = await vi.importActual("@krakenjs/belter/src");

return {
...actual,
getCurrentScript: vi.fn(() => {
return makeMockScriptElement(mockScriptSrc);
}),
waitForWindowReady: vi.fn(() => {
return new Promise((resolve) => {
windowReadyPromiseResolver = resolve;
});
}),
};
});

Expand All @@ -33,4 +55,20 @@ describe(`tracking cases`, () => {
window.document.documentMode = true;
setupLogger();
});

it("should call metric counter on window load", async () => {
windowReadyPromiseResolver();
await new Promise((r) => setTimeout(r)); //Flush promises

Check failure on line 61 in src/tracking.test.js

View workflow job for this annotation

GitHub Actions / main

Promise constructor parameters must be named to match "^_?resolve$"

Check failure on line 61 in src/tracking.test.js

View workflow job for this annotation

GitHub Actions / main

Expected space or tab after '//' in comment

expect(metricCounterSpy).toHaveBeenCalledWith(
expect.objectContaining({
namespace: "sdk_client.init.count",
dimensions: expect.objectContaining({
isPayPalDomain: false,
}),
})
);
});
});

/* eslint-enable promise/no-native, no-restricted-globals, compat/compat */

0 comments on commit 63bfbe0

Please sign in to comment.