From 9015955cfaa6e03fba4ccd39711b147a9af12847 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan-Otto=20Kr=C3=B6pke?= Date: Thu, 15 Oct 2026 15:59:42 +0200 Subject: [PATCH] smb: fix metric types and names, extend 32-bit counters in smb and smbclient (#2710) Signed-off-by: Claude Co-authored-by: Claude --- UPGRADE.md | 11 +- contrib/mixin/dashboards/smb.libsonnet | 8 +- dashboard/windows-exporter-dashboard.json | 8 +- dashboard/windows-exporter-smb.json | 8 +- docs/collector.smb.md | 14 +- docs/collector.smbclient.md | 5 + internal/collector/smb/smb.go | 78 +++++++++- internal/collector/smb/smb_internal_test.go | 71 +++++++++ internal/collector/smb/smb_test.go | 14 +- internal/collector/smbclient/smbclient.go | 147 +++++++++++++++++- .../smbclient/smbclient_internal_test.go | 88 +++++++++++ internal/utils/counter.go | 49 ++++++ internal/utils/counter_test.go | 25 +++ 13 files changed, 492 insertions(+), 34 deletions(-) create mode 100644 internal/collector/smb/smb_internal_test.go create mode 100644 internal/collector/smbclient/smbclient_internal_test.go diff --git a/UPGRADE.md b/UPGRADE.md index f8f0387a..cbf739e1 100644 --- a/UPGRADE.md +++ b/UPGRADE.md @@ -17,7 +17,7 @@ Most installations need no changes. Go through the checklist below and read the | You use the `container` collector | Replace `containerd-state-dir` with `cri-endpoint` | [container collector](#container-collector) | | You query `windows_gpu_info` | Replace the `phys` label with `device_number` | [GPU collector](#gpu-collector) | | You alert on BitLocker status | Replace `status="disabled"` with `status="off"` | [BitLocker status](#bitlocker-status) | -| You query Hyper-V, logical disk queue, cache, IIS output cache, memory, scheduled task or terminal services metrics | Update queries | [Renamed, deprecated and retyped metrics](#renamed-deprecated-and-retyped-metrics) | +| You query Hyper-V, logical disk queue, cache, IIS output cache, memory, scheduled task, SMB server share or terminal services metrics | Update queries | [Renamed, deprecated and retyped metrics](#renamed-deprecated-and-retyped-metrics) | | You enable the `scheduled_task` collector and use tasks below `\Microsoft\` or missed runs | Set `--collector.scheduled_task.exclude` or `missed-runs` | [scheduled_task collector](#scheduled_task-collector) | | You chart SQL Server latencies, SQL Server lock counts or DHCP denials | Expect corrected values | [Corrected values](#corrected-values) | | You enable the `hyperv` or `mscluster` collector | Check the new default sub-collectors | [New default sub-collectors](#new-default-sub-collectors) | @@ -215,6 +215,10 @@ Renamed: | `windows_hyperv_virtual_storage_device_throughput` | `windows_hyperv_virtual_storage_device_throughput_total` | Now a counter, use `rate()` | | `windows_filetime_mtime_timestamp_seconds` | `windows_file_mtime_timestamp_seconds` | See [filetime](#filetime-collector-removed) | | `windows_memory_commit_limit` | `windows_memory_commit_limit_bytes` | Unit suffix added, same value | +| `windows_smb_server_shares_read_requests_count_total` | `windows_smb_server_shares_read_requests_total` | Same value | +| `windows_smb_server_shares_write_requests_count_total` | `windows_smb_server_shares_write_requests_total` | Same value | +| `windows_smb_server_shares_metadata_requests_count_total` | `windows_smb_server_shares_metadata_requests_total` | Same value | +| `windows_smb_server_shares_files_opened_count_total` | `windows_smb_server_shares_files_opened_total` | Same value | Removed: @@ -247,12 +251,13 @@ Changed type, same name: | `windows_iis_server_output_cache_items`, `windows_iis_worker_output_cache_items` | counter | gauge | | `windows_iis_server_output_cache_memory_bytes`, `windows_iis_worker_output_cache_memory_bytes` | counter | gauge | | `windows_iis_server_output_cache_active_flushed_items`, `windows_iis_worker_output_cache_active_flushed_items` | counter | gauge | +| `windows_smb_server_shares_current_open_file_count`, `windows_smb_server_shares_tree_connect_count` | counter | gauge | | `windows_cache_copy_read_hits_total` | gauge | counter | | `windows_udp_datagram_received_total` | gauge | counter | | `windows_hyperv_legacy_network_adapter_bytes_dropped_total` | gauge | counter | -The IIS values are current occupancy, so query them directly instead of with `rate()`. -If Prometheus scrapes with OpenMetrics, the IIS metrics were stored with a `_total` suffix, which counters get in that format. They now arrive without it. +The IIS and SMB values are current occupancy, so query them directly instead of with `rate()`. +If Prometheus scrapes with OpenMetrics, the IIS and SMB metrics were stored with a `_total` suffix, which counters get in that format. They now arrive without it. The cache, UDP and Hyper-V metrics already end in `_total`, so only their type metadata changes; `rate()` queries keep working. Label changes on existing metrics: diff --git a/contrib/mixin/dashboards/smb.libsonnet b/contrib/mixin/dashboards/smb.libsonnet index b9e9b7c2..c40a03e1 100644 --- a/contrib/mixin/dashboards/smb.libsonnet +++ b/contrib/mixin/dashboards/smb.libsonnet @@ -37,13 +37,13 @@ function(on) [serverRate('received_bytes_total'), share + ' received'], ], below='.* received$'), b.graph(227, 'Share requests', 'Read, write and metadata requests per second per share.' + serverRequirement, 'ops', [ - [serverRate('read_requests_count_total'), share + ' read'], - [serverRate('write_requests_count_total'), share + ' write'], - [serverRate('metadata_requests_count_total'), share + ' metadata'], + [serverRate('read_requests_total'), share + ' read'], + [serverRate('write_requests_total'), share + ' write'], + [serverRate('metadata_requests_total'), share + ' metadata'], ]), b.graph(228, 'Open files', 'Files open per share.' + serverRequirement, 'short', [[server('current_open_file_count'), share]]), b.graph(229, 'Connections', 'Client connections (tree connects) per share.' + serverRequirement, 'short', [[server('tree_connect_count'), share]]), - b.graph(230, 'Files opened', 'Files opened per second per share.' + serverRequirement, 'ops', [[serverRate('files_opened_count_total'), share]]), + b.graph(230, 'Files opened', 'Files opened per second per share.' + serverRequirement, 'ops', [[serverRate('files_opened_total'), share]]), b.graph(231, 'Client traffic', 'Bytes read and written per second per remote share. Reads are drawn below the axis.' + clientRequirement, 'Bps', [ [clientRate('write_bytes_total'), path + ' write'], diff --git a/dashboard/windows-exporter-dashboard.json b/dashboard/windows-exporter-dashboard.json index 40d8dfe2..d0f2c9c7 100644 --- a/dashboard/windows-exporter-dashboard.json +++ b/dashboard/windows-exporter-dashboard.json @@ -14494,7 +14494,7 @@ "kind": "DataQuery", "spec": { "editorMode": "code", - "expr": "rate(windows_smb_server_shares_read_requests_count_total{job=~\"$job\", instance=\"$instance\"}[$__rate_interval])", + "expr": "rate(windows_smb_server_shares_read_requests_total{job=~\"$job\", instance=\"$instance\"}[$__rate_interval])", "instant": false, "legendFormat": "{{share}} read", "range": true @@ -14516,7 +14516,7 @@ "kind": "DataQuery", "spec": { "editorMode": "code", - "expr": "rate(windows_smb_server_shares_write_requests_count_total{job=~\"$job\", instance=\"$instance\"}[$__rate_interval])", + "expr": "rate(windows_smb_server_shares_write_requests_total{job=~\"$job\", instance=\"$instance\"}[$__rate_interval])", "instant": false, "legendFormat": "{{share}} write", "range": true @@ -14538,7 +14538,7 @@ "kind": "DataQuery", "spec": { "editorMode": "code", - "expr": "rate(windows_smb_server_shares_metadata_requests_count_total{job=~\"$job\", instance=\"$instance\"}[$__rate_interval])", + "expr": "rate(windows_smb_server_shares_metadata_requests_total{job=~\"$job\", instance=\"$instance\"}[$__rate_interval])", "instant": false, "legendFormat": "{{share}} metadata", "range": true @@ -14883,7 +14883,7 @@ "kind": "DataQuery", "spec": { "editorMode": "code", - "expr": "rate(windows_smb_server_shares_files_opened_count_total{job=~\"$job\", instance=\"$instance\"}[$__rate_interval])", + "expr": "rate(windows_smb_server_shares_files_opened_total{job=~\"$job\", instance=\"$instance\"}[$__rate_interval])", "instant": false, "legendFormat": "{{share}}", "range": true diff --git a/dashboard/windows-exporter-smb.json b/dashboard/windows-exporter-smb.json index 0c029f57..d16d24d9 100644 --- a/dashboard/windows-exporter-smb.json +++ b/dashboard/windows-exporter-smb.json @@ -747,7 +747,7 @@ "kind": "DataQuery", "spec": { "editorMode": "code", - "expr": "rate(windows_smb_server_shares_read_requests_count_total{job=~\"$job\", instance=\"$instance\"}[$__rate_interval])", + "expr": "rate(windows_smb_server_shares_read_requests_total{job=~\"$job\", instance=\"$instance\"}[$__rate_interval])", "instant": false, "legendFormat": "{{share}} read", "range": true @@ -769,7 +769,7 @@ "kind": "DataQuery", "spec": { "editorMode": "code", - "expr": "rate(windows_smb_server_shares_write_requests_count_total{job=~\"$job\", instance=\"$instance\"}[$__rate_interval])", + "expr": "rate(windows_smb_server_shares_write_requests_total{job=~\"$job\", instance=\"$instance\"}[$__rate_interval])", "instant": false, "legendFormat": "{{share}} write", "range": true @@ -791,7 +791,7 @@ "kind": "DataQuery", "spec": { "editorMode": "code", - "expr": "rate(windows_smb_server_shares_metadata_requests_count_total{job=~\"$job\", instance=\"$instance\"}[$__rate_interval])", + "expr": "rate(windows_smb_server_shares_metadata_requests_total{job=~\"$job\", instance=\"$instance\"}[$__rate_interval])", "instant": false, "legendFormat": "{{share}} metadata", "range": true @@ -1136,7 +1136,7 @@ "kind": "DataQuery", "spec": { "editorMode": "code", - "expr": "rate(windows_smb_server_shares_files_opened_count_total{job=~\"$job\", instance=\"$instance\"}[$__rate_interval])", + "expr": "rate(windows_smb_server_shares_files_opened_total{job=~\"$job\", instance=\"$instance\"}[$__rate_interval])", "instant": false, "legendFormat": "{{share}}", "range": true diff --git a/docs/collector.smb.md b/docs/collector.smb.md index 2cfa90a0..d74f0319 100644 --- a/docs/collector.smb.md +++ b/docs/collector.smb.md @@ -17,14 +17,16 @@ None ## Metrics Name | Description | Type | Labels -----|-------------|------|------- -`windows_smb_server_shares_current_open_file_count` | Current total count open files on the SMB Server Share | counter | `share` -`windows_smb_server_shares_tree_connect_count` | Count of user connections to the SMB Server Share | counter | `share` +`windows_smb_server_shares_current_open_file_count` | Current total count open files on the SMB Server Share | gauge | `share` +`windows_smb_server_shares_tree_connect_count` | Count of user connections to the SMB Server Share | gauge | `share` `windows_smb_server_shares_received_bytes_total` | Received bytes on the SMB Server Share | counter | `share` -`windows_smb_server_shares_write_requests_count_total` | Writes requests on the SMB Server Share | counter | `share` -`windows_smb_server_shares_read_requests_count_total` | Read requests on the SMB Server Share | counter | `share` -`windows_smb_server_shares_metadata_requests_count_total` | Metadata requests on the SMB Server Share | counter | `share` +`windows_smb_server_shares_write_requests_total` | Writes requests on the SMB Server Share | counter | `share` +`windows_smb_server_shares_read_requests_total` | Read requests on the SMB Server Share | counter | `share` +`windows_smb_server_shares_metadata_requests_total` | Metadata requests on the SMB Server Share | counter | `share` `windows_smb_server_shares_sent_bytes_total` | Sent bytes on the SMB Server Share | counter | `share` -`windows_smb_server_shares_files_opened_count_total` | Files opened on the SMB Server Share | counter | `share` +`windows_smb_server_shares_files_opened_total` | Files opened on the SMB Server Share | counter | `share` + +Windows keeps `Read Requests/sec` and `Write Requests/sec` as 32-bit raw counters. The collector extends them past 2^32 per share, so `read_requests_total` and `write_requests_total` don't drop back to a low value when the raw counter wraps. The extended value starts at the raw value and follows the raw counter when the share restarts. ### Example metric _This collector does not yet have explained examples, we would appreciate your help adding them!_ diff --git a/docs/collector.smbclient.md b/docs/collector.smbclient.md index 432269ee..44b98679 100644 --- a/docs/collector.smbclient.md +++ b/docs/collector.smbclient.md @@ -35,6 +35,11 @@ Name | Description | Type | Labels `windows_smbclient_write_requests_total` | The write requests on this share | counter | `server`, `share`| `windows_smbclient_read_seconds_total` | Seconds waiting for read requests on this share | counter | `server`, `share`| `windows_smbclient_write_seconds_total` | Seconds waiting for write requests on this share | counter | `server`, `share`| + +Windows keeps several raw counters at 32 bits: the time sums behind `read_seconds_total`, `write_seconds_total` and `request_seconds_total`, and the counts behind `requests_total`, `read_requests_total`, `write_requests_total`, `metadata_requests_total`, `stalls_total`, `turbo_io_reads_total`, `turbo_io_writes_total`, `read_requests_via_smbdirect_total` and `write_requests_via_smbdirect_total`. The collector extends them past 2^32 per share, so they don't drop back to a low value when the raw counter wraps. The extended value starts at the raw value and follows the raw counter when the share reconnects. + +The time sums wrap after 2^32 ticks, about 430 seconds of summed request time, and that sum grows by one second per second for each outstanding request. The collector can only detect one wrap between two scrapes, so with about 7 or more requests outstanding on average over a 60 second scrape interval, the `*_seconds_total` values undercount. Scrape more often on busy shares. + ## Useful queries ``` # Average request queue length (includes read and write). diff --git a/internal/collector/smb/smb.go b/internal/collector/smb/smb.go index b4c28947..a6412dd3 100644 --- a/internal/collector/smb/smb.go +++ b/internal/collector/smb/smb.go @@ -26,6 +26,7 @@ import ( "github.com/prometheus-community/windows_exporter/internal/mi" "github.com/prometheus-community/windows_exporter/internal/pdh" "github.com/prometheus-community/windows_exporter/internal/types" + "github.com/prometheus-community/windows_exporter/internal/utils" "github.com/prometheus/client_golang/prometheus" ) @@ -42,6 +43,10 @@ type Collector struct { perfDataCollector *pdh.Collector[perfDataCounterValues] perfDataObject []perfDataCounterValues + // counters32 extends the 32-bit request counters of each share. + counters32 map[string]*shareCounters32 + generation uint64 + treeConnectCount *prometheus.Desc currentOpenFileCount *prometheus.Desc receivedBytes *prometheus.Desc @@ -52,6 +57,24 @@ type Collector struct { filesOpened *prometheus.Desc } +const ( + // restartBelow treats a drop of a request count from below 2^31 as a + // share restart: a wrap from there needs 2^31 requests between two scrapes. + restartBelow = 1 << 31 + + staleGenerations = 3 +) + +// shareCounters32 holds the 32-bit counters of one share, extended beyond +// 2^32, and the 64-bit byte counters that show a restart of the share. +type shareCounters32 struct { + generation uint64 + receivedBytes float64 + sentBytes float64 + readRequests utils.Counter32 + writeRequests utils.Counter32 +} + func New(config *Config) *Collector { if config == nil { config = &ConfigDefaults @@ -98,19 +121,19 @@ func (c *Collector) Build(logger *slog.Logger, _ *mi.Session) error { nil, ) c.writeRequests = prometheus.NewDesc( - prometheus.BuildFQName(types.Namespace, Name, "server_shares_write_requests_count_total"), + prometheus.BuildFQName(types.Namespace, Name, "server_shares_write_requests_total"), "Writes requests on the SMB Server Share", []string{"share"}, nil, ) c.readRequests = prometheus.NewDesc( - prometheus.BuildFQName(types.Namespace, Name, "server_shares_read_requests_count_total"), + prometheus.BuildFQName(types.Namespace, Name, "server_shares_read_requests_total"), "Read requests on the SMB Server Share", []string{"share"}, nil, ) c.metadataRequests = prometheus.NewDesc( - prometheus.BuildFQName(types.Namespace, Name, "server_shares_metadata_requests_count_total"), + prometheus.BuildFQName(types.Namespace, Name, "server_shares_metadata_requests_total"), "Metadata requests on the SMB Server Share", []string{"share"}, nil, @@ -122,12 +145,14 @@ func (c *Collector) Build(logger *slog.Logger, _ *mi.Session) error { nil, ) c.filesOpened = prometheus.NewDesc( - prometheus.BuildFQName(types.Namespace, Name, "server_shares_files_opened_count_total"), + prometheus.BuildFQName(types.Namespace, Name, "server_shares_files_opened_total"), "Files opened on the SMB Server Share", []string{"share"}, nil, ) + c.counters32 = make(map[string]*shareCounters32) + var err error c.perfDataCollector, err = pdh.NewCollector[perfDataCounterValues](logger.With(slog.String("collector", Name)), pdh.CounterTypeRaw, "SMB Server Shares", pdh.InstancesAll) @@ -145,17 +170,21 @@ func (c *Collector) Collect(ch chan<- prometheus.Metric, _ time.Duration) error return fmt.Errorf("failed to collect SMB Server Shares metrics: %w", err) } + c.generation++ + for _, data := range c.perfDataObject { + c.extendCounters32(&data) + ch <- prometheus.MustNewConstMetric( c.currentOpenFileCount, - prometheus.CounterValue, + prometheus.GaugeValue, data.CurrentOpenFileCount, data.Name, ) ch <- prometheus.MustNewConstMetric( c.treeConnectCount, - prometheus.CounterValue, + prometheus.GaugeValue, data.TreeConnectCount, data.Name, ) @@ -203,5 +232,42 @@ func (c *Collector) Collect(ch chan<- prometheus.Metric, _ time.Duration) error ) } + c.pruneCounters32() + return nil } + +// pruneCounters32 drops the state of shares missing from the last +// staleGenerations scrapes. A share missing from a single sample keeps its +// state, so its extended counters don't fall back to the raw value. +func (c *Collector) pruneCounters32() { + for name, counters := range c.counters32 { + if c.generation-counters.generation >= staleGenerations { + delete(c.counters32, name) + } + } +} + +// extendCounters32 replaces the raw 32-bit request counters with values that +// keep counting past 2^32. Read Requests/sec and Write Requests/sec are +// PERF_COUNTER_COUNTER (32-bit), the byte counters are 64-bit. A drop of a +// byte counter shows that the share restarted. +func (c *Collector) extendCounters32(data *perfDataCounterValues) { + counters, ok := c.counters32[data.Name] + if !ok { + counters = &shareCounters32{ + readRequests: utils.NewCounter32(data.ReadRequests, restartBelow), + writeRequests: utils.NewCounter32(data.WriteRequests, restartBelow), + } + c.counters32[data.Name] = counters + } + + reset := data.ReceivedBytes < counters.receivedBytes || data.SentBytes < counters.sentBytes + + counters.generation = c.generation + counters.receivedBytes = data.ReceivedBytes + counters.sentBytes = data.SentBytes + + data.ReadRequests = counters.readRequests.Update(data.ReadRequests, reset) + data.WriteRequests = counters.writeRequests.Update(data.WriteRequests, reset) +} diff --git a/internal/collector/smb/smb_internal_test.go b/internal/collector/smb/smb_internal_test.go new file mode 100644 index 00000000..98ba66d3 --- /dev/null +++ b/internal/collector/smb/smb_internal_test.go @@ -0,0 +1,71 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// Copyright The Prometheus Authors +// Licensed 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 smb + +import ( + "math" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestExtendCounters32(t *testing.T) { + t.Parallel() + + c := &Collector{counters32: make(map[string]*shareCounters32)} + + sample := func(readRequests, receivedBytes float64) float64 { + c.generation++ + + data := perfDataCounterValues{Name: `\\*\share`, ReadRequests: readRequests, ReceivedBytes: receivedBytes} + c.extendCounters32(&data) + c.pruneCounters32() + + return data.ReadRequests + } + + require.Equal(t, 100.0, sample(100, 1000)) //nolint:testifylint + require.Equal(t, float64(math.MaxUint32)-5, sample(math.MaxUint32-5, 2000)) //nolint:testifylint + require.Equal(t, float64(math.MaxUint32)+11, sample(10, 3000), "wrap") //nolint:testifylint + require.Equal(t, 4.0, sample(4, 10), "share restart shown by the byte counter") //nolint:testifylint + require.Equal(t, 2.0, sample(2, 20), "drop from below 2^31 is a restart") //nolint:testifylint +} + +func TestPruneCounters32(t *testing.T) { + t.Parallel() + + c := &Collector{counters32: make(map[string]*shareCounters32)} + + c.generation++ + c.extendCounters32(&perfDataCounterValues{Name: "a"}) + c.extendCounters32(&perfDataCounterValues{Name: "b"}) + c.pruneCounters32() + + for range staleGenerations - 1 { + c.generation++ + c.extendCounters32(&perfDataCounterValues{Name: "a"}) + c.pruneCounters32() + require.Contains(t, c.counters32, "b", "a share missing from a few samples keeps its state") + } + + c.generation++ + c.extendCounters32(&perfDataCounterValues{Name: "a"}) + c.pruneCounters32() + require.Contains(t, c.counters32, "a") + require.NotContains(t, c.counters32, "b") +} diff --git a/internal/collector/smb/smb_test.go b/internal/collector/smb/smb_test.go index 66858884..df961742 100644 --- a/internal/collector/smb/smb_test.go +++ b/internal/collector/smb/smb_test.go @@ -23,6 +23,8 @@ import ( "github.com/prometheus-community/windows_exporter/internal/collector/smb" "github.com/prometheus-community/windows_exporter/internal/utils/testutils" "github.com/prometheus/client_golang/prometheus" + dto "github.com/prometheus/client_model/go" + "github.com/stretchr/testify/require" ) func BenchmarkCollector(b *testing.B) { @@ -31,5 +33,15 @@ func BenchmarkCollector(b *testing.B) { func TestCollector(t *testing.T) { metrics := testutils.TestCollector(t, smb.New, nil) - testutils.RequireFixtureMetric(t, metrics, smb.Name, "windows_smb_server_shares_received_bytes_total", prometheus.Labels{"share": `\\*\CIShare`}) + if testutils.RequireFixtureMetric(t, metrics, smb.Name, "windows_smb_server_shares_received_bytes_total", prometheus.Labels{"share": `\\*\CIShare`}) == nil { + return + } + + for _, name := range []string{ + "windows_smb_server_shares_current_open_file_count", + "windows_smb_server_shares_tree_connect_count", + } { + require.Contains(t, metrics, name) + require.Equal(t, dto.MetricType_GAUGE, metrics[name].GetType(), name) + } } diff --git a/internal/collector/smbclient/smbclient.go b/internal/collector/smbclient/smbclient.go index b9e7e682..46a90214 100644 --- a/internal/collector/smbclient/smbclient.go +++ b/internal/collector/smbclient/smbclient.go @@ -27,6 +27,7 @@ import ( "github.com/prometheus-community/windows_exporter/internal/mi" "github.com/prometheus-community/windows_exporter/internal/pdh" "github.com/prometheus-community/windows_exporter/internal/types" + "github.com/prometheus-community/windows_exporter/internal/utils" "github.com/prometheus/client_golang/prometheus" ) @@ -41,10 +42,18 @@ var ConfigDefaults = Config{} type Collector struct { config Config + logger *slog.Logger perfDataCollector *pdh.Collector[perfDataCounterValues] perfDataObject []perfDataCounterValues + // unexpectedInstances holds instance names already logged as unparsable. + unexpectedInstances map[string]struct{} + + // counters32 extends the 32-bit counters of each share. + counters32 map[string]*shareCounters32 + generation uint64 + readBytesTotal *prometheus.Desc readBytesTransmittedViaSMBDirectTotal *prometheus.Desc readRequestQueueSecsTotal *prometheus.Desc @@ -69,6 +78,58 @@ type Collector struct { requestSecs *prometheus.Desc } +// shareCounters32 holds the 32-bit counters of one share, extended beyond +// 2^32, and the 64-bit byte counter that shows a reconnect of the share. +type shareCounters32 struct { + generation uint64 + restart [numRestartSignals]float64 + counters [numTimers32 + numCounts32]utils.Counter32 +} + +const ( + numTimers32 = 3 + numCounts32 = 9 + numRestartSignals = 4 + + // restartBelow treats a drop of a request count from below 2^31 as a + // share restart: a wrap from there needs 2^31 requests between two + // scrapes. Timer sums grow by 10^7 per second for each outstanding + // request, so they can wrap from anywhere and don't use it. + restartBelow = 1 << 31 + + staleGenerations = 3 +) + +// restartSignals returns 64-bit counters that only drop when the share +// restarts, for example after an idle disconnect and a reconnect. +func restartSignals(data *perfDataCounterValues) [numRestartSignals]float64 { + return [numRestartSignals]float64{ + data.DataBytesPerSec, + data.ReadBytesPerSec, + data.WriteBytesPerSec, + data.AvgDataQueueLength, + } +} + +// fields32 returns the 32-bit counters: first the PERF_AVERAGE_TIMER +// numerators, then the request counters (PERF_COUNTER_COUNTER). +func fields32(data *perfDataCounterValues) [numTimers32 + numCounts32]*float64 { + return [numTimers32 + numCounts32]*float64{ + &data.AvgSecPerRead, + &data.AvgSecPerWrite, + &data.AvgSecPerDataRequest, + &data.CreditStallsPerSec, + &data.DataRequestsPerSec, + &data.MetadataRequestsPerSec, + &data.ReadRequestsTransmittedViaSMBDirectPerSec, + &data.ReadRequestsPerSec, + &data.TurboIOReadsPerSec, + &data.TurboIOWritesPerSec, + &data.WriteRequestsTransmittedViaSMBDirectPerSec, + &data.WriteRequestsPerSec, + } +} + func New(config *Config) *Collector { if config == nil { config = &ConfigDefaults @@ -96,6 +157,10 @@ func (c *Collector) Close() error { } func (c *Collector) Build(logger *slog.Logger, _ *mi.Session) error { + c.logger = logger.With(slog.String("collector", Name)) + c.unexpectedInstances = make(map[string]struct{}) + c.counters32 = make(map[string]*shareCounters32) + // desc creates a new prometheus description desc := func(metricName string, description string, labels []string) *prometheus.Desc { return prometheus.NewDesc( @@ -193,7 +258,7 @@ func (c *Collector) Build(logger *slog.Logger, _ *mi.Session) error { var err error - c.perfDataCollector, err = pdh.NewCollector[perfDataCounterValues](logger.With(slog.String("collector", Name)), pdh.CounterTypeRaw, "SMB Client Shares", pdh.InstancesAll) + c.perfDataCollector, err = pdh.NewCollector[perfDataCounterValues](c.logger, pdh.CounterTypeRaw, "SMB Client Shares", pdh.InstancesAll) if err != nil { return fmt.Errorf("failed to create SMB Client Shares collector: %w", err) } @@ -208,14 +273,23 @@ func (c *Collector) Collect(ch chan<- prometheus.Metric, _ time.Duration) error return fmt.Errorf("failed to collect SMB Client Shares metrics: %w", err) } + c.generation++ + for _, data := range c.perfDataObject { - parsed := strings.FieldsFunc(data.Name, func(r rune) bool { return r == '\\' }) - if len(parsed) != 2 { - return fmt.Errorf("unexpected number of fields in SMB Client Shares instance name: %q", data.Name) + serverValue, shareValue, ok := parseInstanceName(data.Name) + if !ok { + if _, logged := c.unexpectedInstances[data.Name]; !logged { + c.unexpectedInstances[data.Name] = struct{}{} + + c.logger.Warn("skipping SMB Client Shares instance with unexpected name", + slog.String("instance", data.Name), + ) + } + + continue } - serverValue := parsed[0] - shareValue := parsed[1] + c.extendCounters32(&data) // Request time spent on queue. Convert from ticks to seconds. ch <- prometheus.MustNewConstMetric( @@ -368,5 +442,66 @@ func (c *Collector) Collect(ch chan<- prometheus.Metric, _ time.Duration) error ) } + c.pruneCounters32() + return nil } + +// pruneCounters32 drops the state of shares missing from the last +// staleGenerations scrapes. A share missing from a single sample keeps its +// state, so its extended counters don't fall back to the raw value. +func (c *Collector) pruneCounters32() { + for name, counters := range c.counters32 { + if c.generation-counters.generation >= staleGenerations { + delete(c.counters32, name) + } + } +} + +// extendCounters32 replaces the raw 32-bit counters with values that keep +// counting past 2^32. +func (c *Collector) extendCounters32(data *perfDataCounterValues) { + fields := fields32(data) + + signals := restartSignals(data) + + counters, ok := c.counters32[data.Name] + if !ok { + counters = &shareCounters32{restart: signals} + + for i, field := range fields { + var below uint32 + if i >= numTimers32 { + below = restartBelow + } + + counters.counters[i] = utils.NewCounter32(*field, below) + } + + c.counters32[data.Name] = counters + } + + var reset bool + + for i, signal := range signals { + reset = reset || signal < counters.restart[i] + } + + counters.generation = c.generation + counters.restart = signals + + for i, field := range fields { + *field = counters.counters[i].Update(*field, reset) + } +} + +// parseInstanceName splits an SMB Client Shares instance name such as +// \\server\share into its server and share parts. +func parseInstanceName(name string) (string, string, bool) { + parsed := strings.FieldsFunc(name, func(r rune) bool { return r == '\\' }) + if len(parsed) != 2 { + return "", "", false + } + + return parsed[0], parsed[1], true +} diff --git a/internal/collector/smbclient/smbclient_internal_test.go b/internal/collector/smbclient/smbclient_internal_test.go new file mode 100644 index 00000000..1c1dc076 --- /dev/null +++ b/internal/collector/smbclient/smbclient_internal_test.go @@ -0,0 +1,88 @@ +// SPDX-License-Identifier: Apache-2.0 +// +// Copyright The Prometheus Authors +// Licensed 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 smbclient + +import ( + "math" + "testing" + + "github.com/stretchr/testify/require" +) + +func TestParseInstanceName(t *testing.T) { + t.Parallel() + + for _, tc := range []struct { + name string + server string + share string + ok bool + }{ + {name: `\\fileserver\data`, server: "fileserver", share: "data", ok: true}, + // Windows 11 publishes the instances with a single leading backslash. + {name: `\localhost\C$`, server: "localhost", share: "C$", ok: true}, + {name: `\\domain\dfs\folder`}, + {name: `_Total`}, + {name: ``}, + } { + server, share, ok := parseInstanceName(tc.name) + + require.Equal(t, tc.ok, ok, tc.name) + require.Equal(t, tc.server, server, tc.name) + require.Equal(t, tc.share, share, tc.name) + } +} + +func TestExtendCounters32(t *testing.T) { + t.Parallel() + + c := &Collector{counters32: make(map[string]*shareCounters32)} + + sample := func(data perfDataCounterValues) perfDataCounterValues { + c.generation++ + + data.Name = `\\server\share` + c.extendCounters32(&data) + c.pruneCounters32() + + return data + } + + got := sample(perfDataCounterValues{AvgSecPerRead: 1 << 20, ReadRequestsPerSec: 7, ReadBytesPerSec: 100}) + require.Equal(t, float64(1<<20), got.AvgSecPerRead) //nolint:testifylint + require.Equal(t, 7.0, got.ReadRequestsPerSec) //nolint:testifylint + + // A timer sum can wrap from anywhere: 2^32 ticks are about 430 s of + // summed request time. + got = sample(perfDataCounterValues{AvgSecPerRead: 1 << 10, ReadRequestsPerSec: 9, ReadBytesPerSec: 200}) + require.Equal(t, float64(math.MaxUint32)+1+(1<<10), got.AvgSecPerRead) //nolint:testifylint + require.Equal(t, 9.0, got.ReadRequestsPerSec) //nolint:testifylint + + // A drop of a 64-bit byte counter shows a reconnect, even if DataBytes grew. + got = sample(perfDataCounterValues{AvgSecPerRead: 5, ReadRequestsPerSec: 1, ReadBytesPerSec: 50, DataBytesPerSec: 1000}) + require.Equal(t, 5.0, got.AvgSecPerRead) //nolint:testifylint + require.Equal(t, 1.0, got.ReadRequestsPerSec) //nolint:testifylint + + // A metadata-only share has no byte counter to show the reconnect; a + // request count dropping from below 2^31 is taken as one. + got = sample(perfDataCounterValues{MetadataRequestsPerSec: 50, ReadBytesPerSec: 50, DataBytesPerSec: 1000}) + require.Equal(t, 50.0, got.MetadataRequestsPerSec) //nolint:testifylint + + got = sample(perfDataCounterValues{MetadataRequestsPerSec: 3, ReadBytesPerSec: 50, DataBytesPerSec: 1000}) + require.Equal(t, 3.0, got.MetadataRequestsPerSec) //nolint:testifylint +} diff --git a/internal/utils/counter.go b/internal/utils/counter.go index 4c5e5b23..cd46de61 100644 --- a/internal/utils/counter.go +++ b/internal/utils/counter.go @@ -39,3 +39,52 @@ func (c *Counter) AddValue(value uint32) { func (c *Counter) Value() float64 { return c.totalValue } + +// Counter32 extends a raw 32-bit performance counter, such as a +// PERF_COUNTER_COUNTER or the numerator of a PERF_AVERAGE_TIMER, beyond 2^32. +// Unlike Counter, it starts at the raw value and follows a restart of the +// source, so its value equals the raw value until the first wrap. It can only +// detect one wrap between two samples. +type Counter32 struct { + lastValue uint32 + totalValue float64 + restartBelow uint32 +} + +// NewCounter32 creates a Counter32 from the first raw value. If restartBelow is +// not zero, a decrease from a raw value below restartBelow is taken as a +// restart of the source instead of a wrap. Use it for counters that cannot +// grow by 2^32-restartBelow between two samples, such as request counts. +func NewCounter32(value float64, restartBelow uint32) Counter32 { + raw := toUint32(value) + + return Counter32{ + lastValue: raw, + totalValue: float64(raw), + restartBelow: restartBelow, + } +} + +// Update adds a raw value and returns the extended value. reset reports that +// the source restarted, for example because a share was reconnected; the raw +// value is then taken as is. Any other decrease is a wrap, unless it starts +// below restartBelow. +func (c *Counter32) Update(value float64, reset bool) float64 { + raw := toUint32(value) + + if reset || (raw < c.lastValue && c.lastValue < c.restartBelow) { + c.totalValue = float64(raw) + } else { + c.totalValue += float64(raw - c.lastValue) + } + + c.lastValue = raw + + return c.totalValue +} + +// toUint32 converts through int64, so a sign-extended value keeps its low +// 32 bits on every architecture. +func toUint32(value float64) uint32 { + return uint32(int64(value)) +} diff --git a/internal/utils/counter_test.go b/internal/utils/counter_test.go index 911e1b36..7d2cddb8 100644 --- a/internal/utils/counter_test.go +++ b/internal/utils/counter_test.go @@ -55,3 +55,28 @@ func TestCounter(t *testing.T) { require.Equal(t, float64(math.MaxUint32)*2-9, c.Value()) //nolint:testifylint } + +func TestCounter32(t *testing.T) { + t.Parallel() + + c := utils.NewCounter32(100, 0) + + require.Equal(t, 100.0, c.Update(100, false)) //nolint:testifylint + require.Equal(t, 150.0, c.Update(150, false)) //nolint:testifylint + require.Equal(t, float64(math.MaxUint32)-10, c.Update(math.MaxUint32-10, false)) //nolint:testifylint + require.Equal(t, float64(math.MaxUint32)+21, c.Update(20, false)) //nolint:testifylint + require.Equal(t, 5.0, c.Update(5, true)) //nolint:testifylint + require.Equal(t, 15.0, c.Update(15, false)) //nolint:testifylint + // Without restartBelow, any decrease is a wrap. + require.Equal(t, float64(math.MaxUint32)+4, c.Update(3, false)) //nolint:testifylint +} + +func TestCounter32RestartBelow(t *testing.T) { + t.Parallel() + + c := utils.NewCounter32(50, 1<<31) + + require.Equal(t, 3.0, c.Update(3, false)) //nolint:testifylint + require.Equal(t, float64(math.MaxUint32)-10, c.Update(math.MaxUint32-10, false)) //nolint:testifylint + require.Equal(t, float64(math.MaxUint32)+21, c.Update(20, false)) //nolint:testifylint +}