Add receiverhelper functions for creating simple "scraper" metrics receiver #1875

james-bebbington · 2020-09-28T09:19:26Z

Added receiverhelper functions following a similar pattern to the other component helpers. Also added scraper functions to make it easy to construct a simple receiver that scrapes metrics at a configured frequency.

I've currently only only implemented basic functionality, and only implemented this for Metrics as I don't believe the "scrape" model applies to Traces or Logs.

Resolves #934

codecov · 2020-09-28T09:32:14Z

Codecov Report

Merging #1875 into master will increase coverage by 0.03%.
The diff coverage is 87.75%.

@@            Coverage Diff             @@
##           master    #1875      +/-   ##
==========================================
+ Coverage   91.23%   91.27%   +0.03%     
==========================================
  Files         272      274       +2     
  Lines       16263    16312      +49     
==========================================
+ Hits        14838    14889      +51     
+ Misses        998      994       -4     
- Partials      427      429       +2

Impacted Files	Coverage Δ
receiver/receiverhelper/receiver.go	`84.61% <84.61%> (ø)`
receiver/receiverhelper/scraper.go	`88.88% <88.88%> (ø)`
testutil/testutil.go	`91.52% <0.00%> (ø)`
...mplingprocessor/tailsamplingprocessor/processor.go	`75.00% <0.00%> (+0.96%)`	⬆️
...ssor/tailsamplingprocessor/idbatcher/id_batcher.go	`100.00% <0.00%> (+18.18%)`	⬆️

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 4a45ae9...79dc850. Read the comment docs.

james-bebbington · 2020-09-28T10:34:52Z

receiver/receiverhelper/receiver.go

+		return br.shutdown(ctx)
+	}
+	return nil
+}


Maybe we would want to add NewTraceReceiver, NewMetricsReceiver, & 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 not sure if it would be straightforward to factor out that code, or worth the effort. For now I have left this empty, and these helper functions are only intended to support the NewScraper function, i.e. for "pull" type receivers.

I think it's fine to make it specific to metrics. I can't think of a case where traces and logs are pull (though I could potentially see wanting to pull logs from a remote source but that seems quite out of scope to this.)

james-bebbington · 2020-09-28T12:23:06Z

receiver/receiverhelper/scraper.go

+	"go.opentelemetry.io/collector/consumer/pdata"
+)
+
+// Scraper provides a function to scrape metrics.


Not sure if its worth moving some of these Scraper types to component, configmodels, etc. This is not a first class component so that could be confusing. For now, I've gone with the straightforward approach of leaving these in receiverhelper.

jrcamp · 2020-09-29T04:12:11Z

/cc @pmcollins

jrcamp · 2020-09-29T04:18:31Z

Have you prototyped using this with hostmetrics receiver? Since it has several sub-scrapers will that pose an issue since NewScraper returns a receiver instance? I assume those could be decoupled.

jrcamp · 2020-09-29T04:21:19Z

receiver/receiverhelper/scraper.go

+}
+
+// SetCollectionInterval sets the scraper collection interval.
+func (ss *ScraperSettings) SetCollectionInterval(collectionInterval time.Duration) {


This feels awfully Java-y. 😛 Maybe can be improved as part of decoupling receiver and scraper as part of the comment above.

This is probably overkill, but replicates what the configmodels.Receiver interface does so that it can be used in a similar manner (e.g. see usage in NewScraper below)

james-bebbington · 2020-09-29T04:32:19Z

Have you prototyped using this with hostmetrics receiver? Since it has several sub-scrapers will that pose an issue since NewScraper returns a receiver instance? I assume those could be decoupled.

Tbh I wasn't really thinking about the Host Metrics receiver when I implemented this as that receiver is a bit of a special case with the internal scrapers. I have a prototype working with a skeleton for the Perf Counters receiver I will add later 😄.

But thinking about it now:

It should be relatively straightforward to change the host metrics receiver to use these ScraperHelper function for the root receiver itself. This would clean up a very small amount of code & standardise the observability reporting.
Changing the host metrics receiver's internal scrapers to use the Scraper interface here would probably be possible, but a bit more work. Not sure if that would have any strange consequences and/or cause confusion, given that the "internal" components would implement the Receiver interface. I would consider this pretty low priority unless you have plans for creating similar nested receivers?

jrcamp · 2020-09-29T16:02:04Z

Have you prototyped using this with hostmetrics receiver? Since it has several sub-scrapers will that pose an issue since NewScraper returns a receiver instance? I assume those could be decoupled.

Tbh I wasn't really thinking about the Host Metrics receiver when I implemented this as that receiver is a bit of a special case with the internal scrapers. I have a prototype working with a skeleton for the Perf Counters receiver I will add later 😄.

But thinking about it now:

It should be relatively straightforward to change the host metrics receiver to use these ScraperHelper function for the root receiver itself. This would clean up a very small amount of code & standardise the observability reporting.

Changing the host metrics receiver's internal scrapers to use the Scraper interface here would probably be possible, but a bit more work. Not sure if that would have any strange consequences and/or cause confusion, given that the "internal" components would implement the Receiver interface. I would consider this pretty low priority unless you have plans for creating similar nested receivers?

I doubt other receivers will do the sub-scrapers like hostmetricsreceiver but I could see a receiver wanting to scrape multiple endpoints in parallel and would create a scraper for each. I don't think it should complicate the design too much to decouple the receiver creation from scraper creation and allow the receiver creation to accept multiple scrapers.

james-bebbington · 2020-09-30T00:19:14Z

I doubt other receivers will do the sub-scrapers like hostmetricsreceiver but I could see a receiver wanting to scrape multiple endpoints in parallel and would create a scraper for each. I don't think it should complicate the design too much to decouple the receiver creation from scraper creation and allow the receiver creation to accept multiple scrapers.

Hmm yea maybe it makes sense to allow multiple Scrape functions to be configured. I'll think about this a bit more

james-bebbington · 2020-09-30T09:46:34Z

(I'm re-implementing this with the ability to configure multiple "scrape" targets, will upload this as a new PR when complete for comparison)

james-bebbington · 2020-10-01T07:08:53Z

See #1886 which is a much more flexible version of this that supports multiple scrape targets, etc.

It should theoretically not be too difficult to refactor the host metrics receiver to use those functions.

Leaving this PR open for now just for reference.

github-actions · 2020-10-09T03:25:00Z

This PR was marked stale due to lack of activity. It will be closed in 7 days.

…y#1875) * Ansible: Add proxy configuration for Windows downloads * Add variables to documentation, rename to start with win_ prefix * Update deployments/ansible/roles/collector/tasks/otel_win_install.yml Co-authored-by: Jeff Cheng <[email protected]> * Update deployments/ansible/roles/collector/tasks/win_fluentd_install.yml Co-authored-by: Jeff Cheng <[email protected]> * Update deployments/ansible/roles/collector/README.md Co-authored-by: Jeff Cheng <[email protected]> * code review * add CHANGELOG Co-authored-by: Jeff Cheng <[email protected]>

james-bebbington requested review from bogdandrutu, dmitryax, nilebox, owais, pjanotti and tigrannajaryan as code owners September 28, 2020 09:19

james-bebbington force-pushed the receiverhelper-scraper branch from 1da7c50 to a6816f0 Compare September 28, 2020 10:26

james-bebbington commented Sep 28, 2020

View reviewed changes

james-bebbington force-pushed the receiverhelper-scraper branch 2 times, most recently from 6c63eae to 1b1b253 Compare September 28, 2020 11:06

Add receiverhelper functions for creating a simple scraper receiver

79dc850

james-bebbington force-pushed the receiverhelper-scraper branch from 1b1b253 to 79dc850 Compare September 28, 2020 12:21

james-bebbington commented Sep 28, 2020

View reviewed changes

jrcamp reviewed Sep 29, 2020

View reviewed changes

tigrannajaryan assigned jrcamp Sep 29, 2020

james-bebbington marked this pull request as draft September 30, 2020 09:44

james-bebbington mentioned this pull request Oct 1, 2020

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

Merged

james-bebbington changed the title ~~Add receiverhelper functions for creating simple "scraper" receiver~~ Add receiverhelper functions for creating simple "scraper" metrics receiver Oct 1, 2020

github-actions bot added the Stale label Oct 9, 2020

james-bebbington closed this Oct 9, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add receiverhelper functions for creating simple "scraper" metrics receiver #1875

Add receiverhelper functions for creating simple "scraper" metrics receiver #1875

james-bebbington commented Sep 28, 2020 •

edited

Loading

codecov bot commented Sep 28, 2020 •

edited

Loading

james-bebbington Sep 28, 2020

jrcamp Sep 29, 2020

james-bebbington Sep 28, 2020

jrcamp commented Sep 29, 2020

jrcamp commented Sep 29, 2020

jrcamp Sep 29, 2020

james-bebbington Sep 29, 2020

james-bebbington commented Sep 29, 2020

jrcamp commented Sep 29, 2020

james-bebbington commented Sep 30, 2020

james-bebbington commented Sep 30, 2020

james-bebbington commented Oct 1, 2020 •

edited

Loading

github-actions bot commented Oct 9, 2020

Add receiverhelper functions for creating simple "scraper" metrics receiver #1875

Add receiverhelper functions for creating simple "scraper" metrics receiver #1875

Conversation

james-bebbington commented Sep 28, 2020 • edited Loading

codecov bot commented Sep 28, 2020 • edited Loading

Codecov Report

james-bebbington Sep 28, 2020

Choose a reason for hiding this comment

jrcamp Sep 29, 2020

Choose a reason for hiding this comment

james-bebbington Sep 28, 2020

Choose a reason for hiding this comment

jrcamp commented Sep 29, 2020

jrcamp commented Sep 29, 2020

jrcamp Sep 29, 2020

Choose a reason for hiding this comment

james-bebbington Sep 29, 2020

Choose a reason for hiding this comment

james-bebbington commented Sep 29, 2020

jrcamp commented Sep 29, 2020

james-bebbington commented Sep 30, 2020

james-bebbington commented Sep 30, 2020

james-bebbington commented Oct 1, 2020 • edited Loading

github-actions bot commented Oct 9, 2020

james-bebbington commented Sep 28, 2020 •

edited

Loading

codecov bot commented Sep 28, 2020 •

edited

Loading

james-bebbington commented Oct 1, 2020 •

edited

Loading