collector: make filtered views non-owning (#2587)

Signed-off-by: Jan-Otto Kröpke <mail@jkroepke.de>

pkg/collector/collection.go

@@ -308,9 +308,16 @@ func (c *Collection) Build(ctx context.Context, logger *slog.Logger) error {
 // because closing it would release resources that are still in use.
 // The MI session is then left open as well.
 //
+// Close does nothing on a filtered view returned by WithCollectors. The original
+// Collection owns the shared collectors and MI resources and must be closed.
+//
 // Close is safe to call after a failed or partial Build and more than once.
 // Collectors that were never built are not closed. A closed collector is never built or collected again.
 func (c *Collection) Close() error {
+	if c.isView {
+		return nil
+	}
+
 	c.state.closeMu.Lock()
 	defer c.state.closeMu.Unlock()
 
@@ -390,10 +397,13 @@ func newMISession(app *mi.Application, destinationOptions *mi.DestinationOptions
 // WithCollectors To be called by the exporter for collector initialization.
 // The returned Collection shares the collector instances and their state with c,
 // so a collector never runs twice at the same time, whichever Collection scrapes it.
+// It does not own these resources: Close on a view does nothing. Close the original
+// Collection when all of its views are no longer needed.
 func (c *Collection) WithCollectors(collectors []string) (*Collection, error) {
 	metricCollectors := &Collection{
 		startTime:                   c.startTime,
 		state:                       c.state,
+		isView:                      true,
 		scrapeDurationDesc:          c.scrapeDurationDesc,
 		collectorScrapeDurationDesc: c.collectorScrapeDurationDesc,
 		collectorScrapeSuccessDesc:  c.collectorScrapeSuccessDesc,

pkg/collector/collection_test.go

@@ -117,3 +117,63 @@ func (c *lifecycleCollector) Close() error {
 
 	return c.closeErr
 }
+
+func TestFilteredCloseKeepsNativeSession(t *testing.T) {
+	t.Parallel()
+
+	selected := &sessionProbeCollector{name: "selected"}
+	omitted := &sessionProbeCollector{name: "omitted"}
+	collection := collector.New(collector.Map{"selected": selected, "omitted": omitted})
+	require.NoError(t, collection.Build(t.Context(), slog.New(slog.DiscardHandler)))
+	t.Cleanup(func() { require.NoError(t, collection.Close()) })
+
+	view, err := collection.WithCollectors([]string{"selected"})
+	require.NoError(t, err)
+	require.NoError(t, view.Close())
+	require.False(t, selected.closed)
+	require.False(t, omitted.closed)
+
+	query, err := mi.NewQuery("SELECT Caption FROM Win32_OperatingSystem")
+	require.NoError(t, err)
+
+	var result []struct{ Caption string }
+	require.NoError(t, omitted.session.Query(&result, mi.NamespaceRootCIMv2, query, time.Second*5))
+	require.NotEmpty(t, result)
+	require.NoError(t, collection.Close())
+	require.True(t, selected.closed)
+	require.True(t, omitted.closed)
+}
+
+type sessionProbeCollector struct {
+	lifecycleCollector
+
+	session *mi.Session
+}
+
+func (c *sessionProbeCollector) Build(_ *slog.Logger, session *mi.Session) error {
+	c.session = session
+
+	return nil
+}
+
+func TestFilteredCloseWhileOmittedCollectorActive(t *testing.T) {
+	t.Parallel()
+
+	selected := &lifecycleCollector{name: "selected"}
+	running := newBlockingCollector()
+	collection := collector.New(collector.Map{"selected": selected, "test": running})
+	require.NoError(t, collection.Build(t.Context(), slog.New(slog.DiscardHandler)))
+	t.Cleanup(func() { require.NoError(t, collection.Close()) })
+
+	view, err := collection.WithCollectors([]string{"selected"})
+	require.NoError(t, err)
+	result := scrapeInBackground(t.Context(), collection, time.Minute)
+
+	<-running.started
+	require.NoError(t, view.Close())
+	require.False(t, selected.closed)
+	close(running.block)
+	require.Contains(t, <-result, `windows_exporter_collector_success{collector="test"} 1`)
+	require.NoError(t, collection.Close())
+	require.True(t, selected.closed)
+}

pkg/collector/types.go

@@ -34,6 +34,8 @@ type Collection struct {
 
 	// state is shared with every Collection derived from this one by WithCollectors.
 	state *collectionState
+	// isView marks a non-owning view returned by WithCollectors.
+	isView bool
 
 	scrapeDurationDesc          *prometheus.Desc
 	collectorScrapeDurationDesc *prometheus.Desc