Skip to content

Comments

feat(energy_zone): Added Energy Zone Collector#2027

Closed
vimalk78 wants to merge 1 commit intosustainable-computing-io:rebootfrom
vimalk78:energy-zone-metric
Closed

feat(energy_zone): Added Energy Zone Collector#2027
vimalk78 wants to merge 1 commit intosustainable-computing-io:rebootfrom
vimalk78:energy-zone-metric

Conversation

@vimalk78
Copy link
Collaborator

@vimalk78 vimalk78 commented Apr 23, 2025

  • Added Energy Zone Collector
  • Added Tests
❯ curl localhost:28282/metrics | grep _rapl_zone
# HELP kepler_node_rapl_zone Rapl Zones from sysfs
# TYPE kepler_node_rapl_zone gauge
kepler_node_rapl_zone{index="0",name="core",path="/sys/class/powercap/intel-rapl:0:0"} 1
kepler_node_rapl_zone{index="0",name="package",path="/sys/class/powercap/intel-rapl-mmio:0"} 1
kepler_node_rapl_zone{index="0",name="package",path="/sys/class/powercap/intel-rapl:0"} 1
kepler_node_rapl_zone{index="0",name="psys",path="/sys/class/powercap/intel-rapl:1"} 1
kepler_node_rapl_zone{index="0",name="uncore",path="/sys/class/powercap/intel-rapl:0:1"} 1

@codecov
Copy link

codecov bot commented Apr 23, 2025

Codecov Report

Attention: Patch coverage is 74.19355% with 16 lines in your changes missing coverage. Please review.

Project coverage is 89.26%. Comparing base (2ad09ba) to head (e52bd2d).

Files with missing lines Patch % Lines
internal/exporter/prometheus/collector/procfs.go 37.50% 4 Missing and 1 partial ⚠️
internal/exporter/prometheus/collector/sysfs.go 37.50% 4 Missing and 1 partial ⚠️
...ternal/exporter/prometheus/collector/energyzone.go 91.66% 2 Missing and 1 partial ⚠️
internal/exporter/prometheus/prometheus.go 70.00% 2 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##           reboot    #2027      +/-   ##
==========================================
- Coverage   89.78%   89.26%   -0.53%     
==========================================
  Files          17       20       +3     
  Lines         999     1053      +54     
==========================================
+ Hits          897      940      +43     
- Misses         81       89       +8     
- Partials       21       24       +3     

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

@vimalk78 vimalk78 force-pushed the energy-zone-metric branch 2 times, most recently from ba2103b to 7f15a32 Compare April 23, 2025 17:04
Signed-off-by: Vimal Kumar <vimal78@gmail.com>
@vimalk78 vimalk78 force-pushed the energy-zone-metric branch from 7f15a32 to e52bd2d Compare April 23, 2025 17:13
)

// procFS is an interface to prometheus/procfs
type procFS interface {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

How about cpuInfoProvider or something more specific to the cpuInfo?

sysfs sysfs.FS
}

func (s *realSysFS) Zones() ([]sysfs.RaplZone, error) {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we should reuse cpuPowerMeter here since the raplReader removes non-standard rapl zones ( a functionality that we want to preserve here as well). Also when we implement filtering of zones, we should only show the zones that are filtered.

We could instead have an interface that raplPowerMeter satisfies which returns only the EnergyZones() which provides all information.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

then we can have this metric as part of power collecor itself.

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Indeed, didn't think of that :)

"github.com/prometheus/procfs"
)

// procFS is an interface for CPUInfo.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can this refactor /reorg be a separate commit?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

perhaps if this refactor was done, comment would ask for a refactor

Copy link
Collaborator

@sthaha sthaha left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Requesting change to reuse existing power-meter that provides the zone information. We can add an Init which computes the zones information and cache it.

kepler_node_rapl_zone{index="0",name="package",path="/sys/class/powercap/intel-rapl-mmio:0"} 1

we shouldn't expose this.

@vimalk78 vimalk78 closed this Apr 24, 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.

2 participants