Skip to content
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

Some web vitals are missing #14201

Open
3 tasks done
RexSkz opened this issue Nov 7, 2024 · 3 comments
Open
3 tasks done

Some web vitals are missing #14201

RexSkz opened this issue Nov 7, 2024 · 3 comments

Comments

@RexSkz
Copy link

RexSkz commented Nov 7, 2024

Is there an existing issue for this?

How do you use Sentry?

Sentry Saas (sentry.io)

Which SDK are you using?

Sentry Browser CDN bundle

SDK Version

8.37.0

Framework Version

React 17.0.2

Link to Sentry event

https://moego-ey.sentry.io/discover/homepage/?dataset=transactions&field=title&field=project&field=timestamp&field=measurements.fcp&field=measurements.cls&field=measurements.lcp&field=measurements.ttfb&field=measurements.fp&name=All%20Errors&project=4507922008702976&query=transaction.op%3Apageload&queryDataset=transaction-like&sort=-timestamp&statsPeriod=7d&yAxis=count%28%29

Reproduction Example/SDK Setup

Sentry.init({
  dsn: __SENTRY_DSN__,
  enabled,
  debug,
  environment,
  release,

  tracesSampleRate: 1,

  integrations: [
    window.Sentry.breadcrumbsIntegration({
      console: true,
      dom: { serializeAttribute: ['data-testid', 'data-slot'] },
      fetch: true,
      history: true,
      xhr: true,
    }),
    window.Sentry.browserTracingIntegration?.(),
    window.Sentry.extraErrorDataIntegration?.(),
  ],
  ignoreErrors: [
    'CanceledError',
    'ResizeObserver loop limit exceeded',
  ],
  beforeSend(event, hint) {
    const exception = event.exception?.values?.[0];
    if (
      exception &&
      exception.type === 'UnhandledRejection' &&
      exception.value === 'Object captured as promise rejection with keys: config, data, headers, request, status'
    ) {
      // already reportd in other libraries
      return null;
    }
    return event;
  },
});

Steps to Reproduce

Just open the page. But I noticed a scenario that may trigger the problem: our project will trigger a react-router change to add 2 queries immediately in the App's useEffect:

const history = useHistory();
history.push(`/current-path?query1=value1&query2=value2`);

Expected Result

FCP, LCP, and TTFB should be displayed correctly on the Web Vitals page.

Actual Result

They are missing, only an INP metric is displayed.

After some investigation, FCP, LCP and TTFB were generated, but for some reason, they aren't attached to the pageload event, instead of a navigation event.

@Lms24
Copy link
Member

Lms24 commented Nov 7, 2024

Hey @RexSkz thanks for writing in!

The link you posted only links to a discover link for me, so I don't know which specific transaction to inspect. Do you by any chance have a publicly accessible route in your app that I could access?

Image

But I noticed a scenario that may trigger the problem: our project will trigger a react-router change to add 2 queries immediately in the App's useEffect:

That's a good lead! Without having access to a route where this would happen, I can't say for sure but I have an idea: When you make a react router navigation, the SDK probably cancels the intial pageload transaction to which the web vital measurements would get attached to. It then starts a new navigation transaction for the RR navigation. However, we only attach web vital measurements to the pageload transaction, so they're lost. As I said, this is only a theory but if you turn on debug: true, you should get debug logs about started and stopped spans/transactions, as well as web vital measurements.

Also, I noticed you're using the browser SDK CDN bundles in a React SPA app(?). Is there a specific reason for using the bundle over our NPM packages? We have a dedicated @sentry/react NPM package which includes React Router-specific instrumentations: https://docs.sentry.io/platforms/javascript/guides/react/features/react-router/ Might be worth looking into these. The default browserTracingIntegration that's included in your CDN bundle will only listen to window.history changes instead of react router events.

@RexSkz
Copy link
Author

RexSkz commented Nov 8, 2024

@Lms24 Thanks for your detailed explanation!

Our platform may need a paid account to reproduce the issue. I can't provide a public link for you, but I'll investigate according to your theory and share as much information as possible here.

I've done some research before according to Sentry's support team. I set debug: true and saw FCP logged into console, but it appears inside a navigation event instead of pageload event.

