Skip to content

Commit

Permalink
Fix for incorrect metric causing high cardinality (#192)
Browse files Browse the repository at this point in the history
  • Loading branch information
sioked authored Mar 14, 2024
1 parent 055b024 commit 7f5214b
Show file tree
Hide file tree
Showing 2 changed files with 44 additions and 21 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).toString(),
jsSdkLibrary,
pageType,
token: getTokenType(),
Expand Down
38 changes: 38 additions & 0 deletions src/tracking.test.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
/* @flow */
/* eslint-disable promise/no-native, no-restricted-globals, compat/compat */
import { describe, it, vi, expect } from "vitest";

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((resolve) => setTimeout(resolve)); // Flush promises

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 7f5214b

Please sign in to comment.