albertodonato / query-exporter

Export Prometheus metrics from SQL queries
GNU General Public License v3.0
436 stars 101 forks source link

Exclude `database` label at exporter level #173

Open robbat2 opened 10 months ago

robbat2 commented 10 months ago

Is your feature request related to a problem? Please describe. A clear and concise description of what the problem is. Ex. I'm always frustrated when [...] I want to avoid exporting a meaningless label for database.

Present state

mymetric{database="db",label1="value1"} NNNN

Describe the solution you'd like Require users to explicitly declare the database label in the config.

Desired state:

mymetric{label1="value1"} NNNN

Describe alternatives you've considered label drop in Prometheus/Mimir/VictoriaMetrics is possible, but has to be configured on every scraper.

Additional context The present design implicitly breaks anybody who ever wanted their own label of database.

albertodonato commented 8 months ago

Can't you either name the database as desired in the config (so that it shows as label value), or use a different label name to aggregate as desired?

robbat2 commented 8 months ago

In the README sample configuration and output, there's this example:

queries_total{app="app1",database="db1",query="query1",region="us1",status="success"} 50.0

The database label is NOT declared in the labels section of the config, and there's no direct way in the config to remove it.

For users that want it to persist, I'd tell them to explicitly configure it in labels, e.g.:

databases:
  db1:
    dsn: sqlite://
    connect-sql:
      - PRAGMA application_id = 123
      - PRAGMA auto_vacuum = 1
    labels:
      region: us1
      app: app1
      database: db1 # this line is new

If you were worried about breaking existing configs, maybe accept the label value being empty as a sign to remove it:

databases:
  db1:
    dsn: sqlite://
    connect-sql:
      - PRAGMA application_id = 123
      - PRAGMA auto_vacuum = 1
    labels:
      region: us1
      app: app1
      database: # new line, empty value => remove the label in the output metric

Expected result:

queries_total{app="app1",query="query1",region="us1",status="success"} 50.0
lchopfpt commented 6 months ago

I do not consider the db label useless, but having the ability to not include it could be nice for those that don't want it. Though I can't think of many exporters that allow for finagling which labels are included/excluded.