stats: expose Prometheus metrics as /metrics

[ccmtaylor]

Title: expose Prometheus metrics as /metrics

Description:

By convention, Prometheus expects metrics to be exposed via the endpoint /metrics. While it is possible to override the scraping endpoint vie explicit configuration, this may complicate deployments for users. The implementation in #2026 exposes the metrics as /stats?format=prometheus.

@brancz brought this up in #1947. @mattklein123 and @lita agreed (#1947 (comment) / #1947 (comment)), but #2026 went with /stats?format=prometheus.

[mattklein123]

As discussed in the other issue, we decided to stick with /stats because we already have a /stats endpoint and adding an additional /metrics endpoint would be confusing. We can't change /stats for back compat reasons and we are also not a Prometheus only operation. I think a compromise here might be to allow the stats admin endpoint to be configurable as part of the admin configuration. If someone wants to take on that work SGTM.

[ccmtaylor]

thanks for the feedback, @mattklein123! I didn't see that decision in the comments on #1947 or #2026, so I suspected the path was an oversight.

[brancz]

As /metrics is only a convention of Prometheus, but we want native Prometheus support by Envoy, how about we just add the /metrics endpoint, and only expose the metrics in Prometheus format here. I don't see an inconsistency then. As a side note, Envoy would be the only project I know of that doesn't expose this endpoint on /metrics, so I can see it to be highly confusing to Prometheus users.

[shahsaifi]

BTW, anyone tried using /stats?format=prometheus in Prometheus metrics_path config. I see an error while doing that text format parsing error in line 1: expected float as value, got ".cats.bind_errors:" because it ends-up with /stats%3Fformat=prometheus.

[shahsaifi]

Tried using:

metrics_path: /stats
params:
        format: ['prometheus']

which ends-up with :

"envoy_listener_http_downstream_rq_xx", or TYPE reported after samples

Possibly similar to - deadtrickster/prometheus_rabbitmq_exporter#19

[brancz]

@shahsaifi this might just be a prometheus configuration thing, maybe just ask on IRC or the Prometheus users mailing list, however, I haven't tried the Prometheus support myself yet.

[krootee]

Maybe this helps you, but following Prometheus config works fine for me (running Prometheus in Docker container and Envoy is available as 'envoy' in same Docker network):
`- job_name: "envoy"

scrape_interval: 1s
metrics_path: /stats
params:
  format: ['prometheus']
static_configs:
  - targets: ['envoy:9901']

[shahsaifi]

It worked with prometheus-2.0. I have been trying with 1.7 version. Thanks!

[marsty]

@krootee use this config ,The prometheus error no token found, why?

[objectiser]

May not have seen all of the discussions around this - but just wondering if there has been any consideration of what should happen if envoy proxy does use /metrics endpoint, but the proxied service also exposes its own prometheus metrics on the same endpoint?

Will envoy forward the request to the service still, but then add its own metrics to the response?

[ggreenway]

@objectiser Only the admin endpoint exposes Envoy's stats on /metrics, and the admin endpoint never proxies, so it shouldn't be an issue.

[alvaroaleman]

For Prometheus url parameters are not very nice, because if you use its service discovery mechanism you must know the name of the parameter in advance. This is why e.G. the Prometheus samples for the Kuberntetes service discovery don't even assume they are used.

Would it alternatively be possible to have an additonal route /stats/prometheus that returns stats in Prometheus format?

[ghost]

Requiring the param for Prometheus scraping may be incompatible with the scrape via Kubernetes annotation pattern which is supported by the Prometheus Helm chart in the kubernetes/charts repo.

https://github.com/prometheus/prometheus/blob/master/documentation/examples/prometheus-kubernetes.yml#L245

+1 /stats/prometheus or some other path-only method.

[mattklein123]

/stats/prometheus SGTM if someone wants to make that happen.

[brancz]

At that point, why not go with the Prometheus standard /metrics?

[dio]

Not sure if it is nicer to follow up @mattklein123's comment here, by having e.g.

admin:
  override_endpoint_paths:
    "/metrics":
      path: "/stats"
      query: 
        format: prometheus
        foo: bar

The AdminImpl::runCallback then converts the given path to value.path?query.key=query.val&... (/metrics -> /stats?format=prometheus&foo=bar) first.

[mattklein123]

At that point, why not go with the Prometheus standard /metrics?

Because IMO it's confusing to have /metrics and /stats per previous discussions. /stats/prometheus seems a good compromise.

Not sure if it is nicer to follow up @mattklein123's comment here, by having e.g.

@dio that's a good idea and I think that would work, but my preference is to avoid complexity unless it's really need. Additionally, in the future we are going to promote the admin listener into more of a full listener. It's possible at that point it could have proper routes include prefix_rewrite if needed.

[perlun]

What we've done to workaround this: set up a separate listener where we use a route_config like below to work around this. Posting this since it could help others:

route_config:
  name: local_route
  virtual_hosts:
    {
      name: local_service,
      domains: ["*"],
      routes: [
        {
          match: { prefix: "/metrics" },

          route:
            {
              cluster: envoy-admin,
              prefix_rewrite: "/stats/prometheus"
            }
        }
      ]
    }

...with a cluster config like this:

    - name: envoy-admin
      connect_timeout: 0.25s
      type: STATIC

      load_assignment: {
        cluster_name: envoy-admin,
        endpoints: {
          lb_endpoints: [
            {
              endpoint: {
                address: {
                  socket_address: { address: 127.0.0.1, port_value: 9901 },
                }
              }
            }
          ]
        }
      }

This works quite well, and means that no special configuration is needed on the Prometheus side.

---

@mattklein123 While I understand and respect your reasoning from an Envoy perspective, I would highly appreciate if this was reconsidered. We currently use 13 different kinds of Prometheus endpoints in our current prometheus.yaml file (mysqld_exporter, elasticsearch-exporter, node-exporter, etc). All of them expose their metrics under /metrics (well, TBH, some even ignore the path entirely... which is possible for a dedicated out-of-process Prometheus exporter, since its HTTP listener is solely used to expose Prometheus metrics). All of them, except for Envoy.

While the "Writing Exporters" document from the Prometheus web site doesn't explicitly mandate that the scrape endpoint should be named /metrics, you can read it between the lines in statements like this:

Instrumenting your exporter itself via direct instrumentation is fine, e.g. total bytes transferred or calls performed by the exporter across all scrapes. For exporters such as the blackbox exporter and SMNP exporter, which aren’t tied to a single target, these should only be exposed on a vanilla /metrics call, not on a scrape of a particular target.

[...]

It’s nicer for users if visiting http://yourexporter/ has a simple HTML page with the name of the exporter, and a link to the /metrics page.

(This is clearly speaking about out-of-process exporters, which "own" their entire HTTP namespace.)

Cheers & have a nice weekend. 👍

[obscurerichard]

@perlun thank you for the configuration snippets to expose Prometheus metrics at /metrics. I have integrated those into a fully working example in the Helm chart ConfigMap for Envoy for the book I'm writing for Packt Publishing, Docker for Developers.