Skip to content

Commit

Permalink
use experimentId instead of FxA UID-derived userId
Browse files Browse the repository at this point in the history
  • Loading branch information
rhelmer committed Apr 23, 2024
1 parent a4ebe5e commit 4705929
Show file tree
Hide file tree
Showing 10 changed files with 53 additions and 50 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,7 @@ export type Props = {
fxaSettingsUrl: string;
scanCount: number;
isNewUser: boolean;
telemetryId: string;
experimentationId: string;
totalNumberOfPerformedScans?: number;
};

Expand All @@ -78,7 +78,7 @@ export type TabData = {

export const View = (props: Props) => {
const l10n = useL10n();
const recordTelemetry = useTelemetry();
const recordTelemetry = useTelemetry(props.experimentationId);
const countryCode = useContext(CountryCodeContext);

const adjustedScanResults = props.userScanData.results.map((scanResult) => {
Expand Down Expand Up @@ -413,7 +413,6 @@ export const View = (props: Props) => {
onSelectionChange={(selectedKey) => {
setSelectedTab(selectedKey as TabType);
recordTelemetry("dashboard", "view", {
user_id: props.telemetryId,
dashboard_tab: selectedKey as TabType,
legacy_user: !props.isNewUser,
breach_count: breachesDataArray.length,
Expand Down Expand Up @@ -449,7 +448,6 @@ export const View = (props: Props) => {
onShowFixed={() => {
setSelectedTab("fixed");
recordTelemetry("dashboard", "view", {
user_id: props.telemetryId,
dashboard_tab: "fixed",
legacy_user: !props.isNewUser,
breach_count: breachesDataArray.length,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ import { getEnabledFeatureFlags } from "../../../../../../../db/tables/featureFl
import { getAttributionsFromCookiesOrDb } from "../../../../../../functions/server/attributions";
import { checkSession } from "../../../../../../functions/server/checkSession";
import { isPrePlusUser } from "../../../../../../functions/server/isPrePlusUser";
import { getUserId } from "../../../../../../functions/server/getUserId";
import { getExperimentationId } from "../../../../../../functions/server/getUserId";

export default async function DashboardPage() {
const session = await getServerSession();
Expand Down Expand Up @@ -119,7 +119,7 @@ export default async function DashboardPage() {
scanCount={scanCount}
totalNumberOfPerformedScans={profileStats?.total}
isNewUser={isNewUser}
telemetryId={getUserId(session)}
experimentationId={getExperimentationId(session)}
/>
);
}
13 changes: 8 additions & 5 deletions src/app/(proper_react)/layout.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ import { ReactAriaI18nProvider } from "../../contextProviders/react-aria";
import { CountryCodeProvider } from "../../contextProviders/country-code";
import { getCountryCode } from "../functions/server/getCountryCode";
import { PageLoadEvent } from "../components/client/PageLoadEvent";
import { getUserId } from "../functions/server/getUserId";
import { getExperimentationId } from "../functions/server/getUserId";
import { getEnabledFeatureFlags } from "../../db/tables/featureFlags";

export default async function Layout({ children }: { children: ReactNode }) {
Expand All @@ -22,23 +22,26 @@ export default async function Layout({ children }: { children: ReactNode }) {
const session = await getServerSession();

let enabledFlags = [];
let userId;
let experimentationId;
if (session) {
enabledFlags = await getEnabledFeatureFlags({
user: session.user,
});
userId = getUserId(session.user);
experimentationId = getExperimentationId(session.user);
} else {
enabledFlags = await getEnabledFeatureFlags({ ignoreExperiments: true });
userId = getUserId(null);
experimentationId = getExperimentationId(null);
}

return (
<L10nProvider bundleSources={l10nBundles}>
<ReactAriaI18nProvider locale={getLocale(l10nBundles)}>
<CountryCodeProvider countryCode={countryCode}>
{children}
<PageLoadEvent userId={userId} enabledFlags={enabledFlags} />
<PageLoadEvent
enabledFlags={enabledFlags}
experimentationId={experimentationId}
/>
</CountryCodeProvider>
</ReactAriaI18nProvider>
</L10nProvider>
Expand Down
31 changes: 9 additions & 22 deletions src/app/components/client/PageLoadEvent.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -4,57 +4,44 @@

"use client";

import { useEffect, useMemo } from "react";
import { useEffect } from "react";
import { useCookies } from "react-cookie";
import { usePathname } from "next/navigation";
import { FeatureFlagName } from "../../../db/tables/featureFlags";
import { useTelemetry } from "../../hooks/useTelemetry";
import { GleanMetricMap } from "../../../telemetry/generated/_map";

export type Props = {
userId: string;
experimentationId: string;
enabledFlags: FeatureFlagName[];
};

// Empty component that records a page view on first load.
export const PageLoadEvent = (props: Props) => {
const [cookies, setCookie] = useCookies([
"userId",
"attributionsFirstTouch",
"attributionsLastTouch",
"experimentationId",
]);
const pathname = usePathname();

const recordTelemetry = useTelemetry();
const recordTelemetry = useTelemetry(props.experimentationId);

const userId: GleanMetricMap["page"]["view"]["user_id"] = useMemo(() => {
// If the user is not logged in, use randomly-generated user ID and store in cookie.
if (props.userId.startsWith("guest")) {
if (!cookies.userId) {
setCookie("userId", props.userId);
}
return props.userId;
}

if (props.enabledFlags.includes("FxaUidTelemetry")) {
return props.userId;
} else {
return undefined;
if (props.experimentationId.startsWith("guest")) {
if (!cookies.experimentationId) {
setCookie("experimentationId", props.experimentationId);
}
}, [cookies.userId, setCookie, props.userId, props.enabledFlags]);
}

// On first load of the page, record a page view.
useEffect(() => {
const pageViewParams: GleanMetricMap["page"]["view"] = {
...getUtmParams(),
path: pathname,
};
if (typeof userId === "string") {
pageViewParams.user_id = userId;
}

recordTelemetry("page", "view", pageViewParams);
}, [recordTelemetry, pathname, userId]);
}, [recordTelemetry, pathname]);

useEffect(() => {
// record attributions on page load
Expand Down
6 changes: 3 additions & 3 deletions src/app/functions/server/getExperiments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,11 +17,11 @@ interface Features {
* Call the Cirrus sidecar, which returns a list of eligible experiments for the current user.
*
* @see https://github.com/mozilla/experimenter/tree/main/cirrus
* @param userId Persistent ID for user, either guest or authenticated
* @param experimentationId Persistent ID for user, either guest or authenticated
* @returns
*/
export async function getExperiments(
userId: string | undefined,
experimentationId: string | undefined,
): Promise<Features | undefined> {
const session = await getServerSession();
const headerList = headers();
Expand All @@ -39,7 +39,7 @@ export async function getExperiments(
},
method: "POST",
body: JSON.stringify({
client_id: userId,
client_id: experimentationId,
context: {
locale: session?.user.fxa?.locale,
countryCode: getCountryCode(headerList),
Expand Down
29 changes: 21 additions & 8 deletions src/app/functions/server/getUserId.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -5,28 +5,41 @@
import "server-only";
import { cookies } from "next/headers";
import { randomUUID } from "crypto";
import { v5 as uuidv5 } from "uuid";
import { Session } from "next-auth";

export function getUserId(user: Session["user"] | null) {
/**
* Create a stable ID used for Monitor experimentation, derived from the subscriber ID.
* Instead of using the ID directly,
*
* @param user Session["user"]
* @returns {string} - v5 UUID, possibly with `guest-` prefix.
*/
export function getExperimentationId(user: Session["user"] | null): string {
const accountId = user?.subscriber?.id;
let userId = "";
let experimentationId = "";

if (accountId && typeof accountId === "string") {
// If the user is logged in, use the Subscriber ID.
// Note: we may want to use the FxA UID here, but we need approval for that first.
userId = accountId;
const namespace = process.env.NIMBUS_UUID_NAMESPACE;
if (!namespace) {
throw new Error(
"NIMBUS_UUID_NAMESPACE not set, cannot create experimentationId",
);
}
experimentationId = uuidv5(accountId, namespace);
} else {
// if the user is not logged in, use a cookie with a randomly-generated Nimbus user ID.
// TODO: could we use client ID for this? There's no supported way to get it from GleanJS.
const cookie = cookies().get("userId");
const cookie = cookies().get("experimentationId");
if (cookie) {
userId = cookie.value;
experimentationId = cookie.value;
} else {
// TODO Cookies can only be set in server action or route handler
// @see https://nextjs.org/docs/app/api-reference/functions/cookies#cookiessetname-value-options
// This is set client-side in <PageLoadEvent>.
userId = `guest-${randomUUID()}`;
experimentationId = `guest-${randomUUID()}`;
}
}
return userId;
return experimentationId;
}
1 change: 1 addition & 0 deletions src/app/functions/server/onerep.ts
Original file line number Diff line number Diff line change
Expand Up @@ -470,6 +470,7 @@ export async function getProfilesStats(
from?: Date,
to?: Date,
): Promise<ProfileStats | undefined> {
return undefined;
const queryParams = new URLSearchParams();
if (from) queryParams.set("from", from.toISOString().substring(0, 10));
if (to) queryParams.set("to", to.toISOString().substring(0, 10));
Expand Down
3 changes: 2 additions & 1 deletion src/app/hooks/useGlean.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ import EventMetricType from "@mozilla/glean/private/metrics/event";
import type { GleanMetricMap } from "../../telemetry/generated/_map";
import { PublicEnvContext } from "../../contextProviders/public-env";

export const useGlean = () => {
export const useGlean = (experimentationId?: string) => {
const { PUBLIC_APP_ENV } = useContext(PublicEnvContext);

// Initialize Glean only on the first render of our custom hook.
Expand Down Expand Up @@ -42,6 +42,7 @@ export const useGlean = () => {
maxEvents: 1,
channel: PUBLIC_APP_ENV,
enableAutoPageLoadEvents: true,
experimentationId,
});
// This effect should only run once
// eslint-disable-next-line react-hooks/exhaustive-deps
Expand Down
4 changes: 2 additions & 2 deletions src/app/hooks/useTelemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,9 +17,9 @@ const TelemetryPlatforms = {
Ga: "ga",
} as const;

export const useTelemetry = () => {
export const useTelemetry = (experimentationId?: string) => {
const path = usePathname();
const recordGlean = useGlean();
const recordGlean = useGlean(experimentationId);

const { Glean, Ga } = TelemetryPlatforms;
const recordTelemetry = <
Expand Down
6 changes: 3 additions & 3 deletions src/db/tables/featureFlags.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import { logger } from "../../app/functions/server/logging";
import { FeatureFlagRow } from "knex/types/tables";
import { getExperiments } from "../../app/functions/server/getExperiments";
import { Session } from "next-auth";
import { getUserId } from "../../app/functions/server/getUserId";
import { getExperimentationId } from "../../app/functions/server/getUserId";

const knex = createDbConnection();

Expand Down Expand Up @@ -58,8 +58,8 @@ export async function getEnabledFeatureFlags(
// Use Nimbus to allow features per-user.
try {
if (!options.ignoreExperiments) {
const userId = getUserId(options.user);
const features = await getExperiments(userId);
const experimentationId = getExperimentationId(options.user);
const features = await getExperiments(experimentationId);

if (features) {
for (const feature of Object.keys(features)) {
Expand Down

0 comments on commit 4705929

Please sign in to comment.