promdb2pcp: new tool to import Prometheus node_exporter metrics - #2623
promdb2pcp: new tool to import Prometheus node_exporter metrics#2623Aniruddh9 wants to merge 7 commits into
Conversation
Add promdb2pcp, a Python import tool that converts Prometheus node_exporter metrics (stored as JSON from the query_range API) into PCP archives for analysis with pmrep, pcp2csv, pmchart, etc. Supported metric groups: CPU (per-cpu/per-mode), memory, disk I/O, network interfaces, load averages, scheduler counters, vmstat counters, and PSI pressure stall information. Input is a directory of JSON files produced by the Prometheus HTTP API /api/v1/query_range endpoint. An optional metadata.json file provides hostname and timezone for the archive header.
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited) Review profile: CHILL Plan: Pro Plus Run ID: ⛔ Files ignored due to path filters (1)
📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (2)
📝 WalkthroughSummary by CodeRabbitRelease Notes
WalkthroughA new Changespromdb2pcp tool
Sequence Diagram(s)sequenceDiagram
participant CLI
participant convert
participant JSONFiles as Prometheus JSON files
participant PCP as pmiLogImport
CLI->>convert: datadir and options
convert->>JSONFiles: load metric data and metadata
JSONFiles-->>convert: series, labels, timestamps
convert->>PCP: register metrics and instances
loop sorted timestamps
convert->>PCP: put values and write samples
end
PCP-->>CLI: PCP archive
Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 4✅ Passed checks (4 passed)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 4
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@src/promdb2pcp/promdb2pcp.py`:
- Around line 307-313: The code at line 312 silently selects the first matching
time series with `matching[0]` without validating that exactly one series
matches the metric name. This causes multiple time series with different labels
(such as different instances or jobs) to be silently discarded. Add validation
after the `if not matching: continue` check to ensure exactly one series
matches, or add explicit filtering by label before accessing the matching list.
If multiple matches are found, either raise an error to alert the user about the
ambiguity, skip the metric, or implement label-based filtering to select the
correct series.
- Around line 316-322: The bare except blocks that catch pmi.pmiErr exceptions
and use pass are suppressing critical PCP API failures without any logging or
tracking, allowing partial archives to be created while still exiting
successfully. At each location where pmi.pmiErr is caught (in the
pmiAddMetric/pmiPutText block around lines 316-322, the instance registration
block around lines 393-399, the value write block around lines 451-457, and the
text registration block around lines 560-561), replace the empty pass statement
with proper error handling that logs the failure using the existing log facility
and tracks that a failure occurred. After processing all metrics, check if any
failures were recorded and return a non-zero exit code to signal that the import
failed, ensuring bad imports are visible and actionable rather than silently
producing partial archives.
- Around line 490-499: The json.load(f) call at line 492 can raise a
JSONDecodeError if metadata.json is malformed, causing the entire conversion to
crash even though metadata is optional. Wrap the file open and json.load
operations in a try-except block that catches JSON decode errors, and when
parsing fails, skip the metadata configuration (hostname and timezone setup) and
allow the conversion to continue with default values instead of propagating the
exception.
- Around line 259-267: The `load_json` function lacks exception handling around
the `json.load()` call and file operations, which means malformed JSON or I/O
errors will cause the entire import to fail. Wrap the `with open(filepath)`
block and `json.load(f)` call in a try-except block to catch JSONDecodeError and
other I/O exceptions. When an exception occurs, return None (or log a warning if
desired) so that the function degrades gracefully and allows other valid metric
groups to continue being processed instead of aborting the entire import.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: f6119efb-4df7-436e-8d7a-6ee36bd37f10
📒 Files selected for processing (4)
src/GNUmakefilesrc/promdb2pcp/GNUmakefilesrc/promdb2pcp/promdb2pcp.1src/promdb2pcp/promdb2pcp.py
Harden promdb2pcp against malformed input files and registration failures: - load_json(): catch OSError and JSONDecodeError instead of crashing - register_simple_metrics(): warn on multiple matching series and report pmiErr details instead of silently swallowing them - convert(): handle metadata.json parse failures gracefully with a warning
| if verbose: | ||
| print('%s (%d samples)' % (pcp_name, len(values))) | ||
|
|
||
| cluster += 1 |
There was a problem hiding this comment.
In PCP, when a new cluster is created the item ID number should reset to 0. It looks like when you create a new cluster the item ID keeps incrementing without being reset.
| (prom_name, pcp_name, sem, units_args, | ||
| pcp_type, divisor, helptext) = entry | ||
|
|
||
| matching = [r for r in results |
There was a problem hiding this comment.
Can we move this line above the entries loop and create a hash map for the names?
Then inside the loop we can do a quick O(1) lookup instead of iterating through the results variable each time?
| item += 1 | ||
| values = {} | ||
| for ts, val in r.get('values', []): | ||
| fts = float(ts) |
There was a problem hiding this comment.
Add try/except clause to catch malformed data resulting in TypeError or ValueError and skip it smoothly
| for mode in CPU_MODES: | ||
| pcp_name = 'kernel.percpu.cpu.' + mode | ||
| units = pmapi.pmUnits(0, 1, 0, 0, PM_TIME_MSEC, 0) | ||
| indom = log.pmiInDom(DOMAIN, serial) |
There was a problem hiding this comment.
These metrics should share the same instance domain. So we should move the indom line outside of the loop and the serial should be constant for these metrics.
|
|
||
| for cpu_num in cpu_numbers: | ||
| try: | ||
| log.pmiAddInstance(indom, 'cpu' + cpu_num, int(cpu_num)) |
There was a problem hiding this comment.
on some systems the 'cpu' label could have a value of 'cpu-0' instead of just '0' in this case the int(cpu_num) would fail
|
@Aniruddh9 Hi Ani! Thank you so much for your contribution :) This looks like a great addition to PCP I left some comments in line on the files. Along with those we should have some QA to test the functionality of the tool added to our testsuite under pcp/qa. You can use the ./new script to generate a new test number along with a skeleton qa test script. Or you can search around and see if there is an existing QA test available where it would make sense to add to it. Let me know if you have any questions Lauren |
|
hi @lmchilton , Thank you for the reviews and I am glad that you feel this is a great addition to PCP. I have been busy lately and could not look at the testing part. I will do it this week and update the PR. |
- Reset item ID to 0 when starting a new cluster so each cluster's items are numbered from zero per PCP convention - Build a hash map of metric names before the entries loop for O(1) lookup instead of iterating through results for each metric - Wrap value parsing in try/except to skip malformed data (NaN, non-numeric timestamps) instead of crashing - Share a single instance domain across all per-CPU metrics instead of creating a new one per mode - Handle cpu labels like cpu-0 by extracting the trailing integer with a regex, so int() conversion no longer fails on non-numeric prefixes
Test 1687 verifies basic conversion - metric descriptors, PMID structure with item IDs resetting per cluster, shared CPU instance domain, correct values, and hostname from metadata. Test 1699 verifies edge cases - malformed data (NaN values, bad timestamps) skipped gracefully, cpu-N style labels parsed correctly, invalid metadata.json handled with warning, and proper exit status for missing or empty data directories.
There was a problem hiding this comment.
🧹 Nitpick comments (2)
qa/1687 (1)
111-112: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winAssert the metadata hostname value.
This only verifies that a host label exists, so it passes if
metadata.jsonis ignored. Matchtesthost.example.comexplicitly.Proposed fix
-pmdumplog -l $tmp 2>&1 | grep 'Performance metrics from host' +pmdumplog -l $tmp 2>&1 | grep 'Performance metrics from host testhost.example.com'🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@qa/1687` around lines 111 - 112, Update the hostname assertion in the metadata verification section to match the explicit value testhost.example.com, rather than only checking for the generic “Performance metrics from host” label. Preserve the existing pmdumplog invocation and ensure the test fails when metadata.json is ignored.qa/1699 (1)
70-75: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winExercise the malformed numeric-value path.
These lines only verify the bad timestamp. Add a
mem.freememcheck and use an unambiguously non-numeric fixture value so this test proves malformed values are skipped.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@qa/1699` around lines 70 - 75, Update the QA scenario around the mem.physmem check to use a fixture with an unmistakably non-numeric value, then add a mem.freemem query that exercises and verifies the malformed numeric-value path while retaining the existing malformed-timestamp coverage.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In `@qa/1687`:
- Around line 111-112: Update the hostname assertion in the metadata
verification section to match the explicit value testhost.example.com, rather
than only checking for the generic “Performance metrics from host” label.
Preserve the existing pmdumplog invocation and ensure the test fails when
metadata.json is ignored.
In `@qa/1699`:
- Around line 70-75: Update the QA scenario around the mem.physmem check to use
a fixture with an unmistakably non-numeric value, then add a mem.freemem query
that exercises and verifies the malformed numeric-value path while retaining the
existing malformed-timestamp coverage.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Repository YAML (base), Repository UI (inherited), Organization UI (inherited)
Review profile: CHILL
Plan: Pro Plus
Run ID: d02a2b5c-35b4-45ad-a540-b9f41fc8388e
⛔ Files ignored due to path filters (2)
qa/1687.outis excluded by!**/*.outqa/1699.outis excluded by!**/*.out
📒 Files selected for processing (13)
qa/1687qa/1699qa/groupqa/promdb2pcp/baddata/cpu.jsonqa/promdb2pcp/baddata/memory.jsonqa/promdb2pcp/baddata/metadata.jsonqa/promdb2pcp/cpu.jsonqa/promdb2pcp/disk.jsonqa/promdb2pcp/loadavg.jsonqa/promdb2pcp/memory.jsonqa/promdb2pcp/metadata.jsonqa/promdb2pcp/network.jsonsrc/promdb2pcp/promdb2pcp.py
🚧 Files skipped from review as they are similar to previous changes (1)
- src/promdb2pcp/promdb2pcp.py
|
hi @lmchilton , |
- Test 1687: grep for explicit hostname testhost.example.com so the test fails if metadata.json is ignored, not just on .out diff - Test 1699: use unmistakably non-numeric value in fixture instead of NaN to test the float() ValueError path directly, and add mem.freemem query to verify the malformed value was skipped
2 participants
Add promdb2pcp, a Python import tool that converts Prometheus node_exporter metrics (stored as JSON from the query_range API) into PCP archives for analysis with pmrep, pcp2csv, pmchart, etc.
Supported metric groups: CPU (per-cpu/per-mode), memory, disk I/O, network interfaces, load averages, scheduler counters, vmstat counters, and PSI pressure stall information.
Input is a directory of JSON files produced by the Prometheus HTTP API /api/v1/query_range endpoint. An optional metadata.json file provides hostname and timezone for the archive header.
Pull Request Description
Related Issues :
New feature — no existing issue.
Fixes #
Checklist
Description :
Adds promdb2pcp, a new Python import tool that converts Prometheus node_exporter metrics into PCP archives.
Background: When analyzing OpenShift/Kubernetes node performance, Prometheus node_exporter metrics are often the primary data source. Currently there is no way to import this data into PCP for analysis with tools like pmrep, pcp2csv, and pmchart. This tool bridges that gap.
What it does: Reads a directory of JSON files produced by the Prometheus HTTP API /api/v1/query_range endpoint and creates a PCP archive with proper metric metadata, instance domains, and help text.
Supported metric groups: CPU (per-cpu/per-mode), memory, disk I/O, network interfaces, load averages, scheduler counters, vmstat counters, and PSI pressure stall information.
Conventions: Follows the same patterns as guidellm2pcp and vllmbench2pcp — #!/usr/bin/pmpython shebang, GPL header, pmiID/pmiInDom for proper PMIDs, help text via pmiPutText, domain 510.
Commits :
Single commit: promdb2pcp: new tool to import Prometheus node_exporter metrics
Files added/modified:
src/promdb2pcp/promdb2pcp.py — the import tool
src/promdb2pcp/GNUmakefile — build integration
src/promdb2pcp/promdb2pcp.1 — man page
src/GNUmakefile — register promdb2pcp in the build
Documentation updated
Man page promdb2pcp.1 included with synopsis, description, options, examples, and SEE ALSO references.
Tests added/updated
Manually tested with real Prometheus data (80 timestamps, 51 metrics, 8 CPUs, 2 disk devices, 3 network interfaces).