Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Deprecate all Scraper helpers in scraperhelper #11732

Merged
merged 1 commit into from
Nov 25, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Deprecate all Scraper helpers in scraperhelper
Signed-off-by: Bogdan Drutu <bogdandrutu@gmail.com>
  • Loading branch information
bogdandrutu committed Nov 22, 2024
commit c0fec8a83b678ac228b579a1844eb25212afda7b
25 changes: 25 additions & 0 deletions .chloggen/dep-more-scrapers.yaml
Original file line number Diff line number Diff line change
@@ -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 all Scraper helpers in scraperhelper

# One or more tracking issues or pull requests related to the change
issues: [11732]

# (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: Deprecate ScrapeFunc, ScraperOption, WithStart, WithShutdown in favor of equivalent funcs in scraper package.

# 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: [user]
3 changes: 2 additions & 1 deletion receiver/scraperhelper/obs_metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"go.opentelemetry.io/collector/receiver/internal"
"go.opentelemetry.io/collector/receiver/scrapererror"
"go.opentelemetry.io/collector/receiver/scraperhelper/internal/metadata"
"go.opentelemetry.io/collector/scraper"
)

const (
Expand All @@ -31,7 +32,7 @@ const (
erroredMetricPointsKey = "errored_metric_points"
)

func newObsMetrics(delegate ScrapeFunc, receiverID component.ID, scraperID component.ID, telSettings component.TelemetrySettings) (ScrapeFunc, error) {
func newObsMetrics(delegate scraper.ScrapeMetricsFunc, receiverID component.ID, scraperID component.ID, telSettings component.TelemetrySettings) (scraper.ScrapeMetricsFunc, error) {
telemetryBuilder, errBuilder := metadata.NewTelemetryBuilder(telSettings)
if errBuilder != nil {
return nil, errBuilder
Expand Down
5 changes: 3 additions & 2 deletions receiver/scraperhelper/obs_metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ import (
"go.opentelemetry.io/collector/pdata/pmetric"
"go.opentelemetry.io/collector/pdata/testdata"
"go.opentelemetry.io/collector/receiver/scrapererror"
"go.opentelemetry.io/collector/scraper"
)

var (
Expand Down Expand Up @@ -47,7 +48,7 @@ func TestScrapeMetricsDataOp(t *testing.T) {
{items: 15, err: nil},
}
for i := range params {
var sf ScrapeFunc
var sf scraper.ScrapeMetricsFunc
sf, err = newObsMetrics(func(context.Context) (pmetric.Metrics, error) {
return testdata.GenerateMetrics(params[i].items), params[i].err
}, receiverID, scraperID, tt.TelemetrySettings())
Expand Down Expand Up @@ -94,7 +95,7 @@ func TestCheckScraperMetrics(t *testing.T) {
require.NoError(t, err)
t.Cleanup(func() { require.NoError(t, tt.Shutdown(context.Background())) })

var sf ScrapeFunc
var sf scraper.ScrapeMetricsFunc
sf, err = newObsMetrics(func(context.Context) (pmetric.Metrics, error) {
return testdata.GenerateMetrics(7), nil
}, receiverID, scraperID, tt.TelemetrySettings())
Expand Down
31 changes: 7 additions & 24 deletions receiver/scraperhelper/scraper.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,21 +9,15 @@

"go.opentelemetry.io/collector/component"
"go.opentelemetry.io/collector/pdata/pmetric"
"go.opentelemetry.io/collector/scraper"
)

var errNilFunc = errors.New("nil scrape func")

// ScrapeFunc scrapes metrics.
// Deprecated: [v0.115.0] use scraper.ScrapeMetricsFunc.
type ScrapeFunc func(context.Context) (pmetric.Metrics, error)

func (sf ScrapeFunc) ScrapeMetrics(ctx context.Context) (pmetric.Metrics, error) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not released.

return sf(ctx)
}

// Deprecated: [v0.115.0] use ScrapeMetrics.
func (sf ScrapeFunc) Scrape(ctx context.Context) (pmetric.Metrics, error) {
return sf.ScrapeMetrics(ctx)
return sf(ctx)

Check warning on line 20 in receiver/scraperhelper/scraper.go

View check run for this annotation

Codecov / codecov/patch

receiver/scraperhelper/scraper.go#L20

Added line #L20 was not covered by tests
}

// Deprecated: [v0.115.0] use scraper.Metrics.
Expand All @@ -32,7 +26,7 @@
Scrape(context.Context) (pmetric.Metrics, error)
}

// ScraperOption apply changes to internal options.
// Deprecated: [v0.115.0] use scraper.Option.
type ScraperOption interface {
apply(*baseScraper)
}
Expand All @@ -43,14 +37,14 @@
of(e)
}

// WithStart sets the function that will be called on startup.
// Deprecated: [v0.115.0] use scraper.WithStart.
func WithStart(start component.StartFunc) ScraperOption {
return scraperOptionFunc(func(o *baseScraper) {
o.StartFunc = start
})
}

// WithShutdown sets the function that will be called on shutdown.
// Deprecated: [v0.115.0] use scraper.WithShutdown.
func WithShutdown(shutdown component.ShutdownFunc) ScraperOption {
return scraperOptionFunc(func(o *baseScraper) {
o.ShutdownFunc = shutdown
Expand All @@ -63,14 +57,8 @@
ScrapeFunc
}

// NewScraper creates a scraper.Metrics that calls ScrapeMetrics at the specified collection interval,
// reports observability information, and passes the scraped metrics to the next consumer.
func NewScraper(scrape ScrapeFunc, options ...ScraperOption) (scraper.Metrics, error) {
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Not breaking change because not released yet.

return newBaseScraper(scrape, options...)
}

// TODO: Remove this and embed into NewScraper when NewScraperWithoutType is removed.
func newBaseScraper(scrape ScrapeFunc, options ...ScraperOption) (*baseScraper, error) {
// Deprecated: [v0.115.0] use scraper.NewMetrics.
func NewScraperWithoutType(scrape ScrapeFunc, options ...ScraperOption) (Scraper, error) {

Check warning on line 61 in receiver/scraperhelper/scraper.go

View check run for this annotation

Codecov / codecov/patch

receiver/scraperhelper/scraper.go#L61

Added line #L61 was not covered by tests
if scrape == nil {
return nil, errNilFunc
}
Expand All @@ -83,8 +71,3 @@

return bs, nil
}

// Deprecated: [v0.115.0] use NewScraper.
func NewScraperWithoutType(scrape ScrapeFunc, options ...ScraperOption) (Scraper, error) {
return newBaseScraper(scrape, options...)
}
10 changes: 5 additions & 5 deletions receiver/scraperhelper/scrapercontroller.go
Original file line number Diff line number Diff line change
Expand Up @@ -46,10 +46,10 @@
}

// Deprecated: [v0.115.0] use AddScraper.
func AddScraperWithType(t component.Type, scraper Scraper) ScraperControllerOption {
func AddScraperWithType(t component.Type, scrp Scraper) ScraperControllerOption {

Check warning on line 49 in receiver/scraperhelper/scrapercontroller.go

View check run for this annotation

Codecov / codecov/patch

receiver/scraperhelper/scrapercontroller.go#L49

Added line #L49 was not covered by tests
// Ignore the error since it cannot happen because the Scrape func cannot be nil here.
scrp, _ := NewScraper(scraper.Scrape, WithStart(scraper.Start), WithShutdown(scraper.Shutdown))
return AddScraper(t, scrp)
newScrp, _ := scraper.NewMetrics(scrp.Scrape, scraper.WithStart(scrp.Start), scraper.WithShutdown(scrp.Shutdown))
return AddScraper(t, newScrp)

Check warning on line 52 in receiver/scraperhelper/scrapercontroller.go

View check run for this annotation

Codecov / codecov/patch

receiver/scraperhelper/scrapercontroller.go#L51-L52

Added lines #L51 - L52 were not covered by tests
}

// WithTickerChannel allows you to override the scraper controller's ticker
Expand Down Expand Up @@ -116,12 +116,12 @@
for i := range sc.scrapers {
telSet := set.TelemetrySettings
telSet.Logger = telSet.Logger.With(zap.String("scraper", sc.scrapers[i].id.String()))
var obsScrp ScrapeFunc
var obsScrp scraper.ScrapeMetricsFunc
obsScrp, err = newObsMetrics(sc.scrapers[i].ScrapeMetrics, set.ID, sc.scrapers[i].id, telSet)
if err != nil {
return nil, err
}
sc.obsScrapers[i], err = NewScraper(obsScrp, WithStart(sc.scrapers[i].Start), WithShutdown(sc.scrapers[i].Shutdown))
sc.obsScrapers[i], err = scraper.NewMetrics(obsScrp, scraper.WithStart(sc.scrapers[i].Start), scraper.WithShutdown(sc.scrapers[i].Shutdown))
if err != nil {
return nil, err
}
Expand Down
17 changes: 9 additions & 8 deletions receiver/scraperhelper/scrapercontroller_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@ import (
"go.opentelemetry.io/collector/receiver"
"go.opentelemetry.io/collector/receiver/receivertest"
"go.opentelemetry.io/collector/receiver/scrapererror"
"go.opentelemetry.io/collector/scraper"
)

type testInitialize struct {
Expand Down Expand Up @@ -198,21 +199,21 @@ func configureMetricOptions(t *testing.T, test metricsTestCase, initializeChs []
var metricOptions []ScraperControllerOption

for i := 0; i < test.scrapers; i++ {
var scraperOptions []ScraperOption
var scraperOptions []scraper.Option
if test.initialize {
initializeChs[i] = make(chan bool, 1)
ti := &testInitialize{ch: initializeChs[i], err: test.initializeErr}
scraperOptions = append(scraperOptions, WithStart(ti.start))
scraperOptions = append(scraperOptions, scraper.WithStart(ti.start))
}
if test.close {
closeChs[i] = make(chan bool, 1)
tc := &testClose{ch: closeChs[i], err: test.closeErr}
scraperOptions = append(scraperOptions, WithShutdown(tc.shutdown))
scraperOptions = append(scraperOptions, scraper.WithShutdown(tc.shutdown))
}

scrapeMetricsChs[i] = make(chan int)
tsm := &testScrapeMetrics{ch: scrapeMetricsChs[i], err: test.scrapeErr}
scp, err := NewScraper(tsm.scrape, scraperOptions...)
scp, err := scraper.NewMetrics(tsm.scrape, scraperOptions...)
require.NoError(t, err)

metricOptions = append(metricOptions, AddScraper(component.MustNewType("scraper"), scp))
Expand Down Expand Up @@ -314,7 +315,7 @@ func TestSingleScrapePerInterval(t *testing.T) {

tickerCh := make(chan time.Time)

scp, err := NewScraper(tsm.scrape)
scp, err := scraper.NewMetrics(tsm.scrape)
require.NoError(t, err)

recv, err := NewScraperControllerReceiver(
Expand Down Expand Up @@ -356,7 +357,7 @@ func TestScrapeControllerStartsOnInit(t *testing.T) {
ch: make(chan int, 1),
}

scp, err := NewScraper(tsm.scrape)
scp, err := scraper.NewMetrics(tsm.scrape)
require.NoError(t, err, "Must not error when creating scraper")

r, err := NewScraperControllerReceiver(
Expand Down Expand Up @@ -392,7 +393,7 @@ func TestScrapeControllerInitialDelay(t *testing.T) {
}
)

scp, err := NewScraper(func(context.Context) (pmetric.Metrics, error) {
scp, err := scraper.NewMetrics(func(context.Context) (pmetric.Metrics, error) {
elapsed <- time.Now()
return pmetric.NewMetrics(), nil
})
Expand Down Expand Up @@ -421,7 +422,7 @@ func TestShutdownBeforeScrapeCanStart(t *testing.T) {
InitialDelay: 5 * time.Second,
}

scp, err := NewScraper(func(context.Context) (pmetric.Metrics, error) {
scp, err := scraper.NewMetrics(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
Expand Down
2 changes: 1 addition & 1 deletion scraper/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -87,5 +87,5 @@ func TestMakeFactoryMap(t *testing.T) {
}

func createMetrics(context.Context, Settings, component.Config) (Metrics, error) {
return NewMetrics(context.Background(), nopSettings(), newTestScrapeMetricsFunc(nil))
return NewMetrics(newTestScrapeMetricsFunc(nil))
}
2 changes: 1 addition & 1 deletion scraper/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ type metrics struct {
}

// NewMetrics creates a new Metrics scraper.
func NewMetrics(_ context.Context, _ Settings, scrape ScrapeMetricsFunc, options ...Option) (Metrics, error) {
func NewMetrics(scrape ScrapeMetricsFunc, options ...Option) (Metrics, error) {
if scrape == nil {
return nil, errNilFunc
}
Expand Down
10 changes: 5 additions & 5 deletions scraper/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ import (
)

func TestNewMetrics(t *testing.T) {
mp, err := NewMetrics(context.Background(), nopSettings(), newTestScrapeMetricsFunc(nil))
mp, err := NewMetrics(newTestScrapeMetricsFunc(nil))
require.NoError(t, err)

require.NoError(t, mp.Start(context.Background(), componenttest.NewNopHost()))
Expand All @@ -30,7 +30,7 @@ func TestNewMetrics(t *testing.T) {

func TestNewMetrics_WithOptions(t *testing.T) {
want := errors.New("my_error")
mp, err := NewMetrics(context.Background(), nopSettings(), newTestScrapeMetricsFunc(nil),
mp, err := NewMetrics(newTestScrapeMetricsFunc(nil),
WithStart(func(context.Context, component.Host) error { return want }),
WithShutdown(func(context.Context) error { return want }))
require.NoError(t, err)
Expand All @@ -40,13 +40,13 @@ func TestNewMetrics_WithOptions(t *testing.T) {
}

func TestNewMetrics_NilRequiredFields(t *testing.T) {
_, err := NewMetrics(context.Background(), nopSettings(), nil)
_, err := NewMetrics(nil)
require.Error(t, err)
}

func TestNewMetrics_ProcessMetricsError(t *testing.T) {
want := errors.New("my_error")
mp, err := NewMetrics(context.Background(), nopSettings(), newTestScrapeMetricsFunc(want))
mp, err := NewMetrics(newTestScrapeMetricsFunc(want))
require.NoError(t, err)
_, err = mp.ScrapeMetrics(context.Background())
require.ErrorIs(t, err, want)
Expand All @@ -60,7 +60,7 @@ func TestMetricsConcurrency(t *testing.T) {
dps.AppendEmpty()
dps.AppendEmpty()

mp, err := NewMetrics(context.Background(), nopSettings(), newTestScrapeMetricsFunc(nil))
mp, err := NewMetrics(newTestScrapeMetricsFunc(nil))
require.NoError(t, err)
require.NoError(t, mp.Start(context.Background(), componenttest.NewNopHost()))

Expand Down
Loading