Skip to content

🪟 🐛 Remove reinitialization of AnalyticsService #19304

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

Merged
merged 4 commits into from
Nov 11, 2022
Merged
Show file tree
Hide file tree
Changes from 2 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
17 changes: 7 additions & 10 deletions airbyte-webapp/.storybook/withProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,14 @@ import { FeatureService } from "../src/hooks/services/Feature";
import { ConfigServiceProvider, defaultConfig } from "../src/config";
import { DocumentationPanelProvider } from "../src/views/Connector/ConnectorDocumentationLayout/DocumentationPanelContext";
import { ServicesProvider } from "../src/core/servicesProvider";
import { analyticsServiceContext, AnalyticsServiceProviderValue } from "../src/hooks/services/Analytics";
import { analyticsServiceContext } from "../src/hooks/services/Analytics";
import type { AnalyticsService } from "../src/core/analytics";

const AnalyticsContextMock: AnalyticsServiceProviderValue = {
analyticsContext: {},
setContext: () => {},
addContextProps: () => {},
removeContextProps: () => {},
service: {
const analyticsContextMock: AnalyticsService = {
track: () => {},
},
} as unknown as AnalyticsServiceProviderValue;
setContext: () => {},
removeFromContext: () => {},
} as unknown as AnalyticsService;

