From 60d316075fbf17b10ca5918e166d89a368c609e7 Mon Sep 17 00:00:00 2001 From: Bogdan Drutu Date: Tue, 12 Nov 2024 16:30:17 -0800 Subject: [PATCH] Deprecate Scraper.ID func, pass type when register Scraper (#11659) Changes interface to get closer to https://github.com/open-telemetry/opentelemetry-collector/pull/11657 Signed-off-by: Bogdan Drutu --- .chloggen/rm-scraper-id.yaml | 25 ++++++++++++++++++ receiver/scraperhelper/scraper.go | 21 ++++++++++++--- receiver/scraperhelper/scrapercontroller.go | 25 +++++++++++++----- .../scraperhelper/scrapercontroller_test.go | 26 +++++++++---------- 4 files changed, 75 insertions(+), 22 deletions(-) create mode 100644 .chloggen/rm-scraper-id.yaml diff --git a/.chloggen/rm-scraper-id.yaml b/.chloggen/rm-scraper-id.yaml new file mode 100644 index 00000000000..20c83ddef65 --- /dev/null +++ b/.chloggen/rm-scraper-id.yaml @@ -0,0 +1,25 @@ +# Use this changelog template to create an entry for release notes. + +# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' +change_type: deprecation + +# The name of the component, or a single word describing the area of concern, (e.g. otlpreceiver) +component: scraperhelper + +# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`). +note: Deprecate Scraper.ID func, pass type when register Scraper + +# One or more tracking issues or pull requests related to the change +issues: [11238] + +# (Optional) One or more lines of additional information to render under the primary note. +# These lines will be padded with 2 spaces and then inserted directly into the document. +# Use pipe (|) for multiline entries. +subtext: + +# Optional: The change log or logs in which this entry should be included. +# e.g. '[user]' or '[user, api]' +# Include 'user' if the change is relevant to end users. +# Include 'api' if there is a change to a library API. +# Default: '[user]' +change_logs: [api] diff --git a/receiver/scraperhelper/scraper.go b/receiver/scraperhelper/scraper.go index aa335af87f6..1ba46f333ca 100644 --- a/receiver/scraperhelper/scraper.go +++ b/receiver/scraperhelper/scraper.go @@ -24,7 +24,7 @@ func (sf ScrapeFunc) Scrape(ctx context.Context) (pmetric.Metrics, error) { type Scraper interface { component.Component - // ID returns the scraper id. + // Deprecated: [v0.114.0] use AddScraperWithType. ID() component.ID Scrape(context.Context) (pmetric.Metrics, error) } @@ -67,8 +67,7 @@ func (b *baseScraper) ID() component.ID { return b.id } -// NewScraper creates a Scraper that calls Scrape at the specified collection interval, -// reports observability information, and passes the scraped metrics to the next consumer. +// Deprecated: [v0.114.0] use NewScraperWithoutType. func NewScraper(t component.Type, scrape ScrapeFunc, options ...ScraperOption) (Scraper, error) { if scrape == nil { return nil, errNilFunc @@ -83,3 +82,19 @@ func NewScraper(t component.Type, scrape ScrapeFunc, options ...ScraperOption) ( return bs, nil } + +// NewScraperWithoutType creates a Scraper that calls Scrape at the specified collection interval, +// reports observability information, and passes the scraped metrics to the next consumer. +func NewScraperWithoutType(scrape ScrapeFunc, options ...ScraperOption) (Scraper, error) { + if scrape == nil { + return nil, errNilFunc + } + bs := &baseScraper{ + ScrapeFunc: scrape, + } + for _, op := range options { + op.apply(bs) + } + + return bs, nil +} diff --git a/receiver/scraperhelper/scrapercontroller.go b/receiver/scraperhelper/scrapercontroller.go index 76ed253b42c..c8e37e393c1 100644 --- a/receiver/scraperhelper/scrapercontroller.go +++ b/receiver/scraperhelper/scrapercontroller.go @@ -30,14 +30,22 @@ func (of scraperControllerOptionFunc) apply(e *controller) { of(e) } -// AddScraper configures the provided scrape function to be called +// Deprecated: [v0.114.0] use AddScraperWithType. +func AddScraper(scraper Scraper) ScraperControllerOption { + return AddScraperWithType(scraper.ID().Type(), scraper) +} + +// AddScraperWithType configures the provided scrape function to be called // with the specified options, and at the specified collection interval. // // Observability information will be reported, and the scraped metrics // will be passed to the next consumer. -func AddScraper(scraper Scraper) ScraperControllerOption { +func AddScraperWithType(t component.Type, scraper Scraper) ScraperControllerOption { return scraperControllerOptionFunc(func(o *controller) { - o.scrapers = append(o.scrapers, scraper) + o.scrapers = append(o.scrapers, scraperWithID{ + Scraper: scraper, + id: component.NewID(t), + }) }) } @@ -58,7 +66,7 @@ type controller struct { timeout time.Duration nextConsumer consumer.Metrics - scrapers []Scraper + scrapers []scraperWithID obsScrapers []*obsReport tickerCh <-chan time.Time @@ -69,6 +77,11 @@ type controller struct { obsrecv *receiverhelper.ObsReport } +type scraperWithID struct { + Scraper + id component.ID +} + // NewScraperControllerReceiver creates a Receiver with the configured options, that can control multiple scrapers. func NewScraperControllerReceiver( cfg *ControllerConfig, @@ -104,7 +117,7 @@ func NewScraperControllerReceiver( for i, scraper := range sc.scrapers { sc.obsScrapers[i], err = newScraper(obsReportSettings{ ReceiverID: sc.id, - Scraper: scraper.ID(), + Scraper: scraper.id, ReceiverCreateSettings: set, }) if err != nil { @@ -190,7 +203,7 @@ func (sc *controller) scrapeMetricsAndReport() { md, err := scraper.Scrape(ctx) if err != nil { - sc.logger.Error("Error scraping metrics", zap.Error(err), zap.Stringer("scraper", scraper.ID())) + sc.logger.Error("Error scraping metrics", zap.Error(err), zap.Stringer("scraper", scraper.id)) if !scrapererror.IsPartialScrapeError(err) { scrp.EndMetricsOp(ctx, 0, err) continue diff --git a/receiver/scraperhelper/scrapercontroller_test.go b/receiver/scraperhelper/scrapercontroller_test.go index 88e84951104..09097b74068 100644 --- a/receiver/scraperhelper/scrapercontroller_test.go +++ b/receiver/scraperhelper/scrapercontroller_test.go @@ -212,10 +212,10 @@ func configureMetricOptions(t *testing.T, test metricsTestCase, initializeChs [] scrapeMetricsChs[i] = make(chan int) tsm := &testScrapeMetrics{ch: scrapeMetricsChs[i], err: test.scrapeErr} - scp, err := NewScraper(component.MustNewType("scraper"), tsm.scrape, scraperOptions...) + scp, err := NewScraperWithoutType(tsm.scrape, scraperOptions...) require.NoError(t, err) - metricOptions = append(metricOptions, AddScraper(scp)) + metricOptions = append(metricOptions, AddScraperWithType(component.MustNewType("scraper"), scp)) } return metricOptions @@ -314,20 +314,20 @@ func TestSingleScrapePerInterval(t *testing.T) { tickerCh := make(chan time.Time) - scp, err := NewScraper(component.MustNewType("scaper"), tsm.scrape) + scp, err := NewScraperWithoutType(tsm.scrape) require.NoError(t, err) - receiver, err := NewScraperControllerReceiver( + recv, err := NewScraperControllerReceiver( cfg, receivertest.NewNopSettings(), new(consumertest.MetricsSink), - AddScraper(scp), + AddScraperWithType(component.MustNewType("scaper"), scp), WithTickerChannel(tickerCh), ) require.NoError(t, err) - require.NoError(t, receiver.Start(context.Background(), componenttest.NewNopHost())) - defer func() { require.NoError(t, receiver.Shutdown(context.Background())) }() + require.NoError(t, recv.Start(context.Background(), componenttest.NewNopHost())) + defer func() { require.NoError(t, recv.Shutdown(context.Background())) }() tickerCh <- time.Now() @@ -356,7 +356,7 @@ func TestScrapeControllerStartsOnInit(t *testing.T) { ch: make(chan int, 1), } - scp, err := NewScraper(component.MustNewType("scraper"), tsm.scrape) + scp, err := NewScraperWithoutType(tsm.scrape) require.NoError(t, err, "Must not error when creating scraper") r, err := NewScraperControllerReceiver( @@ -366,7 +366,7 @@ func TestScrapeControllerStartsOnInit(t *testing.T) { }, receivertest.NewNopSettings(), new(consumertest.MetricsSink), - AddScraper(scp), + AddScraperWithType(component.MustNewType("scaper"), scp), ) require.NoError(t, err, "Must not error when creating scrape controller") @@ -392,7 +392,7 @@ func TestScrapeControllerInitialDelay(t *testing.T) { } ) - scp, err := NewScraper(component.MustNewType("timed"), func(context.Context) (pmetric.Metrics, error) { + scp, err := NewScraperWithoutType(func(context.Context) (pmetric.Metrics, error) { elapsed <- time.Now() return pmetric.NewMetrics(), nil }) @@ -402,7 +402,7 @@ func TestScrapeControllerInitialDelay(t *testing.T) { &cfg, receivertest.NewNopSettings(), new(consumertest.MetricsSink), - AddScraper(scp), + AddScraperWithType(component.MustNewType("scaper"), scp), ) require.NoError(t, err, "Must not error when creating receiver") @@ -421,7 +421,7 @@ func TestShutdownBeforeScrapeCanStart(t *testing.T) { InitialDelay: 5 * time.Second, } - scp, err := NewScraper(component.MustNewType("timed"), func(context.Context) (pmetric.Metrics, error) { + scp, err := NewScraperWithoutType(func(context.Context) (pmetric.Metrics, error) { // make the scraper wait for long enough it would disrupt a shutdown. time.Sleep(30 * time.Second) return pmetric.NewMetrics(), nil @@ -432,7 +432,7 @@ func TestShutdownBeforeScrapeCanStart(t *testing.T) { &cfg, receivertest.NewNopSettings(), new(consumertest.MetricsSink), - AddScraper(scp), + AddScraperWithType(component.MustNewType("scaper"), scp), ) require.NoError(t, err, "Must not error when creating receiver") require.NoError(t, r.Start(context.Background(), componenttest.NewNopHost()))