Skip to content

Commit

Permalink
feat: Removing long task (#1153)
Browse files Browse the repository at this point in the history
  • Loading branch information
cwli24 authored Sep 16, 2024
1 parent b12131c commit 304e395
Show file tree
Hide file tree
Showing 11 changed files with 4 additions and 248 deletions.
2 changes: 1 addition & 1 deletion src/common/config/init.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ const model = () => {
obfuscate: undefined,
page_action: { enabled: true },
page_view_event: { enabled: true, autoStart: true },
page_view_timing: { enabled: true, harvestTimeSeconds: 30, long_task: false, autoStart: true },
page_view_timing: { enabled: true, harvestTimeSeconds: 30, autoStart: true },
privacy: { cookies_enabled: true }, // *cli - per discussion, default should be true
proxy: {
assets: undefined, // if this value is set, it will be used to overwrite the webpack asset path used to fetch assets
Expand Down
1 change: 0 additions & 1 deletion src/common/vitals/constants.js
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,5 @@ export const VITAL_NAMES = {
LARGEST_CONTENTFUL_PAINT: 'lcp',
CUMULATIVE_LAYOUT_SHIFT: 'cls',
INTERACTION_TO_NEXT_PAINT: 'inp',
LONG_TASK: 'lt',
TIME_TO_FIRST_BYTE: 'ttfb'
}
51 changes: 0 additions & 51 deletions src/common/vitals/long-task.js

This file was deleted.

3 changes: 1 addition & 2 deletions src/features/metrics/aggregate/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ export class Aggregate extends AggregateBase {
singleChecks () {
// report loaderType
const { distMethod, loaderType } = getRuntime(this.agentIdentifier)
const { proxy, privacy, page_view_timing } = getConfiguration(this.agentIdentifier)
const { proxy, privacy } = getConfiguration(this.agentIdentifier)

if (loaderType) this.storeSupportabilityMetrics(`Generic/LoaderType/${loaderType}/Detected`)
if (distMethod) this.storeSupportabilityMetrics(`Generic/DistMethod/${distMethod}/Detected`)
Expand All @@ -77,7 +77,6 @@ export class Aggregate extends AggregateBase {
})

if (!privacy.cookies_enabled) this.storeSupportabilityMetrics('Config/SessionTracking/Disabled')
if (page_view_timing.long_task) this.storeSupportabilityMetrics('Config/LongTask/Enabled')
} else if (isWorkerScope) {
this.storeSupportabilityMetrics('Generic/Runtime/Worker/Detected')
} else {
Expand Down
3 changes: 0 additions & 3 deletions src/features/page_view_timing/aggregate/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ import { firstPaint } from '../../../common/vitals/first-paint'
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'
import { longTask } from '../../../common/vitals/long-task'
import { subscribeToVisibilityChange } from '../../../common/window/page-visibility'
import { VITAL_NAMES } from '../../../common/vitals/constants'
import { EventBuffer } from '../../utils/event-buffer'
Expand All @@ -37,8 +36,6 @@ export class Aggregate extends AggregateBase {
this.timings = new EventBuffer()
this.curSessEndRecorded = false

if (getConfigurationValue(this.agentIdentifier, 'page_view_timing.long_task') === true) longTask.subscribe(this.#handleVitalMetric)

registerHandler('docHidden', msTimestamp => this.endCurrentSession(msTimestamp), this.featureName, this.ee)
registerHandler('winPagehide', msTimestamp => this.recordPageUnload(msTimestamp), this.featureName, this.ee)

Expand Down
5 changes: 0 additions & 5 deletions tests/assets/js/some-long-task.js

This file was deleted.

22 changes: 0 additions & 22 deletions tests/assets/long-tasks.html

This file was deleted.

36 changes: 1 addition & 35 deletions tests/specs/pvt/timings.e2e.js
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { supportsCumulativeLayoutShift, supportsFirstContentfulPaint, supportsFirstInputDelay, supportsFirstPaint, supportsInteractionToNextPaint, supportsLargestContentfulPaint, supportsLongTaskTiming } from '../../../tools/browser-matcher/common-matchers.mjs'
import { supportsCumulativeLayoutShift, supportsFirstContentfulPaint, 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'
Expand Down Expand Up @@ -181,38 +181,4 @@ describe('pvt timings tests', () => {
})
})
})

describe('long task related timings', () => {
loadersToTest.forEach(loader => {
it.withBrowsersMatching([supportsLongTaskTiming])(`emits long task timings when observed for ${loader} agent`, async () => {
await browser.url(
await browser.testHandle.assetURL('long-tasks.html', { loader, init: { page_view_timing: { long_task: true, harvestTimeSeconds: 3 } } })
)

const [timingsHarvests] = await Promise.all([
timingsCapture.waitForResult({ timeout: 10000 }),
browser.waitUntil(() => browser.execute(function () {
return window.tasksDone === true
}),
{
timeout: 10000,
timeoutMsg: 'tasksDone was never set'
}
)
])
const ltEvents = timingsHarvests
.flatMap(harvest => harvest.request.body)
.filter(timing => timing.name === 'lt')
expect(ltEvents.length).toBeGreaterThanOrEqual(2)

ltEvents.forEach(lt => {
// Attributes array should start with: [ltFrame, ltStart, ltCtr, (ltCtrSrc, ltCtrId, ltCtrName, )...]
expect(lt.value).toBeGreaterThanOrEqual(50)
expect(lt.attributes.length).toBeGreaterThanOrEqual(3)
expect(lt.attributes[1].type).toEqual('doubleAttribute') // entry.startTime
if (lt.attributes[2].value !== 'window') expect(lt.attributes.length).toBeGreaterThanOrEqual(6)
})
})
})
})
})
3 changes: 1 addition & 2 deletions tests/unit/common/config/init.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,8 +83,7 @@ test('init props exist and return expected defaults', () => {
expect(config.page_view_timing).toEqual({
autoStart: true,
enabled: true,
harvestTimeSeconds: 30,
long_task: false
harvestTimeSeconds: 30
})
expect(config.privacy).toEqual({
cookies_enabled: true
Expand Down
122 changes: 0 additions & 122 deletions tests/unit/common/vitals/long-task.test.js

This file was deleted.

4 changes: 0 additions & 4 deletions tools/browser-matcher/common-matchers.mjs
Original file line number Diff line number Diff line change
Expand Up @@ -152,8 +152,4 @@ export const supportsInteractionToNextPaint = new SpecMatcher()
.include('edge>=96')
.include('android>=9.0')

export const supportsLongTaskTiming = new SpecMatcher()
.include('chrome>=58')
.include('edge>=79')

export const supportsCumulativeLayoutShift = supportsLargestContentfulPaint

0 comments on commit 304e395

Please sign in to comment.