const queryClient = new QueryClient({
defaultOptions: {
Expand All @@ -35,7 +32,7 @@ const queryClient = new QueryClient({

export const withProviders = (getStory) => (
<React.Suspense fallback={null}>
<analyticsServiceContext.Provider value={AnalyticsContextMock}>
<analyticsServiceContext.Provider value={analyticsContextMock}>
<QueryClientProvider client={queryClient}>
<ServicesProvider>
<MemoryRouter>
Expand Down
11 changes: 6 additions & 5 deletions airbyte-webapp/src/core/analytics/AnalyticsService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,13 +26,13 @@ describe("AnalyticsService", () => {
});

it("should send events to segment", () => {
const service = new AnalyticsService({});
const service = new AnalyticsService();
service.track(Namespace.CONNECTION, Action.CREATE, {});
expect(window.analytics.track).toHaveBeenCalledWith("Airbyte.UI.Connection.Create", expect.anything());
});

it("should send version and environment for prod", () => {
const service = new AnalyticsService({}, "0.42.13");
const service = new AnalyticsService("0.42.13");
service.track(Namespace.CONNECTION, Action.CREATE, {});
expect(window.analytics.track).toHaveBeenCalledWith(
expect.anything(),
Expand All @@ -41,7 +41,7 @@ describe("AnalyticsService", () => {
});

it("should send version and environment for dev", () => {
const service = new AnalyticsService({}, "dev");
const service = new AnalyticsService("dev");
service.track(Namespace.CONNECTION, Action.CREATE, {});
expect(window.analytics.track).toHaveBeenCalledWith(
expect.anything(),
Expand All @@ -50,7 +50,7 @@ describe("AnalyticsService", () => {
});

it("should pass parameters to segment event", () => {
const service = new AnalyticsService({});
const service = new AnalyticsService();
service.track(Namespace.CONNECTION, Action.CREATE, { actionDescription: "Created new connection" });
expect(window.analytics.track).toHaveBeenCalledWith(
expect.anything(),
Expand All @@ -59,7 +59,8 @@ describe("AnalyticsService", () => {
});

it("should pass context parameters to segment event", () => {
const service = new AnalyticsService({ context: 42 });
const service = new AnalyticsService();
service.setContext({ context: 42 });
service.track(Namespace.CONNECTION, Action.CREATE, { actionDescription: "Created new connection" });
expect(window.analytics.track).toHaveBeenCalledWith(
expect.anything(),
Expand Down
17 changes: 16 additions & 1 deletion airbyte-webapp/src/core/analytics/AnalyticsService.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,25 @@
import { Action, EventParams, Namespace } from "./types";

type Context = Record<string, unknown>;

export class AnalyticsService {
constructor(private context: Record<string, unknown>, private version?: string) {}
private context: Context = {};

constructor(private version?: string) {}

private getSegmentAnalytics = (): SegmentAnalytics.AnalyticsJS | undefined => window.analytics;

public setContext(context: Context) {
this.context = {
...this.context,
...context,
};
}

public removeFromContext(...keys: string[]) {
keys.forEach((key) => delete this.context[key]);
}

alias = (newId: string): void => this.getSegmentAnalytics()?.alias?.(newId);

page = (name: string): void => this.getSegmentAnalytics()?.page?.(name, { ...this.context });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -8,5 +8,7 @@ export const useAnalyticsService = (): AnalyticsService => {
track: jest.fn(),
identify: jest.fn(),
group: jest.fn(),
setContext: jest.fn(),
removeFromContext: jest.fn(),
} as unknown as AnalyticsService;
};
62 changes: 13 additions & 49 deletions airbyte-webapp/src/hooks/services/Analytics/useAnalyticsService.tsx
Original file line number Diff line number Diff line change
@@ -1,62 +1,26 @@
import React, { useContext, useEffect, useMemo } from "react";
import { useMap } from "react-use";
import React, { useContext, useEffect, useRef } from "react";

import { useConfig } from "config";
import { AnalyticsService } from "core/analytics/AnalyticsService";

type AnalyticsContext = Record<string, unknown>;

export interface AnalyticsServiceProviderValue {
analyticsContext: AnalyticsContext;
setContext: (ctx: AnalyticsContext) => void;
addContextProps: (props: AnalyticsContext) => void;
removeContextProps: (props: string[]) => void;
service: AnalyticsService;
}

export const analyticsServiceContext = React.createContext<AnalyticsServiceProviderValue | null>(null);

const AnalyticsServiceProvider = ({
children,
version,
initialContext = {},
}: {
children: React.ReactNode;
version?: string;
initialContext?: AnalyticsContext;
}) => {
const [analyticsContext, { set, setAll, remove }] = useMap(initialContext);

const analyticsService: AnalyticsService = useMemo(
() => new AnalyticsService(analyticsContext, version),
[version, analyticsContext]
);
export const analyticsServiceContext = React.createContext<AnalyticsService | null>(null);

const handleAddContextProps = (props: AnalyticsContext) => {
Object.entries(props).forEach((value) => set(...value));
};
const AnalyticsServiceProvider: React.FC<React.PropsWithChildren<unknown>> = ({ children }) => {
const { version } = useConfig();
const analyticsService = useRef<AnalyticsService>();

const handleRemoveContextProps = (props: string[]) => props.forEach(remove);
if (!analyticsService.current) {
analyticsService.current = new AnalyticsService(version);
}

return (
<analyticsServiceContext.Provider
value={{
analyticsContext,
setContext: setAll,
addContextProps: handleAddContextProps,
removeContextProps: handleRemoveContextProps,
service: analyticsService,
}}
>
{children}
</analyticsServiceContext.Provider>
<analyticsServiceContext.Provider value={analyticsService.current}>{children}</analyticsServiceContext.Provider>
);
};

export const useAnalyticsService = (): AnalyticsService => {
return useAnalytics().service;
};

export const useAnalytics = (): AnalyticsServiceProviderValue => {
const analyticsContext = useContext(analyticsServiceContext);

if (!analyticsContext) {
Expand Down Expand Up @@ -86,15 +50,15 @@ export const useTrackPage = (page: string): void => {
};

export const useAnalyticsRegisterValues = (props?: AnalyticsContext | null): void => {
const { addContextProps, removeContextProps } = useAnalytics();
const service = useAnalyticsService();

useEffect(() => {
if (!props) {
return;
}

addContextProps(props);
return () => removeContextProps(Object.keys(props));
service.setContext(props);
return () => service.removeFromContext(...Object.keys(props));

// eslint-disable-next-line react-hooks/exhaustive-deps
}, [props]);
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,14 @@
import { useEffect } from "react";
import { useIntercom as useIntercomProvider, IntercomContextValues } from "react-use-intercom";

import { useAnalytics } from "hooks/services/Analytics";
import { useCurrentUser } from "packages/cloud/services/auth/AuthService";
import { useCurrentWorkspaceId } from "services/workspaces/WorkspacesService";

export const useIntercom = (): IntercomContextValues => {
const intercomContextValues = useIntercomProvider();

const user = useCurrentUser();
const { analyticsContext } = useAnalytics();
const workspaceId = useCurrentWorkspaceId();

useEffect(() => {
intercomContextValues.boot({
Expand All @@ -18,7 +18,7 @@ export const useIntercom = (): IntercomContextValues => {
userHash: user.intercomHash,

customAttributes: {
workspace_id: analyticsContext.workspaceId,
workspace_id: workspaceId,
},
});

Expand All @@ -29,11 +29,11 @@ export const useIntercom = (): IntercomContextValues => {
useEffect(() => {
intercomContextValues.update({
customAttributes: {
workspace_id: analyticsContext.workspace_id,
Copy link
Contributor Author

Choose a reason for hiding this comment

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

ℹ️ This likely never worked, since there was no workspace_id on that analytics context, but since it was also not typed it was never caught. Switched this to use the actual correct hook.

workspace_id: workspaceId,
},
});
// eslint-disable-next-line react-hooks/exhaustive-deps
}, [analyticsContext.workspace_id]);
}, [workspaceId]);

return intercomContextValues;
};
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ import { LoadingPage } from "components";

import { useConfig } from "config";
import { useI18nContext } from "core/i18n";
import { useAnalytics } from "hooks/services/Analytics";
import { useAnalyticsService } from "hooks/services/Analytics";
import { ExperimentProvider, ExperimentService } from "hooks/services/Experiment";
import type { Experiments } from "hooks/services/Experiment/experiments";
import { FeatureSet, useFeatureService } from "hooks/services/Feature";
Expand Down Expand Up @@ -46,7 +46,7 @@ const LDInitializationWrapper: React.FC<React.PropsWithChildren<{ apiKey: string
const ldClient = useRef<LDClient.LDClient>();
const [state, setState] = useState<LDInitState>("initializing");
const { user } = useAuthService();
const { addContextProps: addAnalyticsContext } = useAnalytics();
const analyticsService = useAnalyticsService();
const { locale } = useIntl();
const { setMessageOverwrite } = useI18nContext();

Expand Down Expand Up @@ -93,7 +93,7 @@ const LDInitializationWrapper: React.FC<React.PropsWithChildren<{ apiKey: string
// The LaunchDarkly promise resolved before the timeout, so we're good to use LD.
setState("initialized");
// Make sure enabled experiments are added to each analytics event
addAnalyticsContext({ experiments: JSON.stringify(ldClient.current?.allFlags()) });
analyticsService.setContext({ experiments: JSON.stringify(ldClient.current?.allFlags()) });
// Check for overwritten i18n messages
updateI18nMessages();
updateFeatureOverwrites(ldClient.current?.variation(FEATURE_FLAG_EXPERIMENT, ""));
Expand All @@ -118,7 +118,7 @@ const LDInitializationWrapper: React.FC<React.PropsWithChildren<{ apiKey: string
useEffectOnce(() => {
const onFeatureFlagsChanged = () => {
// Update analytics context whenever a flag changes
addAnalyticsContext({ experiments: JSON.stringify(ldClient.current?.allFlags()) });
analyticsService.setContext({ experiments: JSON.stringify(ldClient.current?.allFlags()) });
// Check for overwritten i18n messages
updateI18nMessages();
};
Expand Down
6 changes: 2 additions & 4 deletions airbyte-webapp/src/views/common/AnalyticsProvider.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,9 @@ import { useConfig } from "config";
import AnalyticsServiceProvider from "hooks/services/Analytics/useAnalyticsService";
import useSegment from "hooks/useSegment";

const AnalyticsProvider: React.FC<React.PropsWithChildren<unknown>> = ({ children }) => {
export const AnalyticsProvider: React.FC<React.PropsWithChildren<unknown>> = ({ children }) => {
const config = useConfig();
useSegment(config.segment.enabled ? config.segment.token : "");

return <AnalyticsServiceProvider version={config.version}>{children}</AnalyticsServiceProvider>;
return <AnalyticsServiceProvider>{children}</AnalyticsServiceProvider>;
};

export { AnalyticsProvider };