Skip to content

Conversation

harp-intel
Copy link
Contributor

The event group ordering must be consistent for the metrics post-processing (--input) feature to work.

This pull request refactors the metrics loader component for improved determinism and maintainability. The main changes focus on removing unused filtering logic and ensuring deterministic processing and grouping of metric events.

Refactoring and code cleanup:

  • Removed the filterUncollectableMetrics and identifyUncollectableEvents functions, as well as their usage, simplifying the metric loading process by assuming all events are collectable. [1] [2]

Deterministic processing:

  • Sorted directory entries by name in loadEventDefinitions to ensure a consistent and deterministic order when processing event definition files.
  • Sorted metric variable names before grouping to guarantee deterministic group formation.
  • Updated group sorting logic in mergeSmallGroups to be deterministic, including a panic for empty groups and consistent tie-breaking.

Dependency updates:

  • Added the io/fs import to support directory entry sorting.

Signed-off-by: Harper, Jason M <jason.m.harper@intel.com>
@harp-intel harp-intel requested a review from Copilot October 15, 2025 14:39
Copy link
Contributor

@Copilot 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

Ensures deterministic formation and ordering of metric event groups by removing unused filtering logic and introducing explicit sorting steps.

  • Added deterministic sorting of directory entries and metric variable names.
  • Modified group merge sorting logic for determinism, with added panic on empty groups.
  • Removed uncollectable metrics filtering functions and related calls.

Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.

Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
@harp-intel harp-intel merged commit 7a40e8f into main Oct 15, 2025
5 checks passed
@harp-intel harp-intel deleted the armstable branch October 15, 2025 18:25
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.

1 participant