Skip to content

Commit

Permalink
[fix]: update with review comments
Browse files Browse the repository at this point in the history
Signed-off-by: Sam Yuan <[email protected]>
  • Loading branch information
SamYuan1990 committed Nov 25, 2024
1 parent c7129c7 commit 6474232
Show file tree
Hide file tree
Showing 14 changed files with 29 additions and 23 deletions.
6 changes: 6 additions & 0 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -135,3 +135,9 @@ Doc: update Developer.md

We enabled [stale bot](https://github.com/probot/stale) for house keeping. An
Issue or Pull Request becomes stale if no any inactivity for 60 days.

## For Mac and Windows user

kepler currently focus on linux platform.
for other platforms, to make kepler is easy for anyone contributes from any platform, we are welcome any benefits(PRs) for kepler including parts as compilable on other platform.
before the specific platform is supported, we just running CI on linux as PR merge standard and official support.
2 changes: 1 addition & 1 deletion cmd/exporter/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ func main() {
platform.InitPowerImpl()
defer platform.StopPower()

if config.IsEnabledGPU() {
if config.IsGPUEnabled() {
r := accelerator.GetRegistry()
if a, err := accelerator.New(config.GPU, true); err == nil {
r.MustRegister(a) // Register the accelerator with the registry
Expand Down
4 changes: 2 additions & 2 deletions pkg/bpf/exporter.go
Original file line number Diff line number Diff line change
Expand Up @@ -114,7 +114,7 @@ func (e *exporter) attach() error {
return fmt.Errorf("error attaching sched_switch tracepoint: %v", err)
}

if config.IsExposeIRQCounterMetrics() {
if config.ExposeIRQCounterMetrics() {
e.irqLink, err = link.AttachTracing(link.TracingOptions{
Program: e.bpfObjects.KeplerIrqTrace,
AttachType: ebpf.AttachTraceRawTp,
Expand Down Expand Up @@ -145,7 +145,7 @@ func (e *exporter) attach() error {
}

// Return early if hardware counters are not enabled
if !config.IsExposeHardwareCounterMetrics() {
if !config.ExposeHardwareCounterMetrics() {
klog.Infof("Hardware counter metrics are disabled")
return nil
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/bpf/fake_mac.go
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ func (e *exporter) attach() error {
return fmt.Errorf("error attaching sched_switch tracepoint: %v", err)
}

if config.IsExposeIRQCounterMetrics() {
if config.ExposeIRQCounterMetrics() {
e.irqLink, err = link.AttachTracing(link.TracingOptions{
Program: e.bpfObjects.KeplerIrqTrace,
AttachType: ebpf.AttachTraceRawTp,
Expand Down Expand Up @@ -144,7 +144,7 @@ func (e *exporter) attach() error {
}

// Return early if hardware counters are not enabled
if !config.IsExposeHardwareCounterMetrics() {
if !config.ExposeHardwareCounterMetrics() {
klog.Infof("Hardware counter metrics are disabled")
return nil
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/collector/energy/node_energy_collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ func UpdateNodeComponentsEnergy(nodeStats *stats.NodeStats, wg *sync.WaitGroup)
// UpdateNodeGPUEnergy updates each GPU power consumption. Right now we don't support other types of accelerators
func UpdateNodeGPUEnergy(nodeStats *stats.NodeStats, wg *sync.WaitGroup) {
defer wg.Done()
if config.IsEnabledGPU() {
if config.IsGPUEnabled() {
if gpu := acc.GetActiveAcceleratorByType(config.GPU); gpu != nil {
gpuEnergy := gpu.Device().AbsEnergyFromDevice()
for gpu, energy := range gpuEnergy {
Expand Down
2 changes: 1 addition & 1 deletion pkg/collector/metric_collector.go
Original file line number Diff line number Diff line change
Expand Up @@ -158,7 +158,7 @@ func (c *Collector) updateProcessResourceUtilizationMetrics(wg *sync.WaitGroup)
// update process metrics regarding the resource utilization to be used to calculate the energy consumption
// we first updates the bpf which is responsible to include new processes in the ProcessStats collection
resourceBpf.UpdateProcessBPFMetrics(c.bpfExporter, c.ProcessStats)
if config.IsEnabledGPU() {
if config.IsGPUEnabled() {
if acc.GetActiveAcceleratorByType(config.GPU) != nil {
accelerator.UpdateProcessGPUUtilizationMetrics(c.ProcessStats)
}
Expand Down
2 changes: 1 addition & 1 deletion pkg/collector/stats/node_stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ func (ne *NodeStats) ResetDeltaValues() {

func (ne *NodeStats) UpdateIdleEnergyWithMinValue(isComponentsSystemCollectionSupported bool) {
// gpu metric
if config.IsEnabledGPU() {
if config.IsGPUEnabled() {
if acc.GetActiveAcceleratorByType(config.GPU) != nil {
ne.CalcIdleEnergy(config.AbsEnergyInGPU, config.IdleEnergyInGPU, config.GPUComputeUtilization)
}
Expand Down
4 changes: 2 additions & 2 deletions pkg/collector/stats/stats.go
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,7 @@ func NewStats() *Stats {
stats.ResourceUsage[metricName] = types.NewUInt64StatCollection()
}

if config.IsEnabledGPU() {
if config.IsGPUEnabled() {
if acc.GetActiveAcceleratorByType(config.GPU) != nil {
stats.ResourceUsage[config.GPUComputeUtilization] = types.NewUInt64StatCollection()
stats.ResourceUsage[config.GPUMemUtilization] = types.NewUInt64StatCollection()
Expand Down Expand Up @@ -140,7 +140,7 @@ func (s *Stats) UpdateDynEnergy() {
s.CalcDynEnergy(config.AbsEnergyInPlatform, config.IdleEnergyInPlatform, config.DynEnergyInPlatform, sensorID)
}
// GPU metric
if config.IsEnabledGPU() {
if config.IsGPUEnabled() {
if acc.GetActiveAcceleratorByType(config.GPU) != nil {
for gpuID := range s.EnergyUsage[config.AbsEnergyInGPU] {
s.CalcDynEnergy(config.AbsEnergyInGPU, config.IdleEnergyInGPU, config.DynEnergyInGPU, gpuID)
Expand Down
2 changes: 1 addition & 1 deletion pkg/collector/stats/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ func GetProcessFeatureNames() []string {
klog.V(3).Infof("Available ebpf counters: %v", metrics)

// gpu metric
if config.IsEnabledGPU() {
if config.IsGPUEnabled() {
if acc.GetActiveAcceleratorByType(config.GPU) != nil {
gpuMetrics := []string{config.GPUComputeUtilization, config.GPUMemUtilization}
metrics = append(metrics, gpuMetrics...)
Expand Down
6 changes: 3 additions & 3 deletions pkg/config/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -525,7 +525,7 @@ func GetLibvirtMetadataToken() string {
return instance.Libvirt.MetadataToken
}

func IsExposeIRQCounterMetrics() bool {
func ExposeIRQCounterMetrics() bool {
return instance.Kepler.ExposeIRQCounterMetrics
}

Expand Down Expand Up @@ -553,11 +553,11 @@ func GetMockACPIPowerPath() string {
return instance.Kepler.MockACPIPowerPath
}

func IsExposeHardwareCounterMetrics() bool {
func ExposeHardwareCounterMetrics() bool {
return instance.Kepler.ExposeHardwareCounterMetrics
}

func IsEnabledGPU() bool {
func IsGPUEnabled() bool {
return instance.Kepler.EnabledGPU
}

Expand Down
10 changes: 5 additions & 5 deletions pkg/config/config_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,7 +147,7 @@ var _ = Describe("Test Configuration", func() {
Expect(IsExposeVMStatsEnabled()).To(BeTrue())
Expect(IsExposeBPFMetricsEnabled()).To(BeTrue())
Expect(IsExposeComponentPowerEnabled()).To(BeTrue())
Expect(IsExposeIRQCounterMetrics()).To(BeTrue())
Expect(ExposeIRQCounterMetrics()).To(BeTrue())
Expect(GetBPFSampleRate()).To(Equal(0))

})
Expand All @@ -157,10 +157,10 @@ var _ = Describe("Test Configuration", func() {
// test set and is enable functions.
SetEnabledGPU(true)
Expect(Config.Kepler.EnabledGPU).To(BeTrue())
Expect(IsEnabledGPU()).To(BeTrue())
Expect(IsGPUEnabled()).To(BeTrue())
SetEnabledGPU(false)
Expect(Config.Kepler.EnabledGPU).To(BeFalse())
Expect(IsEnabledGPU()).To(BeFalse())
Expect(IsGPUEnabled()).To(BeFalse())

SetEnabledMSR(true)
Expect(Config.Kepler.EnabledMSR).To(BeTrue())
Expand Down Expand Up @@ -188,9 +188,9 @@ var _ = Describe("Test Configuration", func() {

SetEnabledHardwareCounterMetrics(true)
Expect(Config.Kepler.ExposeHardwareCounterMetrics).To(BeTrue())
Expect(IsExposeHardwareCounterMetrics()).To(BeTrue())
Expect(ExposeHardwareCounterMetrics()).To(BeTrue())
SetEnabledHardwareCounterMetrics(false)
Expect(Config.Kepler.ExposeHardwareCounterMetrics).To(BeFalse())
Expect(IsExposeHardwareCounterMetrics()).To(BeFalse())
Expect(ExposeHardwareCounterMetrics()).To(BeFalse())
})
})
2 changes: 1 addition & 1 deletion pkg/metrics/metricfactory/metric_factory.go
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,7 @@ func SCMetricsPromDesc(context string, bpfSupportedMetrics bpf.SupportedMetrics)

func GPUUsageMetricsPromDesc(context string) (descriptions map[string]*prometheus.Desc) {
descriptions = make(map[string]*prometheus.Desc)
if config.IsEnabledGPU() {
if config.IsGPUEnabled() {
if gpu := acc.GetActiveAcceleratorByType(config.GPU); gpu != nil {
for _, name := range consts.GPUMetricNames {
descriptions[name] = resMetricsPromDesc(context, name, gpu.Device().Name())
Expand Down
4 changes: 2 additions & 2 deletions pkg/metrics/utils/utils.go
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,7 @@ func CollectEnergyMetrics(ch chan<- prometheus.Metric, instance interface{}, col
if config.IsExposeComponentPowerEnabled() {
// collect the dynamic energy metrics
for i, collectorName := range consts.EnergyMetricNames {
if collectorName == config.GPU && !config.IsEnabledGPU() {
if collectorName == config.GPU && !config.IsGPUEnabled() {
continue
}
collectEnergy(ch, instance, consts.DynEnergyMetricNames[i], "dynamic", collectors[collectorName])
Expand All @@ -57,7 +57,7 @@ func CollectResUtilizationMetrics(ch chan<- prometheus.Metric, instance interfac
for collectorName := range bpfSupportedMetrics.HardwareCounters {
CollectResUtil(ch, instance, collectorName, collectors[collectorName])
}
if config.IsEnabledGPU() {
if config.IsGPUEnabled() {
if gpu := acc.GetActiveAcceleratorByType(config.GPU); gpu != nil {
for _, collectorName := range consts.GPUMetricNames {
CollectResUtil(ch, instance, collectorName, collectors[collectorName])
Expand Down
2 changes: 1 addition & 1 deletion pkg/model/process_energy.go
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ func addEstimatedEnergy(processIDList []uint64, processesMetrics map[uint64]*sta
klog.V(5).Infoln("Could not estimate the Process Components Power")
}
// estimate the associated power consumption of GPU for each process
if config.IsEnabledGPU() {
if config.IsGPUEnabled() {
if gpu := acc.GetActiveAcceleratorByType(config.GPU); gpu != nil {
processGPUPower, errGPU = processComponentPowerModel.GetGPUPower(isIdlePower)
if errGPU != nil {
Expand Down

0 comments on commit 6474232

Please sign in to comment.