opentelemetry-collector
opentelemetry-collector copied to clipboard
`scraperhelper.NewScraper` is awkward to use
While working on #9208 I realized that the current way scraperhelper.NewScraper works is a bit awkward. The function takes a name string and then builds an ID with it:
https://github.com/open-telemetry/opentelemetry-collector/blob/f5a7315cf88e10c0bce0166b35d9227727deaa61/receiver/scraperhelper/scraper.go#L64
It is very common among contrib scrapers to name the scraper with the same name as the component itself, and so we end up building a component.Type, converting it to string to pass to NewScraper and then building it again. I propose we change the first argument to take a component.Type instead.
I think Scraper interface needs a bit more "love" than that. Will give a try to send some PRs/ideas soon
I agree that it's a bit awkward to use and encountered similar, this is my "triage" attempt. Feel free to rip it apart or add to it if useful
Of note is that we may want to start limiting string length of component.Type as well
So, we've change the underlying datatype from String to Struct in
9472, but there's still a few points of "awkwardness" remaining. From my perspective:
- We don't allow "empty"
Types to exist via the regex, but the empty value is still in use. This means not every constructed type will pass the validation logic. - ScraperHelper constructs a
component.IDbut never uses theNamefield. This is awkward because the constructor itself takes in a string, and I've seen downstream consumers pass incomponent.ID.String()as the value for this. This leads to unexpected behavior given, if a Name is set on the component.ID passed intoNewScraper, the.String()value will not pass the regex. Ex if a component.ID istype=foo,name=bar, the.String()method will befoo/barand fail the regex.
the empty value is still in use
Where do we use the empty value?