search

caddyserver / website

The Caddy website
156 stars 153 forks source link

Fix prometheus.yml metrics config #387

Closed dylanschultzie closed 7 months ago

dylanschultzie commented 7 months ago

Prometheus metrics require the /metrics path to work correctly.

hairyhenderson commented 7 months ago

@dylanschultzie that's the default, so it doesn't need to be specified.

IMO this shouldn't be added as it isn't necessary and complicates matters. If someone needs to know how to configure Prometheus, they should read their docs (as referenced above).

dylanschultzie commented 7 months ago

@hairyhenderson I don't agree with excluding it. Using Prometheus v2.48.0, I had to specify /metrics for them to be consumed.

That being the case, I'd argue omitting it being more detrimental than not. In addition, if caddy is already providing a barebones scrape config, why not ensure it's correct?

francislavoie commented 7 months ago

Re-stating the defaults doesn't make sense.

What's your evidence that it doesn't work without it?

dylanschultzie commented 7 months ago

Listen, I'm just trying to help out.

Potentially forcing users to dig through community forums, or the Prometheus docs "(as referenced above)" to find pertinent information does nobody any good when it can be solved simply in a single line.

francislavoie commented 7 months ago

You haven't shown evidence that it actually solves a problem. We don't make changes to the docs without evidence.