Enable Prometheus scraping by default

[kimtore]

The annotation prometheus.io/scrape is set to false by default. Should we just enable this for everyone by default? /cc @jhrv @terjesannum

[Kyrremann]

Jeg stemmer nei, da det er kjekt å kunne teste ting uten at man må sette den til false for å unngå unødvendig scraping.

[jhrv]

Default-verdi bør reflektere hva vi mener default oppførsel bør være. At det er litt kjekt når man tester ting mener jeg ikke veier opp for at dette må settes eksplisitt for nesten alle apper.

[terjesannum]

Jeg synes ikke det bør være default på

[jhrv]

Jeg synes ikke det bør være default på

Hvorfor ikke?

[terjesannum]

Alle apper har ikke noe prometheus endepunkt, da synes jeg det er ryddigere at de som har det skrur på scraping

[jhrv]

Trodde ca alle apper hadde dette?

Kjøper at det er lettere å få et cluster som er riktigere konfigurert da det sannsynligvis er få som fanger opp at det skjer unødvendig scraping. Men hva er ulempen motsatt? Blir det mye støy om man har satt opp scraping på noe som ikke lar seg scrape?

Jeg syns vi uansett bør fjerne spec.prometheus.enabled, slik at det holder at man angir hvilken path som skal scrapes (spec.prometheus.path). Hvis path angis, setter vi automatisk prometheus.io/scrape annotasjonen til true.

[terjesannum]

Det er ikke noe stort teknisk problem, men derfor vil man heller ikke få folk til å skru det av når man ikke har metrikker. Så jeg synes det er ryddigere at man har et bevisst forhold til dette og enabler det når man har metrikker man vil eksponere. Path er heller ikke standard og svært mange kjører med noe annet enn /metrics, men jeg er enig i at det burde være nok å oppgi denne pathen for å enable scraping.

[kimtore]

It sounds like the consensus is to leave the default behavior as it is, but change the semantics so that:

• .prometheus.enabled is removed
• .prometheus.path will not have a default anymore
• setting .prometheus.path explicitly will enable scraping, a blank value will disable scraping

Executing this change will result in scraping being enabled for existing applications that specify .prometheus.enabled = false, because the metrics path is set by default. The next application deployment will disable scraping again.

/cc @jhrv @terjesannum @Kyrremann

[Kyrremann]

Jeg syns det er dumt at vi skal endre til at folk må skrive inn path, gjør hele opplegget mer komplisert. Jeg tenker heller at man bruker enabled, så får du default oppsett, og de som vil kan bare bruke path, og trenger da ikke sette enabled.

[kimtore]

That sounds confusing. You turn the feature on either setting path or enabled? If it's not auto-enabled by setting path, it should be set explicitly. This makes it easier to document and understand, and makes this issue a no-op.

[terjesannum]

I checked one of the clusters, and a lot of the pods does not use /metrics as metrics path. So I think that specifying a metrics path is the best option to enable scraping.

• setting .prometheus.path explicitly will enable scraping, a blank value will disable scraping

k get pod -o 'jsonpath={..metadata.annotations.prometheus\.io/path}' | tr ' ' '\n' | sort | uniq -c | sort -n
...
      3 /actuator/metrics
      6 /api/actuator/prometheus
     14 internal/metrics
     18 /
     20 /internal/metrics
     26 internal/prometheus
     27 actuator/prometheus
     40 /actuator/prometheus
     73 /internal/prometheus
     81 /prometheus
    264 /metrics

[Kyrremann]

Vi har valgt å ikke merge denne inn, da vi heller ønsker å vente til flere features som ikke er bakoverkompatible skal rulles ut.
Dette for å minske jobben til utviklere, og at dette er mer kjekt å ha enn nødvendig.