Merged
Conversation
Signed-off-by: Harper, Jason M <jason.m.harper@intel.com>
Signed-off-by: Harper, Jason M <jason.m.harper@intel.com>
Contributor
There was a problem hiding this comment.
Pull request overview
This PR generalizes the telemetry command’s CPU idle residency reporting from a C6-only view to a dynamic “all C-states” view, by extracting C-state-related turbostat columns via regex and building telemetry fields at runtime. It also includes a sizable refactor of DIMM parsing logic and adds DIMM unit tests.
Changes:
- Add regex-driven turbostat platform-row extraction to support dynamic C-state field discovery.
- Replace C6 telemetry table/renderer/flag plumbing with generalized “C-States” equivalents and dynamically built table fields.
- Refactor DIMM format detection/extraction and add extensive DIMM unit test coverage.
Reviewed changes
Copilot reviewed 6 out of 6 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| internal/extract/turbostat.go | Adds TurbostatPlatformRowsByRegexMatch for regex-based field extraction from platform rows. |
| cmd/telemetry/telemetry_tables.go | Replaces C6 table values builder with dynamic C-state field discovery and table construction. |
| cmd/telemetry/telemetry_renderers.go | Renames and generalizes the C6 HTML renderer and updates axis labeling. |
| cmd/telemetry/telemetry.go | Renames the C6 telemetry category flag and updates table/renderer registration and summary output. |
| internal/extract/dimm.go | Refactors DIMM format handling into an ordered dimmFormats definition and simplifies parsing/extraction. |
| internal/extract/dimm_test.go | Adds comprehensive tests for DIMM parsing and socket/slot extraction behavior. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Signed-off-by: Harper, Jason M <jason.m.harper@intel.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
This pull request refactors the telemetry codebase to generalize and improve how CPU C-state residency metrics are handled. Instead of focusing solely on C6 residency, the code now supports collecting and displaying all relevant C-state metrics dynamically, making the telemetry tool more flexible and future-proof. The changes include renaming variables and functions, updating help texts, and introducing a more robust extraction mechanism for C-state fields.
The most important changes are:
Generalization of C-state telemetry:
Data extraction improvements:
TurbostatPlatformRowsByRegexMatchinextract/turbostat.goto extract all C-state-related fields from the turbostat output using regular expressions, enabling support for a variety of C-state metrics across different CPU platforms.Renderer and output updates: