fix(rum): resync the date picker after mount and exclude synthetic sessions from the Sessions KPI (#15132)

This commit is contained in:
Bhargav
2026-10-06 11:14:37 +00:00
committed by GitHub
parent 18c985cad1
commit fc8c50bc62
5 changed files with 108 additions and 6 deletions
@@ -14,7 +14,7 @@
// along with this program. If not, see <http://www.gnu.org/licenses/>.
// Verbatim copy of design-v2/final_sql.txt, the SQL that ran on real data in sql-proof-v2.md.
export const Q3_SUMMARY = String.raw`SELECT COUNT(DISTINCT CASE WHEN _timestamp >= {cs} THEN session_id END) AS sessions, COUNT(DISTINCT CASE WHEN _timestamp < {cs} THEN session_id END) AS prev_sessions, COUNT(DISTINCT CASE WHEN _timestamp >= {cs} AND type = 'view' THEN view_id END) AS views, COUNT(CASE WHEN _timestamp >= {cs} AND type IN ('view', 'action') THEN 1 END) AS va_rows, COUNT(DISTINCT CASE WHEN _timestamp >= {cs} AND session_type = 'synthetics' THEN session_id END) AS synthetic_sessions, MAX(_timestamp) AS data_through_us, COUNT(DISTINCT CASE WHEN _timestamp >= {cs} THEN NULLIF(usr_email, '') END) AS usr_email__values, COUNT(DISTINCT CASE WHEN _timestamp >= {cs} AND NULLIF(usr_email, '') IS NOT NULL THEN session_id END) AS usr_email__sessions FROM "_rumdata" WHERE {scope} LIMIT 1`;
export const Q3_SUMMARY = String.raw`SELECT COUNT(DISTINCT CASE WHEN _timestamp >= {cs} AND (session_type IS NULL OR session_type <> 'synthetics') THEN session_id END) AS sessions, COUNT(DISTINCT CASE WHEN _timestamp < {cs} AND (session_type IS NULL OR session_type <> 'synthetics') THEN session_id END) AS prev_sessions, COUNT(DISTINCT CASE WHEN _timestamp >= {cs} AND type = 'view' THEN view_id END) AS views, COUNT(CASE WHEN _timestamp >= {cs} AND type IN ('view', 'action') THEN 1 END) AS va_rows, COUNT(DISTINCT CASE WHEN _timestamp >= {cs} AND session_type = 'synthetics' THEN session_id END) AS synthetic_sessions, MAX(_timestamp) AS data_through_us, COUNT(DISTINCT CASE WHEN _timestamp >= {cs} THEN NULLIF(usr_email, '') END) AS usr_email__values, COUNT(DISTINCT CASE WHEN _timestamp >= {cs} AND NULLIF(usr_email, '') IS NOT NULL THEN session_id END) AS usr_email__sessions FROM "_rumdata" WHERE {scope} LIMIT 1`;
export const Q4_PAGES_IDENTITY = String.raw`WITH v0 AS (SELECT session_id AS sid, type AS ty, CASE WHEN _timestamp >= {cs} THEN 1 ELSE 0 END AS cur, MIN(CASE WHEN type = 'view' THEN view_id END) AS vid, MIN(CASE WHEN type = 'view' THEN view_url END) AS url, first_value(NULLIF(usr_email, '') ORDER BY CASE WHEN NULLIF(usr_email, '') IS NULL THEN 1 ELSE 0 END, date) AS u0, MIN(CASE WHEN NULLIF(usr_email, '') IS NOT NULL THEN date END) AS ut FROM "_rumdata" WHERE {scope} AND (type = 'view' OR NULLIF(usr_email, '') IS NOT NULL) GROUP BY session_id, type, CASE WHEN type = 'view' THEN view_id END, CASE WHEN _timestamp >= {cs} THEN 1 ELSE 0 END),
v1 AS (SELECT sid, ty, cur, vid, url, FIRST_VALUE(u0) OVER (PARTITION BY sid ORDER BY CASE WHEN u0 IS NULL THEN 1 ELSE 0 END, ut) AS u FROM v0),
@@ -367,13 +367,29 @@ describe("scope-load builders (G2)", () => {
);
});
it("Q3 summary equals the proven shape and keeps synthetic sessions countable", () => {
it("Q3 summary equals the proven shape, counts synthetic sessions and excludes them from sessions (P1-M2)", () => {
const sql = summarySql(s, CS);
const unscoped = "application_id = 'web' AND session_id IS NOT NULL";
expect(sql.split(unscoped).join("{scope}").split(String(CS)).join("{cs}")).toBe(
PROVEN.Q3_SUMMARY,
);
expect(sql).not.toContain("<> 'synthetics'");
// `sessions`/`prev_sessions` must drop synthetics so the KPI matches the "N synthetic sessions
// excluded" caption, while `synthetic_sessions` still counts them via its own, unfiltered CASE.
expect(sql).toContain(
"CASE WHEN _timestamp >= {cs} AND (session_type IS NULL OR session_type <> 'synthetics') THEN session_id END) AS sessions".replace(
"{cs}",
String(CS),
),
);
expect(sql).toContain(
"CASE WHEN _timestamp < {cs} AND (session_type IS NULL OR session_type <> 'synthetics') THEN session_id END) AS prev_sessions".replace(
"{cs}",
String(CS),
),
);
expect(sql).toContain(
`CASE WHEN _timestamp >= ${CS} AND session_type = 'synthetics' THEN session_id END) AS synthetic_sessions`,
);
});
it("Q3 without session_type or candidates reports zero synthetics and no identity pairs", () => {
+9 -3
View File
@@ -322,12 +322,18 @@ export function facetOptionsSql(scope: AnalyticsScope, field: "env" | "version")
export function summarySql(scope: AnalyticsScope, currentStartUs: number): string {
const cs = Math.trunc(currentStartUs);
const synthetic = has(scope, "session_type")
const hasSessionType = has(scope, "session_type");
const synthetic = hasSessionType
? `COUNT(DISTINCT CASE WHEN _timestamp >= ${cs} AND session_type = 'synthetics' THEN session_id END) AS synthetic_sessions`
: "0 AS synthetic_sessions";
// The caption says synthetics are excluded, so the sessions KPI (and its prior-period base) must
// actually drop them here too, not just report their count via `synthetic_sessions`.
const notSynthetic = hasSessionType
? " AND (session_type IS NULL OR session_type <> 'synthetics')"
: "";
const cols = [
`COUNT(DISTINCT CASE WHEN _timestamp >= ${cs} THEN session_id END) AS sessions`,
`COUNT(DISTINCT CASE WHEN _timestamp < ${cs} THEN session_id END) AS prev_sessions`,
`COUNT(DISTINCT CASE WHEN _timestamp >= ${cs}${notSynthetic} THEN session_id END) AS sessions`,
`COUNT(DISTINCT CASE WHEN _timestamp < ${cs}${notSynthetic} THEN session_id END) AS prev_sessions`,
`COUNT(DISTINCT CASE WHEN _timestamp >= ${cs} AND type = 'view' THEN view_id END) AS views`,
`COUNT(CASE WHEN _timestamp >= ${cs} AND type IN ('view', 'action') THEN 1 END) AS va_rows`,
synthetic,
+69
View File
@@ -773,6 +773,75 @@ describe("AppAnalytics shell", () => {
expect(router.currentRoute.value.query.period).toBe("30d");
});
it("a reload or shared link moves the already-mounted picker, not just the model (P1-M1)", async () => {
const setSavedDate = vi.fn();
router = makeRouter();
await router.push("/product-analytics/overview?app=web&period=4w");
wrapper = mount(
{ template: "<router-view />" },
{
global: {
plugins: [router, store],
stubs: {
DateTimePickerDashboard: {
props: ["modelValue"],
setup: (_: unknown, { expose }: { expose: (e: object) => void }) => {
expose({ setSavedDate });
return {};
},
template: "<div data-test='date-picker-stub' />",
},
ShareButton: { template: "<button data-test='share-stub' />", props: ["url"] },
},
},
attachTo: document.body,
},
);
await flushPromises();
await flushPromises();
expect(useProductAnalytics().state.datetime).toMatchObject({
valueType: "relative",
relativeTimePeriod: "4w",
});
expect(setSavedDate).toHaveBeenCalledWith({ type: "relative", relativeTimePeriod: "4w" });
});
it("switching sub-tabs with a new range in the URL moves the mounted picker too (P1-M1)", async () => {
const setSavedDate = vi.fn();
router = makeRouter();
await router.push("/product-analytics/overview?app=web&period=7d");
wrapper = mount(
{ template: "<router-view />" },
{
global: {
plugins: [router, store],
stubs: {
DateTimePickerDashboard: {
props: ["modelValue"],
setup: (_: unknown, { expose }: { expose: (e: object) => void }) => {
expose({ setSavedDate });
return {};
},
template: "<div data-test='date-picker-stub' />",
},
ShareButton: { template: "<button data-test='share-stub' />", props: ["url"] },
},
},
attachTo: document.body,
},
);
await flushPromises();
await flushPromises();
setSavedDate.mockClear();
await router.push("/product-analytics/funnels?app=web&period=4w");
await flushPromises();
await flushPromises();
expect(setSavedDate).toHaveBeenCalledWith({ type: "relative", relativeTimePeriod: "4w" });
});
it("mounting inside keep-alive enters once, as a plain mount does (F6)", async () => {
await mountAt("/product-analytics/overview?app=web&period=7d");
await new Promise((r) => setTimeout(r, 30));
+11
View File
@@ -71,6 +71,7 @@ along with this program. If not, see <http://www.gnu.org/licenses/>.
@update:model-value="onVersionChange"
/>
<DateTimePickerDashboard
ref="dateTimePickerRef"
:model-value="state.datetime"
menu-align="end"
data-test="rum-analytics-date-picker"
@@ -350,6 +351,7 @@ let active = false;
let activatedOnce = false;
const invalidDismissed = ref(false);
const identityOpen = ref(false);
const dateTimePickerRef = ref<any>(null);
const org = computed(() => store.state.selectedOrganization?.identifier ?? "");
@@ -470,6 +472,12 @@ const sameRange = (a: AnalyticsDateTime, b: AnalyticsDateTime) =>
? a.relativeTimePeriod === b.relativeTimePeriod
: a.startTime === b.startTime && a.endTime === b.endTime);
// The picker reads this shape into an already-mounted DateTime (see DateTimePickerDashboard.setSavedDate).
const toPickerSavedDate = (dt: AnalyticsDateTime) =>
dt.valueType === "absolute"
? { type: "absolute", startTime: dt.startTime, endTime: dt.endTime }
: { type: "relative", relativeTimePeriod: dt.relativeTimePeriod };
const onDateChange = (value: AnalyticsDateTime) => {
if (sameRange(value, state.datetime)) return;
pa.setScope({ datetime: { ...value } });
@@ -512,6 +520,9 @@ const resumesScope = () =>
const enter = async () => {
if (!resumesScope()) {
pa.initFromRoute(route.query, { keepFunnel: route.name === PA_ROUTES.funnels });
// The picker reads its range only on mount, so a reload or a shared link (both land here
// after it has already mounted with the default range) must move the mounted picker too.
dateTimePickerRef.value?.setSavedDate?.(toPickerSavedDate(state.datetime));
}
enteredOrg = org.value;
invalidDismissed.value = false;