-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
[metrics] standardize/simplify export pipeline setup #395
[metrics] standardize/simplify export pipeline setup #395
Conversation
How about two methods, one
|
* Creates NewRawExporter, NewExportPipeline, InstallNewPipeline methods. * Uses Options rather than Config throughout for options.
I've done as you asked on behalf of the original pull submitter, PTAL. |
// Config supports common options that apply to statsd exporters. | ||
Config struct { | ||
// Options supports common options that apply to statsd exporters. | ||
Options struct { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this is an unrelated remark.
I thought we had moved toward the practice of using "Config" as the structure name, and Option as a functional argument (func(*Config)
), and Options as a []Option
. See api/trace StartConfig
and StartOption
, for example.
That's why I prefer this struct be called Config.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
See #369
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The goal of this was to standardize the patterns for all Exporter options/config. Prometheus and stdout were using options, statsd was using config...
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I like the new simplified exporter setup. I would prefer to keep a Config
struct.
Alright. Let's merge this as is (to make the one case that's Config go to Options for consistency), and then I'll do a quick followup to rename all the Options systematically to Config. |
@paivagustavo want to take a look before we merge? |
This PR implements a method for simplified export pipeline setup for stdout exporter.
Resolves #343
Co-authored-by: Liz Fong-Jones [email protected]