codecov / engineering-team

This is a general repo to use with GH Projects
1 stars 1 forks source link

Bundle caching (carryforward bundle style) #1707

Closed aj-codecov closed 1 week ago

aj-codecov commented 4 months ago

Problem to solve: When bundle data is not uploaded on a commit we report a bundle size of 0 and a massive drop in size (we actually say the bundle was removed, which can be pretty concerning). This prevents reliable bundle size over time record keeping. This is the same problem we encounter with code coverage if somebody is not using carryforward flags.

Proposed Solution: Cache bundle data from commit to commit and make sure we're not reporting 0s when we have a history of bundle data and knowledge that it likely hasn't changed from the prior commit. We then want to surface this on the PR/bundle tab. This should be the default config for new bundle users.

WIP: view designs

### Backend
- [ ] https://github.com/codecov/engineering-team/issues/2034
- [ ] https://github.com/codecov/engineering-team/issues/2038
- [ ] https://github.com/codecov/engineering-team/issues/2039
- [ ] https://github.com/codecov/engineering-team/issues/2041
- [ ] https://github.com/codecov/engineering-team/issues/2036
- [ ] https://github.com/codecov/engineering-team/issues/2037
- [ ] https://github.com/codecov/engineering-team/issues/2035
- [ ] https://github.com/codecov/engineering-team/issues/2040
- [ ] https://github.com/codecov/engineering-team/issues/2045
- [ ] https://github.com/codecov/engineering-team/issues/2055
aj-codecov commented 3 months ago

@codecovdesign I think the UI implications here are small, but important, just making sure people understand the source of the data when we "carryforward" a bundle, what commit/build did it come from if it's not the most recent and when was that build made.

nicholas-codecov commented 3 months ago

Couple of questions:

codecovdesign commented 3 months ago

How do we turn bundle caching on/off?

my understanding is this would be default behavior that doesn't have on/off option. is this not correct?

How do we remove a bundle from being cached (let's say they stop working on one piece)?

can you elaborate, I'm not understanding the scenario 🤔

nicholas-codecov commented 3 months ago

my understanding is this would be default behavior that doesn't have on/off option. is this not correct?

I wasn't under this understanding, tho we can explore it

can you elaborate, I'm not understanding the scenario 🤔

Let's say we stop supporting Rollup in the bundler plugins, we're not longer uploading stats for it, but we're still uploading stats for everything else. Why would we continue to carry that along. So how do we stop it from being carry-forwarded

codecovdesign commented 3 months ago

, tho we can explore it

Is there a blocker from moving forward with the carryforward for now? it's a convention over configuration approach, but perhaps a later iteration looks at on/off as/if needed?

Let's say we stop supporting Rollup in the bundler plugins

Is this an active scenario or accounting for a possible on? are we considering stopping support for Rollup?

nicholas-codecov commented 3 months ago

Is there a blocker from moving forward with the carryforward for now? it's a convention over configuration approach, but perhaps a later iteration looks at on/off as/if needed?

This is a fairly big change in behaviour, and if we do this by default moving forward this could significantly increase storage requirements.

Is this an active scenario or accounting for a possible on? are we considering stopping support for Rollup?

Just a hypothetical, we publish a package for Rollup and collect stats for it, if we stop making that package for whatever reason we don't want it to be carried forward anymore.

aj-codecov commented 2 months ago

@codecovdesign this is analogous to carryforward flags in coverage land, in that spirit, Nick is asking valid questions - we know it's difficult to remove coverage flags set to carryforward today with it requiring both a UI component and a yaml component, we should think through how this might work for bundle (not a blocker to shipping though IMO). I think it makes sense that bundle caching is a default, but could be disabled if a user chooses (I feel the same about CFF as well...).

I think the more likely scenario is a user just no longer wants to carryforward stats for a bundle, it could be for any number of reasons, but the most common if I had to guess is a user setting something up, realizing it's not quite right, starting over and wanting to wipe the old. Similarly, configuring with a test project and then moving to a "main" project. We see this all the time with coverage and it's hard to imagine it being terribly different for bundles.

@nicholas-codecov Does this make sense as a possible bundle_caching: false in the config.js?

I think the follow up should be supporting a deletion/wipe clean scenario, but isn't a blocker to initial launch

nicholas-codecov commented 2 months ago

Does this make sense as a possible bundle_caching: false in the config.js?

@aj-codecov The issue with this, is that they'd be required to do an extra upload with this, which is something they may forget to do, and if they've removed everything then they have to introduce a new config to stop it. Whereas if we had a way through the UI then we can link/inform them through docs and the PR comment if they're using bundle caching

aj-codecov commented 2 months ago

@nicholas-codecov I'm not sure what you mean by extra upload, can you explain?

nicholas-codecov commented 2 months ago

if they've set bundle caching to true, in their bundler config, and then deleted said config because they're no longer maintaining it, how're we supposed to know that they don't want it to be carried forward anymore?

nicholas-codecov commented 2 months ago

@codecovdesign left some comments on the designs

nicholas-codecov commented 1 week ago

All the tasks here are done, so closing out this issue.