Skip to content

Conversation

@harp-intel
Copy link
Contributor

@harp-intel harp-intel commented Jan 27, 2026

Re-enables event and metric file override using the --eventfile and --metricfile flags for architectures that use the perfmon and component loaders. If one file is specified, the other must be too. When these files are specified, the legacy loader will be used.

It is no longer possible to override perfmon loader JSON files.

The metrics trim command will not work if data was initially collected using the override files.

This pull request refactors how metric and event definition overrides are handled, clarifies flag requirements, and simplifies loader logic. The changes ensure that metric and event override files must be specified together, remove unused override paths from non-legacy loaders, and streamline loader configuration.

The most important changes are grouped below:

Loader and Override Handling Improvements:

  • The NewLoader function now accepts a useLegacyLoader boolean to explicitly select the legacy loader when both metric and event override files are provided, ensuring consistent override behavior. [1] [2] [3] [4]
  • The ConfigFileOverride field was removed from LoaderConfig, and only the legacy loader now supports metric and event file overrides; component and perfmon loaders no longer process these overrides. [1] [2] [3] [4] [5]

Flag Validation and Documentation:

  • Validation was added to require both --metricfile and --eventfile flags to be specified together, preventing misconfiguration.
  • Help text for the --metricfile and --eventfile flags was updated to clarify that they must be used together.

Code Cleanup:

  • Unused imports (such as os) were removed from loader_component.go to clean up dependencies.

Signed-off-by: Harper, Jason M <jason.m.harper@intel.com>
@harp-intel harp-intel requested a review from Copilot January 27, 2026 20:29
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull request overview

This PR refactors the metric and event definition override mechanism to enforce that override files must be specified together and simplifies loader logic by restricting overrides to the legacy loader only.

Changes:

  • Added validation requiring both --metricfile and --eventfile flags to be specified together
  • Modified NewLoader to accept a useLegacyLoader boolean parameter that forces use of the legacy loader when overrides are provided
  • Removed override file support from perfmon and component loaders, restricting this functionality to the legacy loader only

Reviewed changes

Copilot reviewed 5 out of 5 changed files in this pull request and generated no comments.

Show a summary per file
File Description
cmd/metrics/loader.go Added useLegacyLoader parameter to NewLoader function and removed ConfigFileOverride from LoaderConfig
cmd/metrics/metrics.go Updated flag help text, added validation for paired override flags, updated NewLoader calls, and simplified getLoaderConfig to only handle legacy loader overrides
cmd/metrics/trim.go Updated NewLoader call to pass false for useLegacyLoader parameter
cmd/metrics/loader_perfmon.go Removed config file override support from perfmon loader
cmd/metrics/loader_component.go Removed metric definition override parameter from component loader and cleaned up unused import

@harp-intel harp-intel merged commit a62ba4f into main Jan 28, 2026
5 checks passed
@harp-intel harp-intel deleted the legacy-override branch January 28, 2026 21:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants