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

1/2 Add receiverhelper functions for creating flexible "scraper" metrics receiver #1886

Merged

Conversation

james-bebbington
Copy link
Member

@james-bebbington james-bebbington commented Oct 1, 2020

  • Added receiverhelper functions that follow a similar pattern to the other component helpers.
  • Added options to configure scrape targets that make it easy to construct a receiver that scrapes sets of metrics at configured frequency(ies).
  • Note: Currently only implemented this for Metrics as I don't believe the "scrape" model applies to Traces or Logs.

Example Usages:

  1. Simple receiver with single scrape target:
type Config struct {
    configmodels.ReceiverSettings  `mapstructure:",squash"`
    receiverhelper.ScraperSettings `mapstructure:",squash"`
}

...

scraper := newScraper(cfg)

return receiverhelper.NewMetricReceiver(
    cfg,
    consumer,
    receiverhelper.AddScraper(cfg.ScraperSettings, scraper.scrape),
)
  1. Receiver with multiple scrape targets with additional configuration:
type Config struct {
    configmodels.ReceiverSettings  `mapstructure:",squash"`

    Scraper1Config receiverhelper.ScraperSettings  `mapstructure:",scraper1"`
    Scraper2Config receiverhelper.ScraperSettings  `mapstructure:",scraper2"`
}

...

scraper1 := newScraper1(cfg.Scraper1Config)
scraper2 := newScraper2(cfg.Scraper2Config)

return receiverhelper.NewMetricReceiver(
    cfg,
    consumer,
    receiverhelper.WithBaseOptions(
        receiverhelper.WithStart(start),
        receiverhelper.WithShutdown(shutdown),
    ),
    receiverhelper.WithDefaultCollectionInterval(30 * time.Second),
    receiverhelper.AddScraper(
        cfg.Scraper1Config, // scraper config embeds collection_interval that will override the default if set
        scraper1.scrape,
        receiverhelper.WithInitialize(scraper1.initialize),
        receiverhelper.WithClose(scraper1.close),
    ),
    receiverhelper.AddScraper(
        cfg.Scraper2Config,  // scraper config embeds collection_interval that will override the default if set
        scraper2.scrape,
        receiverhelper.WithInitialize(scraper2.initialize),
        receiverhelper.WithClose(scraper2.close),
    ),
)
  1. Note it's also valid to create a Metrics Receiver that doesn't scrape but this doesn't do much for you:
type Config struct {
    configmodels.ReceiverSettings  `mapstructure:",squash"`
}

...

return receiverhelper.NewMetricReceiver(
    cfg,
    consumer,
    receiverhelper.WithBaseOptions(
        receiverhelper.WithStart(start),
        receiverhelper.WithShutdown(shutdown),
    ),
)

Resolves #934
Supersedes #1875

}

return componenterror.CombineErrors(errors)
}
Copy link
Member Author

@james-bebbington james-bebbington Oct 1, 2020

Choose a reason for hiding this comment

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

Maybe we would want to add / extend NewTraceReceiver, NewMetricReceiver, & NewLogsReceiver functions here at some point that implement standard HTTP and/or gRPC servers for "push" type receivers.

I haven't implemented any of those types of receivers so I'm not sure if it would be straightforward to factor out that code, or worth the effort. For now, these helper functions are only intended to support the NewMetricReceiver function where you are configuring at least one scrape target, i.e. for "pull" type receivers.

type MetricOption func(*metricsReceiver)

