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

Sort out new solution for monitoring bundlesize. #10472

Open
paulirish opened this issue Mar 16, 2020 · 15 comments
Open

Sort out new solution for monitoring bundlesize. #10472

paulirish opened this issue Mar 16, 2020 · 15 comments
Assignees
Labels

Comments

@paulirish
Copy link
Member

paulirish commented Mar 16, 2020

#1690 was what triggered us to add bundlesize.

Now it's not adding much value. it blocks PRs and we don't see the incremental changes. Also it pings an endpoint on every CI run that 500s and spams our test log. :( We need something better.

A few solutions (some mentioned in #10272):

I'm very attracted to buildtracker as it charts things over time and not just reporting on this specific one.

@XhmikosR
Copy link
Contributor

For what is worth, bundlewatch does look like a good replacement, assuming one is OK with the features it provides. It is smaller for sure: https://packagephobia.now.sh/result?p=bundlesize%2Cbundlewatch

@addyosmani
Copy link
Member

BuildTracker is probably the most comprehensive bundle monitoring solution I've seen to date. I'd +1 it. bundlewatch also seems OK.

@connorjclark
Copy link
Collaborator

We don't end up in anyone's bundles so TBH idc about bundle size. I'd +1 just removing it. At most, maybe we could have a shell script that checks the CDT bundle is <2MB or w.e.

@brendankenny
Copy link
Member

I mean, you're saying this because you just want buildtracker :P

We could just drop in bundlewatch to clean up our CI while setting up buildtracker, unless you want to get it set up right away. Back in #9089 I think I ran into having to enable bundlewatch as a Github app for our org or something, but a lot of github stuff has changed since then (and maybe @addyosmani has that power?).

@XhmikosR
Copy link
Contributor

XhmikosR commented Mar 17, 2020

Now there's even an action available https://github.com/jackyef/bundlewatch-gh-action

EDIT: Although, I'm not sure it follows the best GH Actions practices or if you'd like to rely on a third-party action; I just thought I'd mention it.

@patrickhulce
Copy link
Collaborator

I am also +1 to just removing. The last several times that we've hit it or saw large increases the result has always been "we don't really care", so why bother tracking something we don't take action on.

@brendankenny
Copy link
Member

The last several times that we've hit it or saw large increases the result has always been "we don't really care", so why bother tracking something we don't take action on.

well I agree with your conclusion but I'm disappointed I wasn't there for the "we don't really care" conversation that's the premise of it :)

This is how "Lighthouse is warming up" gets longer and longer and we end up with brainstorming ugly build hacks a year from now to work around laziness today. Bundle size monitoring is good backpressure to try to trim down new additions and prune old stuff occasionally (though it would be nice if there was more cultural support for this). We need a way to be able to spot the "whoa, what's accidentally getting added here" situations even if most of the time the answer is, "sure, adding lots of stack pack strings will do that" and not "maybe we don't need several hundred KB to check a data format everyone already has tools for".

@patrickhulce
Copy link
Collaborator

Sparked again by #10544

spot the "whoa, what's accidentally getting added here" situations

Agreed having something that does this is useful but having an arbitrary threshold that fails doesn't seem like the way to do it.

It seems like what we really want here is something like Google CLA bot that flags a PR with a big obnoxious comment that this increases the bundle by more than X KB and requires explicit approval from an owner "I consent to increasing the bundle size" before it can proceed.

@brendankenny
Copy link
Member

Sparked again by #10544

I like your idea, but I'll also feel more sympathy about team members having to bump a single number when anyone works to make the proto roundtrip test not a new contributor nightmare :P

@patrickhulce
Copy link
Collaborator

about team members having to bump a single number

That's not my largest concern, my largest concern is pointing the finger at the correct contributions and not whatever happens to push it over the edge.

@brendankenny
Copy link
Member

https://github.com/preactjs/compressed-size-action might actually be most of the way there

@exterkamp
Copy link
Member

Followups:

Build tracker supports budgets (and some nice reporting options), however we can add that on later after the data collection has begun.
Remove bundlesize

From #10550

@paulirish
Copy link
Member Author

paulirish commented Jun 23, 2020

Remaining

  • add bundlesize thresholds to buildtracker config
  • collect master branch data (on push)
  • collect all branches data (on PR push)
  • fix merge-base issue on PRs misc(build): give build-tracker a shared git history on PRs #11449
  • Determine what to do about data from PRs (and cross-repo PRs) now..
    • The BT_API_AUTH_TOKEN secret is needed to upload but cross-repo PRs can't access it. Do we care? Probably not, because we don't have a lot of these PRs, though other projects that do likely would. At the same time, what sort of risk do we run if the BT_API_AUTH_TOKEN were public? 😈
    • We're collecting branch data on our PRs, but it isn't giving us any value right now.. Mostly because of the next point..
  • Do something with the budget warnings.
    • The BT CLI output gives us a JSON dump that, on master, currently has two warnings:
      • ⚠️: dist/lightrider/lighthouse-lr-bundle.js failed the gzip budget size limit of 1,464.84 KiB by 388.24 KiB

      • ⚠️: dist/lightrider/report-generator-bundle.js failed the gzip budget size limit of 48.83 KiB by 16.57 KiB

    • there's no formatting of this output.. and we're not doing anything with it. Github CI doesn't have the idea of "warning". So this signal is getting lost. I guess we have to flip the WARN to ERROR if we want to notice it. :/ I don't see other options
    • related upstream issue: clearer violations in the UI: Show budget violations in the graph UI paularmstrong/build-tracker#217

@paularmstrong
Copy link

paularmstrong commented Jan 15, 2021

At the same time, what sort of risk do we run if the BT_API_AUTH_TOKEN were public?

You could end up with someone injecting bogus data in your database. The only thing it allows is write access.

Your comfort level with the possibility of needing to remove some garbage is really all that's at stake (until a future release when it may be possible to delete data, or some other feature is added)

edit: you could also just remove the BT_API_AUTH_TOKEN if you're comfortable with anyone being able to write new stuff. Also note that there should be a sufficient amount of validation already in place server-side when receiving data to ensure it's at least well-formed.

@paulirish
Copy link
Member Author

paulirish commented Dec 20, 2023

~3 years later and we might be in a similar situation.

https://buildtracker.dev/ is definitely closest to what we want, but...

  • heroku is the straightforward integration but their free tier is gone since April.
  • it does have more complexity than other solutions.. and paul noted recently it's unloved maintenance-wise.
  • it's probably the best solution for keeping history over time. We've relied on this history before.
  • misc(build): format build-tracker results and post to gh status api #12925 was an attempt to use the Github Status API to report the diff. Kinda cool, but.. bespoke.
  • our integration also broke when we went ESM. that seems to be resolvable with a rename to build-tracker.config.cjs plus upgrading to the 1.0. but heroku still being "down" makes this fairly moot.

Other options:

Part of me thinks the solution to history is using git as a db: updating some csv/json in a repo. For example, this PR tracking service (data here) hasn't broken since I set it up 5 years ago.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

9 participants