diff --git a/pkg/collector/collection.go b/pkg/collector/collection.go index 014ea20c..d424cfa7 100644 --- a/pkg/collector/collection.go +++ b/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, diff --git a/pkg/collector/collection_test.go b/pkg/collector/collection_test.go index 775363c7..f5cd24ea 100644 --- a/pkg/collector/collection_test.go +++ b/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) +} diff --git a/pkg/collector/types.go b/pkg/collector/types.go index a22e5d26..1485ef46 100644 --- a/pkg/collector/types.go +++ b/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