Changed CompoundPinotMetricsFactory to use PluginManager instead of Class.forName

[gortiz]

This is a small PR created to be an example of the kind of migrations we should apply in order to be able to use the improvements proposed in #13930, which would let us run Pinot without shading.

[codecov-commenter]

Codecov Report

Attention: Patch coverage is 0% with 1 line in your changes missing coverage. Please review.

Project coverage is 63.77%. Comparing base (59551e4) to head (b3192f5).
Report is 1302 commits behind head on master.

Files with missing lines	Patch %	Lines
.../metrics/compound/CompoundPinotMetricsFactory.java	0.00%	1 Missing ⚠️

Additional details and impacted files

@@             Coverage Diff              @@
##             master   #14402      +/-   ##
============================================
+ Coverage     61.75%   63.77%   +2.02%     
- Complexity      207     1557    +1350     
============================================
  Files          2436     2660     +224     
  Lines        133233   145976   +12743     
  Branches      20636    22356    +1720     
============================================
+ Hits          82274    93093   +10819     
- Misses        44911    46000    +1089     
- Partials       6048     6883     +835     

Flag	Coverage Δ
custom-integration1	100.00% <ø> (+99.99%)	⬆️
integration	100.00% <ø> (+99.99%)	⬆️
integration1	100.00% <ø> (+99.99%)	⬆️
integration2	0.00% <ø> (ø)
java-11	63.75% <0.00%> (+2.04%)	⬆️
java-21	63.64% <0.00%> (+2.01%)	⬆️
skip-bytebuffers-false	63.77% <0.00%> (+2.02%)	⬆️
skip-bytebuffers-true	63.62% <0.00%> (+35.89%)	⬆️
temurin	63.77% <0.00%> (+2.02%)	⬆️
unittests	63.76% <0.00%> (+2.02%)	⬆️
unittests1	55.48% <ø> (+8.59%)	⬆️
unittests2	34.14% <0.00%> (+6.41%)	⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

[Jackie-Jiang]

Good catch!
Essentially everything potentially loaded from plugin-ins should be done via PluginManager

[yashmayya]

example of the kind of migrations we should apply in order to be able to use the improvements proposed in #13930, which would let us run Pinot without shading.

These changes are definitely necessary but not sufficient to ensure the above - we can still run into dependency conflict issues between plugin dependencies and Pinot dependencies if we don't use shading (alternative would be to introduce full classloader isolation where plugin dependencies aren't added to the main application classpath).

[gortiz]

These changes are definitely necessary but not sufficient to ensure the above - we can still run into dependency conflict issues between plugin dependencies and Pinot dependencies if we don't use shading (alternative would be to introduce full classloader isolation where plugin dependencies aren't added to the main application classpath).

Yeah, sure. This is necessary but not sufficient. We still need to change the packaging and use the new classpath realm system. But we cannot turn it on until we actually apply this change everywhere we can load plugins. This include plugins loaded with ServiceLoader (which I encouraged in the past), which will need to be rewritten to use PluginManager instead of ServiceLoader

[gortiz]

Anyway, remember we are NOT using shading right now. We are shading (paying the cost in terms of build time and artifact size) but changing all dependencies with the same prefix, so that shading is not preventing any issue between shaded plugins and Pinot