From f62dc9866295d08c57c026289d5e0eb67c1fb880 Mon Sep 17 00:00:00 2001 From: Vihas Makwana Date: Tue, 3 Dec 2024 18:58:01 +0530 Subject: [PATCH 1/3] chore: split cpu initialization as per OS --- metric/cpu/{metrics.go => cpu.go} | 18 ---------- metric/cpu/cpu_other.go | 39 +++++++++++++++++++++ metric/cpu/cpu_windows.go | 58 +++++++++++++++++++++++++++++++ 3 files changed, 97 insertions(+), 18 deletions(-) rename metric/cpu/{metrics.go => cpu.go} (93%) create mode 100644 metric/cpu/cpu_other.go create mode 100644 metric/cpu/cpu_windows.go diff --git a/metric/cpu/metrics.go b/metric/cpu/cpu.go similarity index 93% rename from metric/cpu/metrics.go rename to metric/cpu/cpu.go index c039d091a4..09f4fc54e0 100644 --- a/metric/cpu/metrics.go +++ b/metric/cpu/cpu.go @@ -24,7 +24,6 @@ import ( "github.com/elastic/elastic-agent-libs/mapstr" "github.com/elastic/elastic-agent-libs/opt" "github.com/elastic/elastic-agent-system-metrics/metric" - "github.com/elastic/elastic-agent-system-metrics/metric/system/resolve" ) // CPU manages the CPU metrics from /proc/stat @@ -79,23 +78,6 @@ func (cpu CPU) Total() uint64 { return opt.SumOptUint(cpu.User, cpu.Nice, cpu.Sys, cpu.Idle, cpu.Wait, cpu.Irq, cpu.SoftIrq, cpu.Stolen) } -/* -The below code implements a "metrics tracker" that gives us the ability to -calculate CPU percentages, as we average usage across a time period. -*/ - -// Monitor is used to monitor the overall CPU usage of the system over time. -type Monitor struct { - lastSample CPUMetrics - Hostfs resolve.Resolver -} - -// New returns a new CPU metrics monitor -// Hostfs is only relevant on linux and freebsd. -func New(hostfs resolve.Resolver) *Monitor { - return &Monitor{Hostfs: hostfs} -} - // Fetch collects a new sample of the CPU usage metrics. // This will overwrite the currently stored samples. func (m *Monitor) Fetch() (Metrics, error) { diff --git a/metric/cpu/cpu_other.go b/metric/cpu/cpu_other.go new file mode 100644 index 0000000000..43533a0a12 --- /dev/null +++ b/metric/cpu/cpu_other.go @@ -0,0 +1,39 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//go:build !windows + +package cpu + +import "github.com/elastic/elastic-agent-system-metrics/metric/system/resolve" + +/* +The below code implements a "metrics tracker" that gives us the ability to +calculate CPU percentages, as we average usage across a time period. +*/ + +// Monitor is used to monitor the overall CPU usage of the system over time. +type Monitor struct { + lastSample CPUMetrics + Hostfs resolve.Resolver +} + +// New returns a new CPU metrics monitor +// Hostfs is only relevant on linux and freebsd. +func New(hostfs resolve.Resolver) *Monitor { + return &Monitor{Hostfs: hostfs} +} diff --git a/metric/cpu/cpu_windows.go b/metric/cpu/cpu_windows.go new file mode 100644 index 0000000000..57120fbede --- /dev/null +++ b/metric/cpu/cpu_windows.go @@ -0,0 +1,58 @@ +// Licensed to Elasticsearch B.V. under one or more contributor +// license agreements. See the NOTICE file distributed with +// this work for additional information regarding copyright +// ownership. Elasticsearch B.V. licenses this file to you under +// the Apache License, Version 2.0 (the "License"); you may +// not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +//go:build windows + +package cpu + +import ( + "fmt" + + "github.com/elastic/elastic-agent-libs/helpers/windows/pdh" + "github.com/elastic/elastic-agent-system-metrics/metric/system/resolve" +) + +/* +The below code implements a "metrics tracker" that gives us the ability to +calculate CPU percentages, as we average usage across a time period. +*/ + +// Monitor is used to monitor the overall CPU usage of the system over time. +type Monitor struct { + lastSample CPUMetrics + Hostfs resolve.Resolver + + // windows specific fields + query *pdh.Query + qError error +} + +// New returns a new CPU metrics monitor +// Hostfs is only relevant on linux and freebsd. +func New(hostfs resolve.Resolver) *Monitor { + m := &Monitor{Hostfs: hostfs} + m.query, m.qError = buildQuery() + return m +} + +func buildQuery() (*pdh.Query, error) { + var q pdh.Query + if err := q.Open(); err != nil { + return nil, fmt.Errorf("failed to open query: %w", err) + } + return &q, nil +} From 080dcdb22cee1702e7dd41890a97b2e6cb284a03 Mon Sep 17 00:00:00 2001 From: Vihas Makwana Date: Tue, 3 Dec 2024 19:01:03 +0530 Subject: [PATCH 2/3] comments --- metric/cpu/cpu_windows.go | 1 + 1 file changed, 1 insertion(+) diff --git a/metric/cpu/cpu_windows.go b/metric/cpu/cpu_windows.go index 57120fbede..026ced2bb3 100644 --- a/metric/cpu/cpu_windows.go +++ b/metric/cpu/cpu_windows.go @@ -54,5 +54,6 @@ func buildQuery() (*pdh.Query, error) { if err := q.Open(); err != nil { return nil, fmt.Errorf("failed to open query: %w", err) } + // TODO: implement performance counters as a follow up return &q, nil } From d6a175d7ff3c40c6933308762565391ab149b48a Mon Sep 17 00:00:00 2001 From: Vihas Makwana Date: Tue, 3 Dec 2024 19:19:31 +0530 Subject: [PATCH 3/3] update tests --- go.mod | 2 +- go.sum | 4 ++-- metric/cpu/cpu.go | 13 +++++++++++++ metric/cpu/cpu_other.go | 4 ++-- metric/cpu/cpu_windows.go | 25 +++++++++++++++++++------ metric/cpu/metrics_test.go | 7 +++++-- 6 files changed, 42 insertions(+), 13 deletions(-) diff --git a/go.mod b/go.mod index 61757202ff..264dc2d9f1 100644 --- a/go.mod +++ b/go.mod @@ -4,7 +4,7 @@ go 1.22.8 require ( github.com/docker/docker v26.1.5+incompatible - github.com/elastic/elastic-agent-libs v0.9.13 + github.com/elastic/elastic-agent-libs v0.17.4-0.20241126154321-6ed75416832d github.com/elastic/go-licenser v0.4.2 github.com/elastic/go-structform v0.0.9 github.com/elastic/go-sysinfo v1.14.1 diff --git a/go.sum b/go.sum index c8d6a1819e..26097d7a5d 100644 --- a/go.sum +++ b/go.sum @@ -21,8 +21,8 @@ github.com/docker/go-connections v0.4.0 h1:El9xVISelRB7BuFusrZozjnkIM5YnzCViNKoh github.com/docker/go-connections v0.4.0/go.mod h1:Gbd7IOopHjR8Iph03tsViu4nIes5XhDvyHbTtUxmeec= github.com/docker/go-units v0.5.0 h1:69rxXcBk27SvSaaxTtLh/8llcHD8vYHT7WSdRZ/jvr4= github.com/docker/go-units v0.5.0/go.mod h1:fgPhTUdO+D/Jk86RDLlptpiXQzgHJF7gydDDbaIK4Dk= -github.com/elastic/elastic-agent-libs v0.9.13 h1:D1rh1s67zlkDWmixWQaNWzn+qy6DafIDPTQnLpBNBUA= -github.com/elastic/elastic-agent-libs v0.9.13/go.mod h1:G9ljFvDE+muOOOQBf2eRituF0fE4suGkv25rfjTwY+c= +github.com/elastic/elastic-agent-libs v0.17.4-0.20241126154321-6ed75416832d h1:nY8LSeTYU1uSDAAg7WwGH/cALgdovAXLdIzV25Ky0Bo= +github.com/elastic/elastic-agent-libs v0.17.4-0.20241126154321-6ed75416832d/go.mod h1:5CR02awPrBr+tfmjBBK+JI+dMmHNQjpVY24J0wjbC7M= github.com/elastic/go-licenser v0.4.2 h1:bPbGm8bUd8rxzSswFOqvQh1dAkKGkgAmrPxbUi+Y9+A= github.com/elastic/go-licenser v0.4.2/go.mod h1:W8eH6FaZDR8fQGm+7FnVa7MxI1b/6dAqxz+zPB8nm5c= github.com/elastic/go-structform v0.0.9 h1:HpcS7xljL4kSyUfDJ8cXTJC6rU5ChL1wYb6cx3HLD+o= diff --git a/metric/cpu/cpu.go b/metric/cpu/cpu.go index 09f4fc54e0..7e6a6e994c 100644 --- a/metric/cpu/cpu.go +++ b/metric/cpu/cpu.go @@ -78,6 +78,19 @@ func (cpu CPU) Total() uint64 { return opt.SumOptUint(cpu.User, cpu.Nice, cpu.Sys, cpu.Idle, cpu.Wait, cpu.Irq, cpu.SoftIrq, cpu.Stolen) } +type option struct { + usePerformanceCounter bool +} + +type OptionFunc func(*option) + +// Note: WithPerformanceCounter option is only effective for windows and is ineffective if used by other OS'. +func WithPerformanceCounter() OptionFunc { + return func(o *option) { + o.usePerformanceCounter = true + } +} + // Fetch collects a new sample of the CPU usage metrics. // This will overwrite the currently stored samples. func (m *Monitor) Fetch() (Metrics, error) { diff --git a/metric/cpu/cpu_other.go b/metric/cpu/cpu_other.go index 43533a0a12..67586abeda 100644 --- a/metric/cpu/cpu_other.go +++ b/metric/cpu/cpu_other.go @@ -34,6 +34,6 @@ type Monitor struct { // New returns a new CPU metrics monitor // Hostfs is only relevant on linux and freebsd. -func New(hostfs resolve.Resolver) *Monitor { - return &Monitor{Hostfs: hostfs} +func New(hostfs resolve.Resolver, _ ...OptionFunc) (*Monitor, error) { + return &Monitor{Hostfs: hostfs}, nil } diff --git a/metric/cpu/cpu_windows.go b/metric/cpu/cpu_windows.go index 026ced2bb3..c1de79cad8 100644 --- a/metric/cpu/cpu_windows.go +++ b/metric/cpu/cpu_windows.go @@ -37,16 +37,29 @@ type Monitor struct { Hostfs resolve.Resolver // windows specific fields - query *pdh.Query - qError error + query *pdh.Query } // New returns a new CPU metrics monitor // Hostfs is only relevant on linux and freebsd. -func New(hostfs resolve.Resolver) *Monitor { - m := &Monitor{Hostfs: hostfs} - m.query, m.qError = buildQuery() - return m +func New(hostfs resolve.Resolver, opts ...OptionFunc) (*Monitor, error) { + var query *pdh.Query + var err error + + op := option{} + for _, o := range opts { + o(&op) + } + if !op.usePerformanceCounter { + if query, err = buildQuery(); err != nil { + return nil, err + } + } + + return &Monitor{ + Hostfs: hostfs, + query: query, + }, nil } func buildQuery() (*pdh.Query, error) { diff --git a/metric/cpu/metrics_test.go b/metric/cpu/metrics_test.go index 28b20540d7..43bc0c510e 100644 --- a/metric/cpu/metrics_test.go +++ b/metric/cpu/metrics_test.go @@ -31,7 +31,8 @@ import ( func TestMonitorSample(t *testing.T) { _ = logp.DevelopmentSetup() - cpu := &Monitor{lastSample: CPUMetrics{}, Hostfs: systemtests.DockerTestResolver()} + cpu, err := New(systemtests.DockerTestResolver()) + require.NoError(t, err) s, err := cpu.Fetch() require.NoError(t, err) @@ -46,7 +47,9 @@ func TestCoresMonitorSample(t *testing.T) { cpuMetrics, err := Get(systemtests.DockerTestResolver()) assert.NoError(t, err, "error in Get()") - cores := &Monitor{lastSample: CPUMetrics{list: make([]CPU, len(cpuMetrics.list))}, Hostfs: systemtests.DockerTestResolver()} + cores, err := New(systemtests.DockerTestResolver()) + require.NoError(t, err) + cores.lastSample = CPUMetrics{list: make([]CPU, len(cpuMetrics.list))} sample, err := cores.FetchCores() require.NoError(t, err)