build-plugins Add a way to selectively disable collection of certain metrics (pre-filtering)

Hi! I just finished trying out v2 of the esbuild plugin on our codebase and I'm happy to say that it works out-of-the-box after the recent refactor. Great work!

I would like to have datadog telemetry reporting happening silently in the background on all developer machines, so we can keep an eye on build times across all of our dev machines and make adjustments when necessary.

However, this plugin seems to make a lot of hooks by default that really slow down build performance. Here's our esbuild bundle time without this plugin (1.9s):

and here it is with this plugin enabled (2.8s):

That's 1/3 more time spent bundling with telemetry collection enabled, which is my opinion too high of a price to pay. I would not feel comfortable enabling telemetry collection on development machines with it causing that kind of slowdown.

I tried looking for a way to disable certain performance-heavy metrics to retain only ones that are light on build time impact, but I can't find a way to do so. Filtering seems to only take effect post-collection, and it seems that the only options available to me are collecting all metrics or collecting none at all.

Feature Request

Please add a way for me to choose what metrics I want the plugin to collect and report before filtering takes place, along with some guidance on which metrics are performance-heavy.

Jun 06 '24 19:06 Cldfire

Thanks for reaching out, that's a great suggestion.

Indeed the filtering currently happens post-collection. Historically, the filtering was about not sending too many metrics, hence why it's like that.

But I can definitely see the need to offer some "selection" of what to actually monitor.

Right now, we offer three types of monitoring in the telemetry module:

Plugins
Modules
Stats

I believe we can offer to enable only the ones you'd want. In your case, I believe it would only be "Bundler stats" which should not add any runtime.

Does that sound like a good plan to you @Cldfire?

Jun 20 '24 12:06 yoannmoinet

🙏 @Cldfire can you also give me the timing with the plugin in, but with the telemetry module disabled:

datadogEsbuildPlugin({
    telemetry: {
        disabled: true
    }
});

Just to confirm that the added time is actually coming from the telemetry module.

Jun 20 '24 12:06 yoannmoinet

Hey @yoannmoinet! Yeah, that plan sounds great to me 😄.

Here's the timing with telemetry enabled:

and disabled:

(Note that in the time since filing this issue I've moved from a M1 Max machine to a M3 Max machine, which is why the numbers are different.)

Jun 20 '24 16:06 Cldfire

I tried to implement this in a large Webpack project. Running with telemetry: false works well and doesn't slow the build times at all (I benchmarked using hyperfine). Running it with telemetry enabled causes OOMs in Node.js 😬 I'd also be interested in just pulling stats!

Jul 18 '24 21:07 raygesualdo

Nice, I love seeing more traction for this. I can assure you this is the next item on my list.

I'm currently finalising #86, then release, and next I'll start working on an update of the telemetry module.

In the meantime, just to orient myself in the right direction, could you, @Cldfire and @raygesualdo, tell me which data in particular are you most interested in.

Just to be sure I don't miss the whole point of this 😄

Thanks a ton for sticking around 🙇

Jul 19 '24 07:07 yoannmoinet

Build duration is my primary concern, but top-level counts are helpful as well. Here's what my filter array was:

filters: [
  ...helpers.telemetry.filters,
  (metric) => {
    if (
      metric.metric === 'assets.count' ||
      metric.metris === 'chunks.count' ||
      metric.metric === 'modules.count' ||
      metric.metric === 'entries.count' ||
      metric.metric === 'loaders.count' ||
      metric.metric === 'plugins.count' ||
      metric.metric === 'compilation.duration'
    ) {
      return metric
    }
    return null
  },
],

Jul 19 '24 12:07 raygesualdo

@yoannmoinet saw #86 got merged. Any word on when this might make it in?

Sep 03 '24 17:09 raygesualdo

@raygesualdo I'm still currently working on it.

It's pretty extensive as I'm re-writing most of the telemetry module to follow the earlier refactor of the whole package (universal support).

You can tag along in https://github.com/DataDog/build-plugins/pull/90 to see where I'm at, it will probably be split in two different PRs at some point.

I expect to have this ready for review in the coming days or next week.

It's been pretty difficult to reconciliate all the bundlers under the same umbrella as they all are doing things differently 😓, but I'm getting there.

Really sorry for the delay, I can assure you this has been my main priority these past few months, and I'm really trying to make the transition as smooth as possible.

Sep 04 '24 07:09 yoannmoinet

@yoannmoinet No worries! Appreciate your work on all of this!

Sep 04 '24 11:09 raygesualdo

Quick update.

2 PRs are now in review.

https://github.com/DataDog/build-plugins/pull/92
https://github.com/DataDog/build-plugins/pull/93

I expect them to be merged/released in the coming days.

I'll write up a proper release note with all the changes, but the big ones related to this issue is mostly:

Now enbaledTracing is off by default, which remove all the metrics loaders.*, plugins.* which were are only available to eslint and webpack but were using a lot of process to be computed. You can have enableTracing: true to have them back, with the additional overhead.

I'll try to at least bring back the counts (plugins.count and loaders.count) but they might merge together, as loaders is a webpack only thing.

For the same reason, chunks.* are now merged into assets.*.