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

Cap TTFB in attribution #440

Merged
merged 4 commits into from Mar 19, 2024
Merged

Cap TTFB in attribution #440

merged 4 commits into from Mar 19, 2024

Conversation

tunetheweb
Copy link
Member

Fixes #439

Copy link
Member

@philipwalton philipwalton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it'd be better to extract the logic in the ofTTFB() into a helper in /src/lib/ and then all of these files could reuse the same logic (and also point to the bugs that explain why the logic is there).

@philipwalton
Copy link
Member

...thinking about this abit more (and having my memory jogged after looking at the bugs in the source code comments), I think it might be better to not report attribution data in cases where the TTFB value is not trustworthy (since an incorrect TTFB value would impact the accuracy of other sub-part timing values as well).

We don't report the TTFB metric in these cases, and I think it would make sense to do that here as well.

@tunetheweb
Copy link
Member Author

OK re-did this to load from single lib function and also return without an attribution model when the TTFB is invalid.

src/lib/invalidTiming.ts Outdated Show resolved Hide resolved
@philipwalton philipwalton merged commit 398f331 into main Mar 19, 2024
4 checks passed
@philipwalton philipwalton deleted the cap-ttfb-in-attribution branch March 19, 2024 18:21
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
2 participants