Add flags to consul connect envoy for metrics merging. #9768
Conversation
github-actions
bot
added
the
theme/cli
ndhanushkodi
requested review from
a team,
lkysow,
ishustava and
thisisnotashwin
and removed request for
a team and
lkysow
There was a problem hiding this comment.
This looks great. You will need to go fmt -s based on what i saw on the failing CI task.
Excellent work @ndhanushkodi !!
ndhanushkodi
force-pushed
the
metrics-merging
branch
from
2dba2ac to
a0e20d1
Compare
ndhanushkodi
force-pushed
the
metrics-merging
branch
from
a0e20d1 to
3925548
Compare
| @@ -168,6 +199,7 @@ func TestGenerateConfig(t *testing.T) { | |||
| AdminBindPort: "19000", | |||
| LocalAgentClusterName: xds.LocalAgentClusterName, | |||
| Token: "c9a52720-bf6c-4aa6-b8bc-66881a5ade95", | |||
| PrometheusScrapePath: "/metrics", | |||
There was a problem hiding this comment.
This is being added to "WantArgs" everywhere since it is a default value, so even when we don't pass the flag to consul connect envoy, we want it to be set to "/metrics"
ndhanushkodi
force-pushed
the
metrics-merging
branch
from
3925548 to
7179705
Compare
| @@ -213,19 +213,23 @@ func (c *BootstrapConfig) ConfigureArgs(args *BootstrapTplArgs, omitDeprecatedTa | |||
| } | |||
| // Setup prometheus if needed. This MUST happen after the Static*JSON is set above | |||
| if c.PrometheusBindAddr != "" { | |||
| if err := c.generateListenerConfig(args, c.PrometheusBindAddr, "envoy_prometheus_metrics", "path", "/metrics", "/stats/prometheus"); err != nil { | |||
| // if args.PrometheusBackendPort is not an empty string, this will tell | |||
There was a problem hiding this comment.
I feel like this comment belongs somewhere in generateListenerConfig instead
There was a problem hiding this comment.
Sounds good, I removed this and modified the comment in generateListenerConfig
| @@ -165,6 +167,19 @@ func (c *cmd) init() { | |||
| "consul.destination.[service|dc|...]. The old tags were preserved for backward compatibility,"+ | |||
| "but can be disabled with this flag.") | |||
|
|
|||
| c.flags.StringVar(&c.prometheusBackendPort, "prometheus-backend-port", "", | |||
There was a problem hiding this comment.
I believe there should also be website updates to the CLI section and possibly something else (?).
There was a problem hiding this comment.
I added it to that CLI section and looked around for any other areas these flags are mentioned and didn't see any. There will be a future docs PR for the overall metrics feature in Consul-k8s that should hopefully cover anything else.
Allows setting -prometheus-backend-port to configure the cluster envoy_prometheus_bind_addr points to. Allows setting -prometheus-scrape-path to configure which path envoy_prometheus_bind_addr exposes metrics on. -prometheus-backend-port is used by the consul-k8s metrics merging feature, to configure envoy_prometheus_bind_addr to point to the merged metrics endpoint that combines Envoy and service metrics so that one set of annotations on a Pod can scrape metrics from the service and it's Envoy sidecar. -prometheus-scrape-path is used to allow configurability of the path where prometheus metrics are exposed on envoy_prometheus_bind_addr.
ndhanushkodi
force-pushed
the
metrics-merging
branch
from
7179705 to
afed670
Compare
|
🍒 If backport labels were added before merging, cherry-picking will start automatically. To retroactively trigger a backport after merging, add backport labels and re-run https://circleci.com/gh/hashicorp/consul/333665. |
|
Could this be backported to 1.9.x, please? :-) |
Changes Proposed:
Allows setting -prometheus-backend-port to configure the cluster
envoy_prometheus_bind_addr points to.
Allows setting -prometheus-scrape-path to configure which path
envoy_prometheus_bind_addr exposes metrics on.
-prometheus-backend-port is used by the consul-k8s metrics merging feature, to
configure envoy_prometheus_bind_addr to point to the merged metrics
endpoint that combines Envoy and service metrics so that one set of
annotations on a Pod can scrape metrics from the service and it's Envoy
sidecar.
-prometheus-scrape-path is used to allow configurability of the path
where prometheus metrics are exposed on envoy_prometheus_bind_addr. This value is also configurable via consul-k8s/consul-helm so for that it needs to be configurable here.