-
Notifications
You must be signed in to change notification settings - Fork 4.6k
chore: Add an event queue in segment #39411
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
Changes from 3 commits
5377c76
ad5223a
5e78c78
18fc4c5
1429f4f
5fc39b9
b88797c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,222 @@ | ||
| import SegmentSingleton from "./segment"; | ||
| import { getAppsmithConfigs } from "ee/configs"; | ||
| import log from "loglevel"; | ||
|
|
||
| // Mock external dependencies | ||
| jest.mock("ee/configs"); | ||
| jest.mock("loglevel"); | ||
| jest.mock("@segment/analytics-next", () => ({ | ||
| AnalyticsBrowser: { | ||
| load: jest.fn(), | ||
| }, | ||
| })); | ||
|
|
||
| // Mock implementations | ||
| const mockAnalytics = { | ||
| track: jest.fn(), | ||
| identify: jest.fn(), | ||
| addSourceMiddleware: jest.fn(), | ||
| reset: jest.fn(), | ||
| user: jest.fn(), | ||
| }; | ||
|
|
||
| const mockAnalyticsBrowser = { | ||
| load: jest.fn().mockResolvedValue([mockAnalytics]), | ||
| }; | ||
|
|
||
| // Setup before each test | ||
| beforeEach(() => { | ||
| jest.clearAllMocks(); | ||
|
|
||
| // Reset singleton instance | ||
| (SegmentSingleton as unknown as { instance: unknown }).instance = undefined; | ||
|
|
||
| // Default mock for getAppsmithConfigs | ||
| (getAppsmithConfigs as jest.Mock).mockReturnValue({ | ||
| segment: { | ||
| enabled: true, | ||
| apiKey: "test-api-key", | ||
| ceKey: "test-ce-key", | ||
| }, | ||
| }); | ||
|
|
||
| // Set up AnalyticsBrowser mock | ||
| // eslint-disable-next-line @typescript-eslint/no-var-requires | ||
| require("@segment/analytics-next").AnalyticsBrowser = mockAnalyticsBrowser; | ||
| }); | ||
|
|
||
| describe("SegmentSingleton", () => { | ||
| describe("getInstance", () => { | ||
| it("should return the same instance when called multiple times", () => { | ||
| const instance1 = SegmentSingleton.getInstance(); | ||
| const instance2 = SegmentSingleton.getInstance(); | ||
|
|
||
| expect(instance1).toBe(instance2); | ||
| }); | ||
| }); | ||
|
|
||
| describe("init", () => { | ||
| it("should initialize successfully with API key", async () => { | ||
| const segment = SegmentSingleton.getInstance(); | ||
| const result = await segment.init(); | ||
|
|
||
| expect(result).toBe(true); | ||
| expect(mockAnalyticsBrowser.load).toHaveBeenCalledWith( | ||
| { writeKey: "test-api-key" }, | ||
| expect.any(Object), | ||
| ); | ||
| }); | ||
|
|
||
| it("should not initialize when segment is disabled", async () => { | ||
| (getAppsmithConfigs as jest.Mock).mockReturnValue({ | ||
| segment: { enabled: false }, | ||
| }); | ||
|
|
||
| const segment = SegmentSingleton.getInstance(); | ||
| const result = await segment.init(); | ||
|
|
||
| expect(result).toBe(true); | ||
| expect(mockAnalyticsBrowser.load).not.toHaveBeenCalled(); | ||
| }); | ||
|
|
||
| it("should use ceKey when apiKey is not available", async () => { | ||
| (getAppsmithConfigs as jest.Mock).mockReturnValue({ | ||
| segment: { | ||
| enabled: true, | ||
| apiKey: "", | ||
| ceKey: "test-ce-key", | ||
| }, | ||
| }); | ||
|
|
||
| const segment = SegmentSingleton.getInstance(); | ||
| const result = await segment.init(); | ||
|
|
||
| expect(result).toBe(true); | ||
| expect(mockAnalyticsBrowser.load).toHaveBeenCalledWith( | ||
| { writeKey: "test-ce-key" }, | ||
| expect.any(Object), | ||
| ); | ||
| }); | ||
| }); | ||
|
|
||
| describe("track", () => { | ||
| it("should queue events when not initialized", () => { | ||
| const segment = SegmentSingleton.getInstance(); | ||
| const eventData = { test: "data" }; | ||
|
|
||
| segment.track("test-event", eventData); | ||
|
|
||
| expect(mockAnalytics.track).not.toHaveBeenCalled(); | ||
| expect(log.debug).toHaveBeenCalledWith( | ||
| "Event queued for later processing", | ||
| "test-event", | ||
| eventData, | ||
| ); | ||
| }); | ||
|
|
||
| it("should process queued events after initialization", async () => { | ||
| const segment = SegmentSingleton.getInstance(); | ||
| const eventData = { test: "data" }; | ||
|
|
||
| segment.track("test-event", eventData); | ||
| await segment.init(); | ||
|
|
||
| expect(mockAnalytics.track).toHaveBeenCalledWith("test-event", eventData); | ||
| }); | ||
|
|
||
| it("should track events directly when initialized", async () => { | ||
| const segment = SegmentSingleton.getInstance(); | ||
|
|
||
| await segment.init(); | ||
|
|
||
| const eventData = { test: "data" }; | ||
|
|
||
| segment.track("test-event", eventData); | ||
|
|
||
| expect(mockAnalytics.track).toHaveBeenCalledWith("test-event", eventData); | ||
| }); | ||
| }); | ||
|
|
||
| describe("identify", () => { | ||
| it("should call analytics identify when initialized", async () => { | ||
| const segment = SegmentSingleton.getInstance(); | ||
|
|
||
| await segment.init(); | ||
|
|
||
| const userId = "test-user"; | ||
| const traits = { name: "Test User" }; | ||
|
|
||
| await segment.identify(userId, traits); | ||
|
|
||
| expect(mockAnalytics.identify).toHaveBeenCalledWith(userId, traits); | ||
| }); | ||
| }); | ||
|
|
||
| describe("reset", () => { | ||
| it("should call analytics reset when initialized", async () => { | ||
| const segment = SegmentSingleton.getInstance(); | ||
|
|
||
| await segment.init(); | ||
|
|
||
| segment.reset(); | ||
|
|
||
| expect(mockAnalytics.reset).toHaveBeenCalled(); | ||
| }); | ||
| }); | ||
|
|
||
| describe("error handling", () => { | ||
| it("should handle initialization failure", async () => { | ||
| mockAnalyticsBrowser.load.mockRejectedValueOnce(new Error("Init failed")); | ||
|
|
||
| const segment = SegmentSingleton.getInstance(); | ||
| const result = await segment.init(); | ||
|
|
||
| expect(result).toBe(false); | ||
| expect(log.error).toHaveBeenCalledWith( | ||
| "Failed to initialize Segment:", | ||
| expect.any(Error), | ||
| ); | ||
| }); | ||
| }); | ||
| describe("avoidTracking", () => { | ||
| it("should not track events after avoidTracking is called", async () => { | ||
| const segment = SegmentSingleton.getInstance(); | ||
| await segment.init(); | ||
|
|
||
| // Track an event before calling avoidTracking | ||
| segment.track("pre-avoid-event", { data: "value" }); | ||
| expect(mockAnalytics.track).toHaveBeenCalledTimes(1); | ||
|
|
||
| // Call avoidTracking | ||
| segment.avoidTracking(); | ||
|
|
||
| // Track an event after calling avoidTracking | ||
| segment.track("post-avoid-event", { data: "value" }); | ||
|
|
||
| // Should still have only been called once (from the first event) | ||
| expect(mockAnalytics.track).toHaveBeenCalledTimes(1); | ||
| expect(log.debug).toHaveBeenCalledWith( | ||
| expect.stringContaining("Event logged locally"), | ||
| "post-avoid-event", | ||
| { data: "value" } | ||
| ); | ||
| }); | ||
|
|
||
| it("should flush queued events when avoidTracking is called before initialization", async () => { | ||
| const segment = SegmentSingleton.getInstance(); | ||
|
|
||
| // Queue some events | ||
| segment.track("queued-event-1", { data: "value1" }); | ||
| segment.track("queued-event-2", { data: "value2" }); | ||
|
|
||
| // Call avoidTracking before initialization | ||
| segment.avoidTracking(); | ||
|
|
||
| // Initialize | ||
| await segment.init(); | ||
|
|
||
| // Analytics track should not be called since we're avoiding tracking | ||
| expect(mockAnalytics.track).not.toHaveBeenCalled(); | ||
| }); | ||
| }); | ||
| }); | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -7,9 +7,18 @@ import { | |
| import { getAppsmithConfigs } from "ee/configs"; | ||
| import log from "loglevel"; | ||
|
|
||
| enum SegmentInitState { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Just a bit of nit picking. I feel that
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Done |
||
| WAITING = "waiting", | ||
| INITIALIZED = "initialized", | ||
| FAILED = "failed", | ||
| NOT_REQUIRED = "not_required", | ||
| } | ||
|
|
||
| class SegmentSingleton { | ||
| private static instance: SegmentSingleton; | ||
| private analytics: Analytics | null = null; | ||
| private eventQueue: Array<{ name: string; data: EventProperties }> = []; | ||
| private initState: SegmentInitState = SegmentInitState.WAITING; | ||
|
|
||
| public static getInstance(): SegmentSingleton { | ||
| if (!SegmentSingleton.instance) { | ||
|
|
@@ -43,6 +52,8 @@ class SegmentSingleton { | |
| const { segment } = getAppsmithConfigs(); | ||
|
|
||
| if (!segment.enabled) { | ||
| this.avoidTracking(); | ||
|
|
||
| return true; | ||
| } | ||
|
|
||
|
|
@@ -56,6 +67,7 @@ class SegmentSingleton { | |
|
|
||
| if (!writeKey) { | ||
| log.error("Segment key was not found."); | ||
| this.avoidTracking(); | ||
|
|
||
| return true; | ||
| } | ||
|
|
@@ -80,21 +92,43 @@ class SegmentSingleton { | |
| ); | ||
|
|
||
| this.analytics = analytics; | ||
| this.initState = SegmentInitState.INITIALIZED; | ||
| // Process queued events after successful initialization | ||
| this.processEventQueue(); | ||
|
|
||
| return true; | ||
| } catch (error) { | ||
| log.error("Failed to initialize Segment:", error); | ||
| // Clear the queue if error occurred in init | ||
| this.flushEventQueue(); | ||
| this.initState = SegmentInitState.FAILED; | ||
|
|
||
| return false; | ||
| } | ||
| } | ||
|
|
||
| private processEventQueue() { | ||
| while (this.eventQueue.length > 0) { | ||
| const event = this.eventQueue.shift(); | ||
|
|
||
| if (event) { | ||
| this.track(event.name, event.data); | ||
| } | ||
| } | ||
| } | ||
|
|
||
| private flushEventQueue() { | ||
| this.eventQueue = []; | ||
| } | ||
|
|
||
| public track(eventName: string, eventData: EventProperties) { | ||
| // In scenarios where segment was never initialised, we are logging the event locally | ||
| // This is done so that we can debug event logging locally | ||
| if (this.analytics) { | ||
| log.debug("Event fired", eventName, eventData); | ||
| this.analytics.track(eventName, eventData); | ||
| } else if (this.initState === SegmentInitState.WAITING) { | ||
| // Only queue events if we're in WAITING state | ||
| this.eventQueue.push({ name: eventName, data: eventData }); | ||
| log.debug("Event queued for later processing", eventName, eventData); | ||
| } else { | ||
| log.debug("Event fired locally", eventName, eventData); | ||
| } | ||
|
|
@@ -112,6 +146,11 @@ class SegmentSingleton { | |
| } | ||
| } | ||
|
|
||
| public avoidTracking() { | ||
| this.initState = SegmentInitState.NOT_REQUIRED; | ||
| this.flushEventQueue(); | ||
| } | ||
|
|
||
| public reset() { | ||
| if (this.analytics) { | ||
| this.analytics.reset(); | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.