Skip to content

Commit

Permalink
fix: Roll back to previous FirstInteraction implementation (#1359)
Browse files Browse the repository at this point in the history
  • Loading branch information
metal-messiah authored Jan 31, 2025
1 parent cb866e5 commit c2e22ab
Show file tree
Hide file tree
Showing 12 changed files with 115 additions and 101 deletions.
6 changes: 1 addition & 5 deletions src/common/vitals/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,13 +5,9 @@
export const VITAL_NAMES = {
FIRST_PAINT: 'fp',
FIRST_CONTENTFUL_PAINT: 'fcp',
FIRST_INTERACTION: 'fi',
FIRST_INPUT_DELAY: 'fi',
LARGEST_CONTENTFUL_PAINT: 'lcp',
CUMULATIVE_LAYOUT_SHIFT: 'cls',
INTERACTION_TO_NEXT_PAINT: 'inp',
TIME_TO_FIRST_BYTE: 'ttfb'
}

export const PERFORMANCE_ENTRY_TYPE = {
FIRST_INPUT: 'first-input'
}
28 changes: 28 additions & 0 deletions src/common/vitals/first-input-delay.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,28 @@
/**
* Copyright 2020-2025 New Relic, Inc. All rights reserved.
* SPDX-License-Identifier: Apache-2.0
*/
import { onFID } from 'web-vitals/attribution'
import { VitalMetric } from './vital-metric'
import { VITAL_NAMES } from './constants'
import { initiallyHidden, isBrowserScope } from '../constants/runtime'

export const firstInputDelay = new VitalMetric(VITAL_NAMES.FIRST_INPUT_DELAY)

if (isBrowserScope) {
onFID(({ value, attribution }) => {
if (initiallyHidden || firstInputDelay.isValid) return
const attrs = {
type: attribution.eventType,
fid: Math.round(value),
eventTarget: attribution.eventTarget,
loadState: attribution.loadState
}

// CWV will only report one (THE) first-input entry to us; fid isn't reported if there are no user interactions occurs before the *first* page hiding.
firstInputDelay.update({
value: attribution.eventTime,
attrs
})
})
}
38 changes: 0 additions & 38 deletions src/common/vitals/first-interaction.js

This file was deleted.

4 changes: 2 additions & 2 deletions src/features/page_view_timing/aggregate/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -11,8 +11,8 @@ import { FEATURE_NAMES } from '../../../loaders/features/features'
import { AggregateBase } from '../../utils/aggregate-base'
import { cumulativeLayoutShift } from '../../../common/vitals/cumulative-layout-shift'
import { firstContentfulPaint } from '../../../common/vitals/first-contentful-paint'
import { firstInputDelay } from '../../../common/vitals/first-input-delay'
import { firstPaint } from '../../../common/vitals/first-paint'
import { firstInteraction } from '../../../common/vitals/first-interaction'
import { interactionToNextPaint } from '../../../common/vitals/interaction-to-next-paint'
import { largestContentfulPaint } from '../../../common/vitals/largest-contentful-paint'
import { timeToFirstByte } from '../../../common/vitals/time-to-first-byte'
Expand All @@ -37,8 +37,8 @@ export class Aggregate extends AggregateBase {
this.waitForFlags(([])).then(() => {
firstPaint.subscribe(this.#handleVitalMetric)
firstContentfulPaint.subscribe(this.#handleVitalMetric)
firstInputDelay.subscribe(this.#handleVitalMetric)
largestContentfulPaint.subscribe(this.#handleVitalMetric)
firstInteraction.subscribe(this.#handleVitalMetric)
interactionToNextPaint.subscribe(this.#handleVitalMetric)
timeToFirstByte.subscribe(({ attrs }) => {
this.addTiming('load', Math.round(attrs.navigationEntry.loadEventEnd))
Expand Down
5 changes: 5 additions & 0 deletions src/features/session_trace/aggregate/trace/storage.js
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,11 @@ export class TraceStorage {

processPVT (name, value, attrs) {
this.storeTiming({ [name]: value })
if (hasFID(name, attrs)) this.storeEvent({ type: 'fid', target: 'document' }, 'document', value, value + attrs.fid)

function hasFID (name, attrs) {
return name === 'fi' && !!attrs && typeof attrs.fid === 'number'
}
}

storeTiming (timingEntry, isAbsoluteTimestamp = false) {
Expand Down
11 changes: 11 additions & 0 deletions tests/components/page_view_timing/aggregate.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,10 @@ jest.mock('web-vitals/attribution', () => ({
value: 1,
attribution: {}
})),
onFID: jest.fn(cb => cb({
value: 1234,
attribution: { eventTime: 5, eventType: 'pointerdown' }
})),
onINP: jest.fn((cb) => cb({
value: 8,
attribution: {}
Expand Down Expand Up @@ -81,6 +85,13 @@ test('LCP event with CLS attribute', () => {
}
})

test('sends expected FI attributes when available', () => {
expect(timingsAggregate.events.get()[0].data.length).toBeGreaterThanOrEqual(1)
const fiPayload = timingsAggregate.events.get()[0].data.find(x => x.name === 'fi')
expect(fiPayload.value).toEqual(5)
expect(fiPayload.attrs).toEqual(expect.objectContaining({ type: 'pointerdown', fid: 1234, cls: 0.1119, ...expectedNetworkInfo }))
})

test('sends CLS node with right val on vis change', () => {
let clsNode = timingsAggregate.events.get()[0].data.find(tn => tn.name === VITAL_NAMES.CUMULATIVE_LAYOUT_SHIFT)
expect(clsNode).toBeUndefined()
Expand Down
11 changes: 11 additions & 0 deletions tests/components/page_view_timing/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,10 @@ jest.mock('web-vitals/attribution', () => ({
value: 1,
attribution: {}
})),
onFID: jest.fn(cb => cb({
value: 1234,
attribution: { eventTime: 5, eventType: 'pointerdown' }
})),
onINP: jest.fn((cb) => cb({
value: 8,
attribution: {}
Expand Down Expand Up @@ -83,6 +87,13 @@ describe('pvt aggregate tests', () => {
}
})

test('sends expected FI attributes when available', () => {
expect(pvtAgg.events.get()[0].data.length).toBeTruthy()
const fiPayload = pvtAgg.events.get()[0].data.find(x => x.name === 'fi')
expect(fiPayload.value).toEqual(5)
expect(fiPayload.attrs).toEqual(expect.objectContaining({ type: 'pointerdown', fid: 1234, cls: 0.1119, ...expectedNetworkInfo }))
})

test('sends CLS node with right val on vis change', () => {
let clsNode = pvtAgg.events.get()[0].data.find(tn => tn.name === VITAL_NAMES.CUMULATIVE_LAYOUT_SHIFT)
expect(clsNode).toBeUndefined()
Expand Down
7 changes: 6 additions & 1 deletion tests/components/session_trace/aggregate.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ test('creates right nodes', async () => {
document.dispatchEvent(new CustomEvent('DOMContentLoaded')) // simulate natural browser event
window.dispatchEvent(new CustomEvent('load')) // load is actually ignored by Trace as it should be passed by the PVT feature, so it should not be in payload
sessionTraceAggregate.events.storeXhrAgg('xhr', '[200,null,null]', { method: 'GET', status: 200 }, { rxSize: 770, duration: 99, cbTime: 0, time: 217 }) // fake ajax data
sessionTraceAggregate.events.processPVT('fi', 30) // fake pvt data
sessionTraceAggregate.events.processPVT('fi', 30, { fid: 8 }) // fake pvt data

const payload = sessionTraceAggregate.makeHarvestPayload()[0].payload
let res = payload.body
Expand Down Expand Up @@ -58,6 +58,11 @@ test('creates right nodes', async () => {
expect(pvt.o).toEqual('document')
expect(pvt.s).toEqual(pvt.e) // that FI has no duration
expect(pvt.t).toEqual('timing')
pvt = res.filter(node => node.n === 'fid')[0]
expect(pvt.o).toEqual('document')
expect(pvt.s).toEqual(30) // that FID has a duration relative to FI'
expect(pvt.e).toEqual(30 + 8)
expect(pvt.t).toEqual('event')

let unknown = res.filter(n => n.o === 'unknown')
expect(unknown.length).toEqual(0) // no events with unknown origin
Expand Down
12 changes: 8 additions & 4 deletions tests/specs/pvt/timings.e2e.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import { supportsCumulativeLayoutShift, supportsFirstPaint, supportsInteractionToNextPaint, supportsLargestContentfulPaint, supportsPerformanceEventTiming } from '../../../tools/browser-matcher/common-matchers.mjs'
import { supportsCumulativeLayoutShift, supportsFirstInputDelay, supportsFirstPaint, supportsInteractionToNextPaint, supportsLargestContentfulPaint } from '../../../tools/browser-matcher/common-matchers.mjs'
import { testTimingEventsRequest } from '../../../tools/testing-server/utils/expect-tests'

const isClickInteractionType = type => type === 'pointerdown' || type === 'mousedown' || type === 'click'
const loadersToTest = ['rum', 'spa']

describe('pvt timings tests', () => {
Expand Down Expand Up @@ -71,7 +72,7 @@ describe('pvt timings tests', () => {

describe('interaction related timings', () => {
loadersToTest.forEach(loader => {
it(`FI, INP & LCP for ${loader} agent`, async () => {
it(`FI, FID, INP & LCP for ${loader} agent`, async () => {
const start = Date.now()
await browser.url(
await browser.testHandle.assetURL('basic-click-tracking.html', { loader })
Expand All @@ -84,17 +85,20 @@ describe('pvt timings tests', () => {
.then(async () => browser.url(await browser.testHandle.assetURL('/')))
])

if (browserMatch(supportsPerformanceEventTiming)) {
if (browserMatch(supportsFirstInputDelay)) {
// FID is replaced by subscribing to 'first-input'
const fi = timingsHarvests.find(harvest => harvest.request.body.find(t => t.name === 'fi'))
?.request.body.find(timing => timing.name === 'fi')
expect(fi.value).toBeGreaterThanOrEqual(0)
expect(fi.value).toBeLessThan(Date.now() - start)

const isClickInteractionType = type => type === 'pointerdown' || type === 'mousedown' || type === 'click'
const fiType = fi.attributes.find(attr => attr.key === 'type')
expect(isClickInteractionType(fiType.value)).toEqual(true)
expect(fiType.type).toEqual('stringAttribute')

const fid = fi.attributes.find(attr => attr.key === 'fid')
expect(fid.value).toBeGreaterThanOrEqual(0)
expect(fid.type).toEqual('doubleAttribute')
}

if (browserMatch(supportsLargestContentfulPaint)) {
Expand Down
Original file line number Diff line number Diff line change
@@ -1,43 +1,34 @@
import { PERFORMANCE_ENTRY_TYPE } from '../../../../src/common/vitals/constants'

beforeEach(() => {
jest.resetModules()
jest.resetAllMocks()
jest.clearAllMocks()

const mockPerformanceObserver = jest.fn(cb => ({
// Note: this is an imperfect mock, as observer.disconnect() is not functional
observe: () => {
const callCb = () => {
cb({
getEntries: () => [{
name: PERFORMANCE_ENTRY_TYPE.FIRST_INPUT,
startTime: 1
}]
})
setTimeout(callCb, 250)
}
setTimeout(callCb, 250)
},
disconnect: jest.fn()
}))
global.PerformanceObserver = mockPerformanceObserver
global.PerformanceObserver.supportedEntryTypes = [PERFORMANCE_ENTRY_TYPE.FIRST_INPUT]
})

const getFreshImport = async (codeToRun) => {
const { firstInteraction } = await import('../../../../src/common/vitals/first-interaction')
codeToRun(firstInteraction)
const fidAttribution = {
eventTarget: 'html>body',
eventType: 'pointerdown',
eventTime: 1,
loadState: 'loading'
}
let triggeronFIDCallback
const getFreshFIDImport = async (codeToRun) => {
jest.doMock('web-vitals/attribution', () => ({
onFID: jest.fn(cb => { triggeronFIDCallback = cb; cb({ value: 100, attribution: fidAttribution }) })
}))
const { firstInputDelay } = await import('../../../../src/common/vitals/first-input-delay')
codeToRun(firstInputDelay)
}

describe('fi (first interaction)', () => {
test('reports fi', (done) => {
getFreshImport(metric => {
metric.subscribe(({ value }) => {
expect(value).toEqual(1)
done()
})
})
describe('fid', () => {
test('reports fcp from web-vitals', (done) => {
getFreshFIDImport(metric => metric.subscribe(({ value, attrs }) => {
expect(value).toEqual(1)
expect(attrs.type).toEqual(fidAttribution.eventType)
expect(attrs.fid).toEqual(100)
expect(attrs.eventTarget).toEqual(fidAttribution.eventTarget)
expect(attrs.loadState).toEqual(fidAttribution.loadState)
done()
}))
})

test('Does NOT report values if initiallyHidden', (done) => {
Expand All @@ -47,7 +38,7 @@ describe('fi (first interaction)', () => {
isBrowserScope: true
}))

getFreshImport(metric => {
getFreshFIDImport(metric => {
metric.subscribe(() => {
console.log('should not have reported')
expect(1).toEqual(2)
Expand All @@ -63,7 +54,7 @@ describe('fi (first interaction)', () => {
isBrowserScope: false
}))

getFreshImport(metric => {
getFreshFIDImport(metric => {
metric.subscribe(({ value, attrs }) => {
console.log('should not have reported...')
expect(1).toEqual(2)
Expand All @@ -79,7 +70,7 @@ describe('fi (first interaction)', () => {
isBrowserScope: true
}))
let witness = 0
getFreshImport(metric => {
getFreshFIDImport(metric => {
metric.subscribe(({ value }) => {
expect(value).toEqual(1)
witness++
Expand All @@ -99,14 +90,15 @@ describe('fi (first interaction)', () => {
isBrowserScope: true
}))
let triggered = 0
getFreshImport(metric => metric.subscribe(({ value }) => {
triggered++
expect(value).toEqual(1)
expect(triggered).toEqual(1)
setTimeout(() => {
getFreshFIDImport(metric => {
metric.subscribe(({ value }) => {
triggered++
expect(value).toEqual(1)
expect(triggered).toEqual(1)
done()
}, 1000)
}))
})
triggeronFIDCallback({ value: 'notequal1' })
expect(triggered).toEqual(1)
done()
})
})
})
6 changes: 3 additions & 3 deletions tests/unit/features/page_view_timing/aggregate/index.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -162,7 +162,7 @@ function testCases () {
},
{
type: 'doubleAttribute',
key: 'foo',
key: 'fid',
value: 12.34
}
]
Expand Down Expand Up @@ -201,7 +201,7 @@ function testCases () {
},
{
type: 'doubleAttribute',
key: 'foo',
key: 'fid',
value: 12.34
}
]
Expand All @@ -218,7 +218,7 @@ function testCases () {
},
{
type: 'doubleAttribute',
key: 'foo',
key: 'fid',
value: 12.34
}
]
Expand Down
Loading

0 comments on commit c2e22ab

Please sign in to comment.