-
Notifications
You must be signed in to change notification settings - Fork 65
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
Rename from prometheus-remote to prometheus-rw #79
Conversation
Renamed the field name for consistency with the env var.
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.
LGTM, besides the prefix question, and that I'm not sure about the envconfig removal, but @na-- would know better.
Also, could we add some E2E tests to this repo that actually test against Prometheus? I can open an issue if you agree.
@@ -6,7 +6,7 @@ import ( | |||
) | |||
|
|||
func init() { | |||
output.RegisterExtension("output-prometheus-remote", func(p output.Params) (output.Output, error) { | |||
output.RegisterExtension("xk6-prometheus-rw", func(p output.Params) (output.Output, error) { |
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.
We don't want to use experimental-*
here?
And does xk6-*
make sense, considering this extension will be imported directly by k6, without using xk6?
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.
We can't use experimental-
, since that will already be built into k6 and cause an error.
In other extensions, we kept the xk6 module registration even after we bundled them into the core as experimental
because some users probably already had a setup that used them as xk6 extensions. In this case, since we are changing the extension name and all of its parameters, it probably males less sense, so I am fine either way - leave it as it was, change it, doesn't really matter all that much 😅
Though some xk6 registration should remain, since this is still going to be an xk6 extension in its own repo, even after it is bundled in experimental
. We will make changes here and only occasionally merge them back in k6, so some user might want to use the latest and greatest version of the extension through xk6.
It renames the registered name from
"output-prometheus-remote
toxk6-prometheus-rw
and it makes the env vars consistent with the new name renaming the prefix toK6_PROMETHEUS_RW_*