prometheus / mysqld_exporter

Exporter for MySQL server metrics
http://prometheus.io/
Apache License 2.0
2.16k stars 758 forks source link

Command-line flag help ordering inconsistent #895

Open dswarbrick opened 4 days ago

dswarbrick commented 4 days ago

Host operating system: output of uname -a

n/a / irrelevant.

mysqld_exporter version: output of mysqld_exporter --version

any / all.

MySQL server version

n/a

mysqld_exporter command line flags

--help or --help-man

What did you do that produced an error?

n/a

What did you expect to see?

Consistent ordering of command-line flags.

What did you see instead?

Inconsistent ordering of command-line flags.

This is really a nit / trivial issue. When running mysqld_exporter --help, the ordering of the collector flags is inconsistent across invocations. Likewise the manpage produced by --help-man suffers the same problem. This has the knock-on effect that distros which use the --help-man feature to bundle a manpage within a package result in unreproducible builds.

Whilst this is probably of little consequence to most users (and certainly of no interest to non-Debian/Ubuntu users), even vanilla upstream releases will have the annoying trait of producing inconsistent output with the --help flag. Users may be confused when they cannot find a flag that they were sure was in a particular place in the list previously.

This phenomenon seems to be due to how the flags are initialized from a Go map (which does not ensure consistent order):

    for scraper, enabledByDefault := range scrapers {
        defaultOn := "false"
        if enabledByDefault {
            defaultOn = "true"
        }

        f := kingpin.Flag(
            "collect."+scraper.Name(),
            scraper.Help(),
        ).Default(defaultOn).Bool()

        scraperFlags[scraper] = f
    }

scrapers is a map of collectors defined as:

// scrapers lists all possible collection methods and if they should be enabled by default.
var scrapers = map[collector.Scraper]bool{
    collector.ScrapeGlobalStatus{}:                        true,
    collector.ScrapeGlobalVariables{}:                     true,
    collector.ScrapeSlaveStatus{}:                         true,
    collector.ScrapeProcesslist{}:                         false,
...
}

Any time that the loop ranges over scrapers, the order is non-deterministic, meaning that the flags are not always added in the same order.

Perhaps a different mechanism of collectors registering themselves with the exporter, similar to how node_exporter does this, would be a better option.

dswarbrick commented 4 days ago

Sample diff of two consecutive invocations of mysqld_exporter --help

--- help1       2024-11-21 09:23:29.947194481 +0100
+++ help2       2024-11-21 09:24:00.378403127 +0100
@@ -72,6 +72,11 @@
       --web.config.file=""       Path to configuration file that can
                                  enable TLS or authentication. See:
                                  https://github.com/prometheus/exporter-toolkit/blob/master/docs/web-configuration.md
+      --[no-]collect.info_schema.innodb_metrics  
+                                 Collect metrics from
+                                 information_schema.innodb_metrics
+      --[no-]collect.global_status  
+                                 Collect from SHOW GLOBAL STATUS
       --[no-]collect.global_variables  
                                  Collect from SHOW GLOBAL VARIABLES
       --[no-]collect.slave_status  
@@ -85,11 +90,12 @@
       --[no-]collect.info_schema.innodb_tablespaces  
                                  Collect metrics from
                                  information_schema.innodb_sys_tablespaces
-      --[no-]collect.info_schema.innodb_metrics  
+      --[no-]collect.perf_schema.eventswaits  
                                  Collect metrics from
-                                 information_schema.innodb_metrics
-      --[no-]collect.global_status  
-                                 Collect from SHOW GLOBAL STATUS
+                                 performance_schema.events_waits_summary_global_by_event_name
+      --[no-]collect.auto_increment.columns  
+                                 Collect auto_increment columns and max values
+                                 from information_schema
       --[no-]collect.binlog_size  
                                  Collect the current size of all registered
                                  binlog files
@@ -108,12 +114,12 @@
       --[no-]collect.perf_schema.eventsstatementssum  
                                  Collect metrics of grand sums from
                                  performance_schema.events_statements_summary_by_digest
-      --[no-]collect.perf_schema.eventswaits  
+      --[no-]collect.info_schema.userstats  
+                                 If running with userstat=1, set to true to
+                                 collect user statistics
+      --[no-]collect.perf_schema.file_events  
                                  Collect metrics from
-                                 performance_schema.events_waits_summary_global_by_event_name
-      --[no-]collect.auto_increment.columns  
-                                 Collect auto_increment columns and max values
-                                 from information_schema
+                                 performance_schema.file_summary_by_event_name
       --[no-]collect.perf_schema.file_instances  
                                  Collect metrics from
                                  performance_schema.file_summary_by_instance
@@ -134,12 +140,11 @@
                                  from sys.x$user_summary. See
                                  https://dev.mysql.com/doc/refman/5.7/en/sys-user-summary.html
                                  for details
-      --[no-]collect.info_schema.userstats  
+      --[no-]collect.engine_innodb_status  
+                                 Collect from SHOW ENGINE INNODB STATUS
+      --[no-]collect.info_schema.clientstats  
                                  If running with userstat=1, set to true to
-                                 collect user statistics
-      --[no-]collect.perf_schema.file_events  
-                                 Collect metrics from
-                                 performance_schema.file_summary_by_event_name
+                                 collect client statistics
       --[no-]collect.info_schema.tablestats  
                                  If running with userstat=1, set to true to
                                  collect table statistics
@@ -157,17 +162,12 @@
                                  query_response_time_stats is ON.
       --[no-]collect.engine_tokudb_status  
                                  Collect from SHOW ENGINE TOKUDB STATUS
-      --[no-]collect.engine_innodb_status  
-                                 Collect from SHOW ENGINE INNODB STATUS
-      --[no-]collect.info_schema.clientstats  
-                                 If running with userstat=1, set to true to
-                                 collect client statistics
+      --[no-]collect.heartbeat   Collect from heartbeat
       --[no-]collect.slave_hosts  
                                  Scrape information from 'SHOW SLAVE HOSTS'
       --[no-]collect.info_schema.replica_host  
                                  Collect metrics from
                                  information_schema.replica_host_status
-      --[no-]collect.heartbeat   Collect from heartbeat
       --log.level=info           Only log messages with the given severity or
                                  above. One of: [debug, info, warn, error]
       --log.format=logfmt        Output format of log messages. One of: [logfmt,