The reason why we use CDN is complicated, but more like a historical reason. We're migrating from CDN to NPM now.

@Lms24
Copy link
Member

Lms24 commented Nov 8, 2024

Thanks for the information! If you find out more let me know!

I'll link to some relevant code pieces for more context.

  1. Here's where we end an ongoing (pageload) span if we start a new navigation span:

    client.on('startNavigationSpan', startSpanOptions => {
    if (getClient() !== client) {
    return;
    }
    if (activeSpan && !spanToJSON(activeSpan).timestamp) {
    DEBUG_BUILD && logger.log(`[Tracing] Finishing current root span with op: ${spanToJSON(activeSpan).op}`);
    // If there's an open transaction on the scope, we need to finish it before creating an new one.
    activeSpan.end();
    }
    activeSpan = _createRouteSpan(client, {
    op: 'navigation',
    ...startSpanOptions,
    });
    });

  2. Here's where we decide when to start a navigation span in the base browser SDK (your current SDK):

    if (instrumentNavigation) {
    addHistoryInstrumentationHandler(({ to, from }) => {
    /**
    * This early return is there to account for some cases where a navigation transaction starts right after
    * long-running pageload. We make sure that if `from` is undefined and a valid `startingURL` exists, we don't
    * create an uneccessary navigation transaction.
    *
    * This was hard to duplicate, but this behavior stopped as soon as this fix was applied. This issue might also
    * only be caused in certain development environments where the usage of a hot module reloader is causing
    * errors.
    */
    if (from === undefined && startingUrl && startingUrl.indexOf(to) !== -1) {
    startingUrl = undefined;
    return;
    }
    if (from !== to) {
    startingUrl = undefined;
    startBrowserTracingNavigationSpan(client, {
    name: WINDOW.location.pathname,
    attributes: {
    [SEMANTIC_ATTRIBUTE_SENTRY_SOURCE]: 'url',
    [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.navigation.browser',
    },
    });
    }
    });
    }
    }

(As you can see, we don't always start a navigation span on every history change but from looking at it, two subsequent navigations might slip through here and in fact start a navigation span)

  1. Here's where we add web vital measurements to a span:
    if (op === 'pageload') {
    _addTtfbRequestTimeToMeasurements(_measurements);
    const fidMark = _measurements['mark.fid'];
    if (fidMark && _measurements['fid']) {
    // create span for FID
    startAndEndSpan(span, fidMark.value, fidMark.value + msToSec(_measurements['fid'].value), {
    name: 'first input delay',
    op: 'ui.action',
    attributes: {
    [SEMANTIC_ATTRIBUTE_SENTRY_ORIGIN]: 'auto.ui.browser.metrics',
    },
    });
    // Delete mark.fid as we don't want it to be part of final payload
    delete _measurements['mark.fid'];
    }
    // If FCP is not recorded we should not record the cls value
    // according to the new definition of CLS.
    // TODO: Check if the first condition is still necessary: `onCLS` already only fires once `onFCP` was called.
    if (!('fcp' in _measurements) || !options.recordClsOnPageloadSpan) {
    delete _measurements.cls;
    }
    Object.entries(_measurements).forEach(([measurementName, measurement]) => {
    setMeasurement(measurementName, measurement.value, measurement.unit);
    });
    // Set timeOrigin which denotes the timestamp which to base the LCP/FCP/FP/TTFB measurements on
    span.setAttribute('performance.timeOrigin', timeOrigin);
    // In prerendering scenarios, where a page might be prefetched and pre-rendered before the user clicks the link,
    // the navigation starts earlier than when the user clicks it. Web Vitals should always be based on the
    // user-perceived time, so they are not reported from the actual start of the navigation, but rather from the
    // time where the user actively started the navigation, for example by clicking a link.
    // This is user action is called "activation" and the time between navigation and activation is stored in
    // the `activationStart` attribute of the "navigation" PerformanceEntry.
    span.setAttribute('performance.activationStart', getActivationStart());
    _setWebVitalAttributes(span);
    }

Note that we explicitly check for the span op to be pageload, meaning, navigation spans don't get web vital measurements attached

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Status: Waiting for: Community
Development

No branches or pull requests

2 participants