Polish config #48
Polish config #48
Conversation
codebien
force-pushed
the
simplify-config
branch
from
89ade43 to
b6e931b
Compare
README.md
Outdated
| K6_PROMETHEUS_REMOTE_URL=https://localhost:9090/api/v1/write K6_PROMETHEUS_INSECURE_SKIP_TLS_VERIFY=false K6_CA_CERT_FILE=example/tls.crt K6_PROMETHEUS_USER=foo K6_PROMETHEUS_PASSWORD=bar ./k6 run script.js -o output-prometheus-remote | ||
| K6_PROMETHEUS_REMOTE_URL=https://localhost:9090/api/v1/write \ | ||
| K6_PROMETHEUS_INSECURE_SKIP_TLS_VERIFY=false \ | ||
| K6_PROMETHEUS_CA_CERT_FILE=example/tls.crt \ |
There was a problem hiding this comment.
I forgot to ask before. Do we need this? The user could use the Go env vars SSL_CERT_FILE and SSL_CERT_DIR for passing the location or the dir of the root store.
Note: the comment is related to K6_PROMETHEUS_CA_CERT_FILE
There was a problem hiding this comment.
It's a good question 🤔
Honestly, I think that we can stick with the default golang's environment variable if that works precisely the same way, but I guess it's still important to mention that because maybe not all of our users are golang devs. But I'm not so strict in that opinion.
There was a problem hiding this comment.
Apologies for the delay! This message totally got lost in my Github subscriptions.
This env var appeared in order to connect to Prometheus set up with auth: if there is another way to connect to such an instance without using env var, it should be checked.
Btw, Mimir has something similar in its setup to Prometheus: https://grafana.com/docs/mimir/latest/operators-guide/secure/securing-communications-with-tls/#configure-tls-certificates-in-grafana-mimir
Additionally, IIRC, I had a reasoning that one might want to keep specific certificates separately, and external env vars like SSL_CERT_FILE are not explicit enough and / or may not be available. This reasoning is debatable and a matter of product design, of course.
Hope that helps!
There was a problem hiding this comment.
Thanks, @yorugac. If I'm understanding well we don't have a specific requirement for it; then it sounds reasonable to me to keep the k6 and the output config consistent and with as less as possible options for reducing the risk of breaking change in the future. I would be happy to bring back this if we will receive an explicit request for improving the UX on this front.
if there is another way to connect to such an instance without using env var, it should be checked.
There is and it's even better than env var because it is just implicit and it is the system trusted certs store. We are doing the same with the k6 core. grafana/k6#218 (comment)
Removed useless Options where k6 native alternatives are available. Renamed some options to have them consistent with other k6 outputs.
It helps to see what is the configured remote url and catch eventual error in the set config.
codebien
force-pushed
the
simplify-config
branch
from
b6e931b to
1db2193
Compare
This PR includes changes for the config and the URL printing in the description.
Renamed for consistency the following env var options:
K6_PROMETHEUS_FLUSH_PERIOD=>K6_PROMETHEUS_PUSH_INTERVAL(it aligns to the same name used by timescaledb, influxdb and statsd outputs).K6_PROMETHEUS_USER=>K6_PROMETHEUS_USERNAME(it aligns to the same name used by influxdb output).Removed completely for aligning to what we are doing with other outputs the following options:
KeepTags,KeepNameTag,KeepUrlTag, as explained in #47, tags should be controlled bysystemTagsoption or by the solution explained in #5 (TLDR; grafana/k6#1321) when we will have implemented it.Removed
Mappingoption because at the moment the output does not support different kinds of mappings so it does not make sense to export an option.The documentation must be aligned when we will have merged this PR.