From 6b82e5a05f23590dc929a2a1c27ff680d79234e7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Jan-Otto=20Kr=C3=B6pke?= Date: Thu, 15 Oct 2026 11:22:35 +0200 Subject: [PATCH] terminal_services: keep session_info when the Terminal Services Session object is missing (#2708) Signed-off-by: Claude Co-authored-by: Claude --- docs/collector.terminal_services.md | 2 + .../terminal_services/terminal_services.go | 30 +++++++-- .../terminal_services_internal_test.go | 63 +++++++++++++++++++ 3 files changed, 91 insertions(+), 4 deletions(-) create mode 100644 internal/collector/terminal_services/terminal_services_internal_test.go diff --git a/docs/collector.terminal_services.md b/docs/collector.terminal_services.md index 3509e324..45cca9b9 100644 --- a/docs/collector.terminal_services.md +++ b/docs/collector.terminal_services.md @@ -37,6 +37,8 @@ None `*` The `connection_broker_*` metrics are only collected if the server has the `Remote Desktop Connection Broker` role and its performance counters can be read. If the counters can't be read, the collector logs a warning and still exports the session metrics. +The per-session process metrics (`handles`, `page_fault_total`, `page_file_bytes*`, `cpu_time_seconds_total`, `pool_*_bytes`, `private_bytes`, `threads`, `virtual_bytes*` and `working_set_bytes*`) are read from the `Terminal Services Session` performance object. Some hosts don't register this object, for example some Windows Server Core installations. There, the collector logs one warning at startup, skips these metrics and still exports `session_info` and the Connection Broker metrics. + ### Example metric diff --git a/internal/collector/terminal_services/terminal_services.go b/internal/collector/terminal_services/terminal_services.go index 2b8422d5..fd41fcc5 100644 --- a/internal/collector/terminal_services/terminal_services.go +++ b/internal/collector/terminal_services/terminal_services.go @@ -44,6 +44,12 @@ const ( connectionBrokerServiceName = "Tssdis" ) +// terminalServicesSessionObject is the performance object of the per-session +// process metrics. Tests replace it to simulate a host without the object. +// +//nolint:gochecknoglobals +var terminalServicesSessionObject = "Terminal Services Session" + type Config struct{} //nolint:gochecknoglobals @@ -288,9 +294,18 @@ func (c *Collector) Build(logger *slog.Logger, miSession *mi.Session) error { return fmt.Errorf("failed to open WTS server: %w", err) } - c.perfDataCollectorTerminalServicesSession, err = pdh.NewQueryCollector[perfDataCounterValuesTerminalServicesSession](c.logger, c.perfDataQuery, pdh.CounterTypeRaw, "Terminal Services Session", pdh.InstancesAll) + c.perfDataCollectorTerminalServicesSession, err = pdh.NewQueryCollector[perfDataCounterValuesTerminalServicesSession](c.logger, c.perfDataQuery, pdh.CounterTypeRaw, terminalServicesSessionObject, pdh.InstancesAll) if err != nil { - return fmt.Errorf("failed to create Terminal Services Session collector: %w", err) + // Some hosts, e.g. some Windows Server Core installations, don't + // register the Terminal Services Session object. session_info comes + // from the WTS API, so keep it and skip only the per-session process + // metrics, instead of failing the whole collector. + c.logger.Warn("failed to create Terminal Services Session collector, skipping per-session performance metrics", + slog.Any("err", err), + ) + + c.perfDataCollectorTerminalServicesSession.Close() + c.perfDataCollectorTerminalServicesSession = nil } return nil @@ -305,6 +320,11 @@ func (c *Collector) Collect(ch chan<- prometheus.Metric, _ time.Duration) error errs = append(errs, fmt.Errorf("failed collecting terminal services session infos: %w", err)) } + // Without session and Connection Broker counters, the query has nothing to sample. + if c.perfDataCollectorTerminalServicesSession == nil && !c.connectionBrokerEnabled { + return errors.Join(errs...) + } + // The session and Connection Broker counters read this sample. Unlike // netframework, a failed sample, including pdh.ErrNoData, skips both, so // they don't publish the values of an earlier sample again. @@ -314,8 +334,10 @@ func (c *Collector) Collect(ch chan<- prometheus.Metric, _ time.Duration) error return errors.Join(errs...) } - if err := c.collectTSSessionCounters(ch); err != nil { - errs = append(errs, fmt.Errorf("failed collecting terminal services session count metrics: %w", err)) + if c.perfDataCollectorTerminalServicesSession != nil { + if err := c.collectTSSessionCounters(ch); err != nil { + errs = append(errs, fmt.Errorf("failed collecting terminal services session count metrics: %w", err)) + } } // only collect CollectionBrokerPerformance if host is a Connection Broker diff --git a/internal/collector/terminal_services/terminal_services_internal_test.go b/internal/collector/terminal_services/terminal_services_internal_test.go new file mode 100644 index 00000000..a13ff46c --- /dev/null +++ b/internal/collector/terminal_services/terminal_services_internal_test.go @@ -0,0 +1,63 @@ +// 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 terminal_services + +import ( + "log/slog" + "strings" + "testing" + "time" + + "github.com/prometheus/client_golang/prometheus" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" +) + +// TestCollectorWithoutSessionObject covers hosts without the Terminal Services +// Session performance object, e.g. Server Core installations: the collector +// must build and scrape without errors, and skip only the per-session +// process metrics. +func TestCollectorWithoutSessionObject(t *testing.T) { + original := terminalServicesSessionObject + terminalServicesSessionObject = "windows_exporter missing object" + + t.Cleanup(func() { terminalServicesSessionObject = original }) + + c := New(nil) + + t.Cleanup(func() { assert.NoError(t, c.Close()) }) + + require.NoError(t, c.Build(slog.New(slog.DiscardHandler), nil)) + require.Nil(t, c.perfDataCollectorTerminalServicesSession) + + for range 2 { + ch := make(chan prometheus.Metric, 1024) + + require.NoError(t, c.Collect(ch, time.Second)) + close(ch) + + for metric := range ch { + desc := metric.Desc().String() + assert.NotContains(t, desc, `"windows_terminal_services_handles"`) + assert.True(t, + strings.Contains(desc, `"windows_terminal_services_session_info"`) || + strings.Contains(desc, `"windows_terminal_services_connection_broker_`), + "unexpected metric %s", desc) + } + } +}