Skip to content

Conversation

harp-intel
Copy link
Contributor

@harp-intel harp-intel commented Aug 25, 2025

This pull request introduces support for threshold expressions in metrics and refactors metric naming to deprecate legacy names. It adds new fields to the MetricDefinition struct for threshold logic, updates how metric names are handled throughout the codebase, and makes several changes to ensure threshold expressions are parsed, transformed, and evaluated consistently. Additionally, it introduces a command-line flag to optionally use legacy metric names for backward compatibility.

Metric threshold support:

  • Added ThresholdExpression, ThresholdVariables, and ThresholdEvaluable fields to the MetricDefinition struct, enabling metrics to have associated threshold logic that is parsed and evaluated at runtime. [1] [2]
  • Implemented functions to extract, transform, and initialize threshold expressions and their variables, ensuring they are handled similarly to regular metric expressions. [1] [2] [3]
  • Added logic to parse and evaluate threshold expressions using govaluate and to transform their syntax for compatibility. [1] [2]

Metric naming refactor:

  • Deprecated the use of LegacyName in favor of MetricName throughout the codebase, updating filtering, lookup, and logging to use the new naming convention. [1] [2] [3] [4] [5] [6]
  • Added a --legacy-names command-line flag to optionally use legacy metric names, with clear marking that this is deprecated and will be removed in the future. [1] [2] [3] [4]

Metrics output and summary updates:

  • Updated output and summary functions (printMetrics, summarizeMetrics, etc.) to accept and utilize the full set of metric definitions, including threshold information, for more accurate reporting. [1] [2] [3] [4] [5]

General improvements:

  • Improved transformation and parsing of metric and threshold expressions for consistency, including syntax normalization and conditional logic conversion.
  • Enhanced error handling and logging to provide clearer diagnostics when metrics or threshold expressions fail to parse or contain uncollectable events. [1] [2] [3] [4]

These changes collectively improve the flexibility and robustness of metric handling, especially for advanced use cases involving thresholds, while moving the project away from legacy naming conventions.

… deprecated. All metric descriptions in HTML report.

Signed-off-by: Harper, Jason M <[email protected]>
@harp-intel harp-intel requested a review from Copilot August 25, 2025 22:31
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

This PR modernizes metric naming to match perfmon standards by removing legacy names as the primary metric identifiers. Legacy names are now optional and deprecated, supporting backward compatibility through a new --legacy-names flag. Additionally, metric descriptions are now included in HTML reports for enhanced usability.

  • Updated metric naming system to use modern perfmon names by default
  • Introduced optional --legacy-names flag for backward compatibility
  • Enhanced HTML reports with metric descriptions for better user experience

Reviewed Changes

Copilot reviewed 11 out of 11 changed files in this pull request and generated 1 comment.

Show a summary per file
File Description
cmd/metrics/summary.go Added metricDefinitions parameter to summary functions and updated template variable naming
cmd/metrics/resources/perfmon/srf/srf.json Removed LegacyName fields from metric definitions
cmd/metrics/resources/perfmon/spr/spr.json Removed LegacyName fields from metric definitions
cmd/metrics/resources/perfmon/icx/icx.json Removed LegacyName fields from metric definitions
cmd/metrics/resources/perfmon/gnr/gnr.json Removed LegacyName fields from metric definitions
cmd/metrics/resources/perfmon/emr/emr.json Removed LegacyName fields from metric definitions
cmd/metrics/resources/base.html Replaced hardcoded metric descriptions with dynamic template variable
cmd/metrics/print.go Added metric name translation functions and updated output format functions
cmd/metrics/metrics.go Added legacy names flag and updated function calls
cmd/metrics/loader_perfmon.go Updated metric lookups to use MetricName instead of LegacyName
cmd/metrics/loader.go Extended MetricDefinition struct with additional fields

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

@harp-intel harp-intel changed the title Metric names modernized to match perfmon. "Legacy" names optional and… perfmon Metric names and descriptions Aug 25, 2025
Signed-off-by: Harper, Jason M <[email protected]>
Signed-off-by: Harper, Jason M <[email protected]>
Signed-off-by: Harper, Jason M <[email protected]>
@harp-intel harp-intel changed the title perfmon Metric names and descriptions perfmon Metric names, descriptions, and thresholds Aug 27, 2025
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.

Include a description for every metric listed in the summary HTML, all metrics tab. add metrics insights
1 participant