From 1ac041655a74725fe8dfcb8a92fce9aa5a853243 Mon Sep 17 00:00:00 2001 From: Vihas Makwana <121151420+VihasMakwana@users.noreply.github.com> Date: Tue, 3 Dec 2024 21:43:48 +0530 Subject: [PATCH] [cpu] - Split the initialisation into multiple files (#194) This PR is a split from https://github.com/elastic/elastic-agent-system-metrics/pull/192. It aims to simplify the query building for performance counters. Using a global variable might create problems if multiple modules (`cpu` and `core`) are trying to initialise it at a same time. `sync.Once` adds unnecessary CPU overhead (makes it 4x slower). Instead, we can initialise the performance counters in `New(...)` and return error to the caller (i.e. metricbeat). --- go.mod | 2 +- go.sum | 4 +- metric/cpu/{metrics.go => cpu.go} | 23 ++++------ metric/cpu/cpu_other.go | 39 +++++++++++++++++ metric/cpu/cpu_windows.go | 72 +++++++++++++++++++++++++++++++ metric/cpu/metrics_test.go | 7 ++- 6 files changed, 128 insertions(+), 19 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/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/metrics.go b/metric/cpu/cpu.go similarity index 93% rename from metric/cpu/metrics.go rename to metric/cpu/cpu.go index c039d091a4..7e6a6e994c 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,21 +78,17 @@ 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 +type option struct { + usePerformanceCounter bool } -// 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} +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. diff --git a/metric/cpu/cpu_other.go b/metric/cpu/cpu_other.go new file mode 100644 index 0000000000..67586abeda --- /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, _ ...OptionFunc) (*Monitor, error) { + return &Monitor{Hostfs: hostfs}, nil +} diff --git a/metric/cpu/cpu_windows.go b/metric/cpu/cpu_windows.go new file mode 100644 index 0000000000..c1de79cad8 --- /dev/null +++ b/metric/cpu/cpu_windows.go @@ -0,0 +1,72 @@ +// 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 +} + +// New returns a new CPU metrics monitor +// Hostfs is only relevant on linux and freebsd. +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) { + var q pdh.Query + 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 +} 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)