// WithBaseOptions applies any base options to a metrics receiver.
func WithBaseOptions(options ...Option) MetricOption {
Copy link
Member Author

@james-bebbington james-bebbington Oct 1, 2020

Choose a reason for hiding this comment

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

I'm interested if anyone can think of a better way of doing this btw. This seems slightly nicer than changing the NewMetricReceiver definition to:

func NewMetricReceiver(config configmodels.Receiver, nextConsumer consumer.MetricsConsumer, baseOptions []Option, options ...MetricOption) (component.Receiver, error)

@jrcamp jrcamp self-assigned this Oct 1, 2020
@jrcamp
Copy link
Contributor

jrcamp commented Oct 1, 2020

Seems like scraping could be its own package that builds on top of receiverhelper. Trying to sketch out what this might look like in use:

Edit: ignore most of this, yours is more in line with the existing API. But what do you think about having the embedded ScrapeConfig struct that gets passed around instead of having to call WithCollectionInterval() (maybe we'll add more options in the future to scraper config and this would prevent having to update all receivers that use it from having to update to pass them through).

package scraping

type ScrapeConfig struct {
    CollectionInterval int    
}

package redisreceiver

type Config struct {
    scraping.ScrapeConfig
    // other config here
}

type scraper struct {
}

// maybe put these in params so it can be easily expanded later. would
// probably need log instance in it as well?
func (s *scraper) collect(ctx, consumer.MetricsConsumer) {
}

type redisReceiver struct {
    
}

func (r *redisReceiver) start() {
}

func (r *redisReceiver) shutdown() {
}

s := &scraper{}
r := &redisReceiver{}

sb := scraping.Builder{}
sb.AddScraper(
    cfg.ScrapeConfig,
    scraping.WithInit(s.init), 
    scraping.WithCollect(s.collect), 
    scraping.WithClose(s.close))
// Build takes all receiverhelper options and calls them before scraper Inits / after scraper Closes.    
sb.NewMetricsReceiver(cfg, receiverhelper.WithStart(r.start), receiverhelper.WithShutdown(r.shutdown))    

@james-bebbington
Copy link
Member Author

james-bebbington commented Oct 2, 2020

what do you think about having the embedded ScrapeConfig struct that gets passed around instead of having to call WithCollectionInterval() (maybe we'll add more options in the future to scraper config and this would prevent having to update all receivers that use it from having to update to pass them through).

Yea I went back and forwards on this, but I guess it is nicer & more future proof to include this. Have added & updated examples - LMKWYT.

I could go further and also add a ReceiverScraperConfig for the receiver itself that embeds ReceiverSettings and adds the DefaultCollectionInterval value, but that might be overkill?

@codecov
Copy link

codecov bot commented Oct 2, 2020

Codecov Report

Merging #1886 into master will increase coverage by 0.03%.
The diff coverage is 96.47%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1886      +/-   ##
==========================================
+ Coverage   91.28%   91.32%   +0.03%     
==========================================
  Files         277      279       +2     
  Lines       16489    16574      +85     
==========================================
+ Hits        15052    15136      +84     
- Misses       1006     1007       +1     
  Partials      431      431              
Impacted Files Coverage Δ
receiver/receiverhelper/scraper.go 90.00% <90.00%> (ø)
receiver/receiverhelper/receiver.go 97.33% <97.33%> (ø)
translator/internaldata/resource_to_oc.go 91.48% <0.00%> (+2.12%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dab0c56...9ddaf6f. Read the comment docs.

@james-bebbington james-bebbington changed the title Add receiverhelper functions for creating flexible "scraper" metrics receiver 1/2 Add receiverhelper functions for creating flexible "scraper" metrics receiver Oct 2, 2020
@james-bebbington
Copy link
Member Author

james-bebbington commented Oct 2, 2020

Copy link
Member

@bogdandrutu bogdandrutu left a comment

Choose a reason for hiding this comment

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

I think we should merge this and iterate over the design

Collector automation moved this from In progress to Reviewer approved Oct 9, 2020
@bogdandrutu bogdandrutu merged commit 478a983 into open-telemetry:master Oct 9, 2020
Collector automation moved this from Reviewer approved to Done Oct 9, 2020
tigrannajaryan pushed a commit to open-telemetry/opentelemetry-collector-contrib that referenced this pull request Oct 30, 2020
…#1211)

Adds skeleton for Windows Performance Counters Receiver with very basic initial functionality that can read simple counters that do not include any instance into a Gauge metric - see the added README.md for more details.

**Link to tracking Issue:** #1088

Note the scaffolding code makes use of the new `receiverhelper` & `scraperhelper` functions added in the Core PRs referenced below.

**Depends on:** #1175, open-telemetry/opentelemetry-collector#1886, open-telemetry/opentelemetry-collector#1890
MovieStoreGuy pushed a commit to atlassian-forks/opentelemetry-collector that referenced this pull request Nov 11, 2021
…#1886)

* Add support for scheme in OTEL_EXPORTER_OTLP_ENDPOINT according to the spec

* Changes after review from pellared

* Changes after review from pellared - 2

* Changes after review from pellared - 3

Co-authored-by: Anthony Mirabella <a9@aneurysm9.com>
hughesjj pushed a commit to hughesjj/opentelemetry-collector that referenced this pull request Apr 27, 2023
* Update changelog

* Update java agent to 1.14.2
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
No open projects
Collector
  
Done
Development

Successfully merging this pull request may close these issues.

Create common scraping library
3 participants