Standardized support for filtering metrics using verbosity levels, inclusion and exclusion regex.#636
Standardized support for filtering metrics using verbosity levels, inclusion and exclusion regex.#636RakeshwarK wants to merge 2 commits intomicrosoft:mainfrom
Conversation
RakeshwarK
changed the title
There was a problem hiding this comment.
Pull request overview
This PR introduces standardized metric filtering capabilities to the Virtual Client, allowing users to control which metrics are captured and logged using verbosity-based filtering, regex-based inclusion filtering, and exclusion filtering. The changes affect the Fio, DiskSpd, and Memtier workload parsers, along with core metric filtering infrastructure and comprehensive documentation.
Changes:
- Added comprehensive metric filtering documentation to the profiles guide with examples for verbosity, inclusion, and exclusion filtering
- Updated verbosity level definitions from the previous 0-2 scale to a new 1-5 scale (with only 1, 3, and 5 actively used in modified workloads)
- Enhanced MetricExtensions.FilterBy() to support verbosity filtering, exclusion filters (prefix with "-"), and regex-based name filtering
- Updated Fio, DiskSpd, and Memtier metric parsers to assign appropriate verbosity levels to metrics
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
| website/docs/guides/0011-profiles.md | Added comprehensive documentation section on metric filtering including verbosity levels, regex patterns, exclusion filters, and combined filter examples |
| src/VirtualClient/VirtualClient.Contracts/Metric.cs | Updated XML documentation for Verbosity property to describe new 1-5 scale with detailed descriptions for each level |
| src/VirtualClient/VirtualClient.Contracts/Extensions/MetricExtensions.cs | Enhanced FilterBy() method to support verbosity filtering, exclusion filters, and inclusion filters; updated LogConsole() to use verbosity level 1 instead of 0 for critical metrics |
| src/VirtualClient/VirtualClient.Contracts.UnitTests/MetricExtensionsTests.cs | Added comprehensive test coverage for new filtering capabilities including verbosity levels, exclusion filters, and complex filter combinations |
| src/VirtualClient/VirtualClient.Actions/Memtier/MemtierMetricsParser.cs | Updated metric verbosity levels from 0-2 to 1/3/5 scale; added missing metricUnit to Hits/sec and Misses/sec metrics |
| src/VirtualClient/VirtualClient.Actions/FIO/FioMetricsParser.cs | Updated all FIO metrics to use new verbosity levels: 1 for critical metrics (bandwidth, IOPS, p50/p99), 3 for detailed percentiles (p70, p90, p99.9), and 5 for diagnostic metrics (histograms, stddev, byte/IO counts) |
| src/VirtualClient/VirtualClient.Actions/DiskSpd/DiskSpdMetricsParser.cs | Updated DiskSpd metrics to use new verbosity levels: 1 for critical metrics (bandwidth, IOPS, latency percentiles), 5 for diagnostic metrics (CPU usage, byte/IO counts) |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| /// Verbosity levels (only 1, 3, and 5 are currently used; 2 and 4 are reserved): | ||
| /// - 1 (Standard/Critical): Most important metrics - bandwidth, throughput, IOPS, key latency percentiles (p50, p99) | ||
| /// - 2 (Reserved): Reserved for future expansion | ||
| /// - 3 (Detailed): Additional detailed metrics - supplementary percentiles (p70, p90, p95, p99.9) | ||
| /// - 4 (Reserved): Reserved for future expansion |
There was a problem hiding this comment.
The statement "only levels 1, 3, and 5 are currently used" is inaccurate. Many other workload parsers in the codebase still use verbosity level 2 (e.g., BombardierMetricsParser, NTttcpMetricsParser, SockPerfMetricsParser, etc.). This statement should either be scoped to the specific workloads being modified in this PR (Fio, DiskSpd, Memtier) or removed entirely to avoid confusion.
| /// Verbosity levels (only 1, 3, and 5 are currently used; 2 and 4 are reserved): | |
| /// - 1 (Standard/Critical): Most important metrics - bandwidth, throughput, IOPS, key latency percentiles (p50, p99) | |
| /// - 2 (Reserved): Reserved for future expansion | |
| /// - 3 (Detailed): Additional detailed metrics - supplementary percentiles (p70, p90, p95, p99.9) | |
| /// - 4 (Reserved): Reserved for future expansion | |
| /// Verbosity levels: | |
| /// - 1 (Standard/Critical): Most important metrics - bandwidth, throughput, IOPS, key latency percentiles (p50, p99) | |
| /// - 2 (Intermediate): Workload-specific metrics that provide more detail than level 1 but are not as detailed as level 3 | |
| /// - 3 (Detailed): Additional detailed metrics - supplementary percentiles (p70, p90, p95, p99.9) | |
| /// - 4 (Intermediate/Extended): Workload-specific metrics between detailed (3) and verbose (5) |
| // Step 1: Handle verbosity filtering first | ||
| string verbosityFilter = filterTerms.FirstOrDefault(f => f.Contains("verbosity", StringComparison.OrdinalIgnoreCase)); | ||
| if (!string.IsNullOrEmpty(verbosityFilter)) | ||
| { | ||
| switch (verbosityFilter.ToLower()) | ||
| // Extract the verbosity level from the filter (e.g., "verbosity:3" -> 3) | ||
| string[] parts = verbosityFilter.Split(':'); | ||
| if (parts.Length == 2 && int.TryParse(parts[1].Trim(), out int maxVerbosity) && maxVerbosity >= 1 && maxVerbosity <= 5) | ||
| { | ||
| case "verbosity:0": | ||
| filteredMetrics = filteredMetrics.Where(m => ((int)m.Verbosity) <= 0); | ||
| break; | ||
| case "verbosity:1": | ||
| filteredMetrics = filteredMetrics.Where(m => ((int)m.Verbosity) <= 1); | ||
| break; | ||
| case "verbosity:2": | ||
| filteredMetrics = filteredMetrics.Where(m => ((int)m.Verbosity) <= 2); | ||
| break; | ||
|
|
||
| default: | ||
| filteredMetrics = Enumerable.Empty<Metric>(); | ||
| break; | ||
| // Filter metrics to include only those with verbosity <= maxVerbosity | ||
| filteredMetrics = filteredMetrics.Where(m => m.Verbosity <= maxVerbosity); | ||
| } | ||
| else | ||
| { | ||
| // Invalid verbosity format or out of range - return empty set | ||
| filteredMetrics = Enumerable.Empty<Metric>(); | ||
| } |
There was a problem hiding this comment.
The filtering logic rejects verbosity level 0 as invalid (line 73 requires maxVerbosity >= 1), but many existing workload parsers in the codebase still use verbosity level 0 for critical metrics (e.g., ThreeDMarkMetricsParser, BombardierMetricsParser, OpenSslMetricsParser, etc.). This creates a breaking change where users cannot filter for these metrics using verbosity:0. Consider either: (1) accepting verbosity:0 in the filter and treating it as equivalent to verbosity:1, or (2) updating all existing parsers to migrate from verbosity:0 to verbosity:1 in a separate PR.
| table.Columns.Add("Unit", typeof(string)); | ||
|
|
||
| IEnumerable<Metric> metricsToPrint = criticalOnly ? metrics.Where(m => m.Verbosity == 0).ToList() : metrics; | ||
| IEnumerable<Metric> metricsToPrint = criticalOnly ? metrics.Where(m => m.Verbosity == 1).ToList() : metrics; |
There was a problem hiding this comment.
The LogConsole method was updated to filter for verbosity == 1 instead of verbosity == 0 for critical metrics. However, many existing workload parsers still use verbosity: 0 for their critical metrics (e.g., ThreeDMarkMetricsParser, BombardierMetricsParser, OpenSslMetricsParser). This means those critical metrics will not be displayed when criticalOnly=true. This change should be coordinated with updating all existing parsers to use the new verbosity levels.
| IEnumerable<Metric> metricsToPrint = criticalOnly ? metrics.Where(m => m.Verbosity == 1).ToList() : metrics; | |
| IEnumerable<Metric> metricsToPrint = criticalOnly ? metrics.Where(m => m.Verbosity == 0 || m.Verbosity == 1).ToList() : metrics; |
| ``` | ||
|
|
||
| The example above will: | ||
| 1. Include only metrics with verbosity ? 3 |
There was a problem hiding this comment.
Character encoding issue: The "?" character should be replaced with "≤" (less than or equal to). The text should read "Include only metrics with verbosity ≤ 3".
| 1. Include only metrics with verbosity ? 3 | |
| 1. Include only metrics with verbosity ≤ 3 |
| /// Verbosity Levels (only 1, 3, and 5 are currently used): | ||
| /// - 1 (Standard/Critical): Most important metrics for decision making - bandwidth, throughput, IOPS, key latency percentiles (p50, p99) | ||
| /// - 2 (Reserved): Reserved for future expansion | ||
| /// - 3 (Detailed): Additional detailed metrics - supplementary percentiles (p70, p90, p95, p99.9) | ||
| /// - 4 (Reserved): Reserved for future expansion | ||
| /// - 5 (Verbose): All diagnostic/internal metrics - histogram buckets, standard deviations, byte counts, I/O counts | ||
| /// | ||
| /// Currently, only levels 1, 3, and 5 are actively used. Levels 2 and 4 are reserved for future use. |
There was a problem hiding this comment.
The documentation states "only 1, 3, and 5 are currently used" but this is inaccurate. Many other workload parsers in the codebase still use verbosity level 2 (e.g., BombardierMetricsParser, NTttcpMetricsParser, SockPerfMetricsParser, etc.). The statement should either be scoped to the specific workloads being modified in this PR (Fio, DiskSpd, Memtier) or removed entirely to avoid confusion.
| /// Verbosity Levels (only 1, 3, and 5 are currently used): | |
| /// - 1 (Standard/Critical): Most important metrics for decision making - bandwidth, throughput, IOPS, key latency percentiles (p50, p99) | |
| /// - 2 (Reserved): Reserved for future expansion | |
| /// - 3 (Detailed): Additional detailed metrics - supplementary percentiles (p70, p90, p95, p99.9) | |
| /// - 4 (Reserved): Reserved for future expansion | |
| /// - 5 (Verbose): All diagnostic/internal metrics - histogram buckets, standard deviations, byte counts, I/O counts | |
| /// | |
| /// Currently, only levels 1, 3, and 5 are actively used. Levels 2 and 4 are reserved for future use. | |
| /// Verbosity levels and recommended usage: | |
| /// - 1 (Standard/Critical): Most important metrics for decision making - bandwidth, throughput, IOPS, key latency percentiles (p50, p99) | |
| /// - 2 (Reserved): Reserved for future expansion | |
| /// - 3 (Detailed): Additional detailed metrics - supplementary percentiles (p70, p90, p95, p99.9) | |
| /// - 4 (Reserved): Reserved for future expansion | |
| /// - 5 (Verbose): All diagnostic/internal metrics - histogram buckets, standard deviations, byte counts, I/O counts | |
| /// | |
| /// These levels define a common convention across workloads; individual workloads may use any level as appropriate. |
1 participant
Made changes for Fio, DiskSpd and Memtier workloads.