Skip to content
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.

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

Product analtyics improvements #4794

Merged
merged 2 commits into from
Sep 12, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
61 changes: 59 additions & 2 deletions app/packages/analytics/src/analytics.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,6 @@ describe("Analytics", () => {
// Mock return value of AnalyticsBrowser.load
AnalyticsBrowser.load.mockReturnValue(mockSegment);
analytics = new Analytics({
eventRateLimit: 5,
debounceInterval: 5000,
});
});
Expand Down Expand Up @@ -78,6 +77,7 @@ describe("Analytics", () => {
analytics.track("debounced_event");
expect(mockSegment.track).toHaveBeenCalledWith(
"debounced_event",
undefined,
undefined
);

Expand Down Expand Up @@ -124,7 +124,6 @@ describe("Analytics", () => {
});

it("should not log debug information when debug mode is disabled", () => {
analytics.load(SIMPLE_CONFIG);
const consoleSpy = vi.spyOn(console, "log").mockImplementation(() => {});
analytics = new Analytics({
writeKey: "test",
Expand All @@ -138,4 +137,62 @@ describe("Analytics", () => {

consoleSpy.mockRestore();
});

it("should allow disabling of url tracking", () => {
analytics = new Analytics();
analytics.load({
writeKey: "test",
userId: "user",
userGroup: "group",
debug: false,
disableUrlTracking: true,
});
analytics.track("custom_event");
// segment should be called with context.page.url = undefined
expect(mockSegment.track).toHaveBeenCalledWith("custom_event", undefined, {
context: {
page: { url: undefined },
},
});
});

it("should obfuscate uri properties of all events", () => {
analytics = new Analytics();
analytics.load({
writeKey: "test",
userId: "user",
userGroup: "group",
debug: false,
redact: ["uri"],
});
analytics.track("random_event", { uri: "@my_name/my_plugin/my_operator" });
// segment should be called with properties.uri = "<redacted>"
console.log(mockSegment.track.mock.calls[0]);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
console.log(mockSegment.track.mock.calls[0]);

expect(mockSegment.track).toHaveBeenCalledWith(
"random_event",
{ uri: "<redacted>" },
undefined
);
});

it("should redact properties properly", () => {
analytics = new Analytics();
analytics.load({
writeKey: "test",
userId: "user",
userGroup: "group",
debug: false,
redact: ["uri"],
});
const redacted = analytics.redact({
uri: "@my_name/my_plugin/my_operator",
});
expect(redacted).toEqual({ uri: "<redacted>" });
const redacted2 = analytics.redact({ other: "value" });
expect(redacted2).toEqual({ other: "value" });
const redacted3 = analytics.redact({});
expect(redacted3).toEqual({});
const redacted4 = analytics.redact(undefined);
expect(redacted4).toEqual(undefined);
});
});
28 changes: 27 additions & 1 deletion app/packages/analytics/src/usingAnalytics.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ export type AnalyticsInfo = {
userGroup: string;
doNotTrack?: boolean;
debug: boolean;
disableUrlTracking?: boolean;
redact?: string[];
};

export type AnalyticsConfig = {
Expand All @@ -29,13 +31,25 @@ export class Analytics {
private _debug = false;
private _lastEventTimestamps: Record<string, number> = {}; // Tracks last event times
private _debounceInterval = 1000; // Default debounce interval in milliseconds (5 seconds)
private _disableUrlTracking = false;
private _redactedProperties: string[] = [];

constructor(config?: AnalyticsConfig) {
if (config?.debounceInterval) {
this._debounceInterval = config.debounceInterval;
}
}

redact(properties: Record<string, any>) {
if (!properties) return properties;
return Object.keys(properties).reduce((acc, key) => {
if (this._redactedProperties.includes(key)) {
acc[key] = "<redacted>";
}
return acc;
}, properties);
}

load(info: AnalyticsInfo) {
if (this._segment) return;
this._debug = info?.debug;
Expand All @@ -45,6 +59,10 @@ export class Analytics {
this.disable();
return;
}
if (info.redact) {
this._redactedProperties = info.redact;
}
this._disableUrlTracking = info.disableUrlTracking;
if (!info.writeKey) {
console.warn("Analytics disabled (no write key)");
this.disable();
Expand All @@ -71,12 +89,14 @@ export class Analytics {

page(name?: string, properties?: {}) {
if (!this._segment) return;
properties = this.redact(properties);
this._segment.page(name, properties);
}

track(name: string, properties?: {}) {
const now = Date.now();
const lastTimestamp = this._lastEventTimestamps[name] || 0;
properties = this.redact(properties);

if (now - lastTimestamp < this._debounceInterval) {
if (this._debug) {
Expand All @@ -92,7 +112,11 @@ export class Analytics {
}

if (!this._segment) return;
this._segment.track(name, properties);
let opts;
if (this._disableUrlTracking) {
opts = { context: { page: { url: undefined } } };
}
this._segment.track(name, properties, opts);
}

trackEvent(name: string, properties?: {}) {
Expand All @@ -101,11 +125,13 @@ export class Analytics {

identify(userId: string, traits?: {}) {
if (!this._segment) return;
traits = this.redact(traits);
this._segment.identify(userId, traits);
}

group(groupId: string, traits?: {}) {
if (!this._segment) return;
traits = this.redact(traits);
this._segment.group(groupId, traits);
}
}
4 changes: 2 additions & 2 deletions app/packages/app/src/components/AnalyticsConsent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -85,8 +85,8 @@ export default function AnalyticsConsent({
Help us improve FiftyOne
</Typography>
<Typography marginBottom={1}>
We use cookies to understand how FiftyOne is used and to improve the
product. You can help us by enabling analytics.
We use cookies to understand how FiftyOne is used and improve the product.
You can help us by enabling anonymous analytics.
</Typography>
<Grid container gap={2} justifyContent="end" direction="row">
<Grid item alignContent="center">
Expand Down
Loading