Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

promhttp.HandlerOpts has nil ErrorLog which prevents errors from being logged #1886

Closed
zmedico opened this issue Nov 14, 2020 · 1 comment · Fixed by #1887
Closed

promhttp.HandlerOpts has nil ErrorLog which prevents errors from being logged #1886

zmedico opened this issue Nov 14, 2020 · 1 comment · Fixed by #1887

Comments

@zmedico
Copy link

zmedico commented Nov 14, 2020

Host operating system: output of uname -a

Linux hostname 5.4.65 #1 SMP Tue Sep 15 21:22:36 UTC 2020 x86_64 AMD EPYC 7551P 32-Core Processor AuthenticAMD GNU/Linux

node_exporter version: output of node_exporter --version

# node_exporter --version
node_exporter, version 1.0.1 (branch: HEAD, revision: 3715be6ae899f2a9b9dbfd9c39f3e09a7bd4559f)
  build user:       root@1f76dbbcfa55
  build date:       20200616-12:44:12
  go version:       go1.14.4

node_exporter command line flags

node_exporter -- --collector.textfile.directory=/var/lib/node_exporter/ --collector.bonding --collector.buddyinfo --collector.ntp --no-collector.infiniband --no-collector.nfs --no-collector.nfsd --no-collector.wifi --no-collector.zfs --web.listen-address=127.0.0.1:9100

Are you running node_exporter in Docker?

No.

What did you do that produced an error?

I noticed that errors were not being logged when troubleshooting #1885, and the test case is the same. Write two different prom files to the textfile collector directory, containing metrics with the same names but different labels. For example:

$ cat /var/lib/node_exporter/app-instance-1.prom
# TYPE app_metric_name counter
app_metric_name{instance="1"} 0
$ cat /var/lib/node_exporter/app-instance-2.prom
# TYPE app_metric_name counter
app_metric_name{instance="2"} 0

What did you expect to see?

Expected to see some kind of error message in the log, since metrics from app-instance-2.prom were missing from the exported metrics as reported in #1600 and #1885.

What did you see instead?

The error was not logged, but I was able to log it with the following patch:
* [from Gatherer #2] collected metric app_metric_name label:<name:"instance" value:"2" > counter:<value:0 > has help "Metric read from /var/lib/node_exporter/app-instance-2.prom" but should have "Metric read from /var/lib/node_exporter/app-instance-1.prom"

--- node_exporter-1.0.1/node_exporter.go
+++ node_exporter-1.0.1/node_exporter.go
@@ -31,6 +31,7 @@
 	"github.com/prometheus/node_exporter/collector"
 	"github.com/prometheus/node_exporter/https"
 	kingpin "gopkg.in/alecthomas/kingpin.v2"
+	stdlog "log"
 )
 
 // handler wraps an unfiltered http.Handler but uses a filtered handler,
@@ -121,6 +122,7 @@
 	handler := promhttp.HandlerFor(
 		prometheus.Gatherers{h.exporterMetricsRegistry, r},
 		promhttp.HandlerOpts{
+			ErrorLog:            stdlog.New(os.Stderr, "", stdlog.LstdFlags),
 			ErrorHandling:       promhttp.ContinueOnError,
 			MaxRequestsInFlight: h.maxRequests,
 			Registry:            h.exporterMetricsRegistry,

The lack of logging explains why #1600 hasn't been resolved yet.

@SuperQ
Copy link
Member

SuperQ commented Nov 14, 2020

We probably don't want to just send it to stderr, as we use the promlog package.

SuperQ added a commit that referenced this issue Nov 14, 2020
Fix the error logging of the promhttp handler by connecting it to the
promlog setup.

Fixes: #1886

Signed-off-by: Ben Kochie <[email protected]>
SuperQ added a commit to prometheus/client_golang that referenced this issue Nov 15, 2020
Don't use newlines for error formatting, avoids newlines in logs.

Related to prometheus/node_exporter#1886

Signed-off-by: Ben Kochie <[email protected]>
SuperQ added a commit to SuperQ/kit that referenced this issue Nov 29, 2020
Capture newlines in the stdlib adapter messages. Avoids cutting off
multi-line log messages.

See: prometheus/node_exporter#1886

Signed-off-by: Ben Kochie <[email protected]>
SuperQ added a commit to SuperQ/kit that referenced this issue Nov 30, 2020
Capture newlines in the stdlib adapter messages. Avoids cutting off
multi-line log messages.

See: prometheus/node_exporter#1886

Signed-off-by: Ben Kochie <[email protected]>
peterbourgon pushed a commit to go-kit/kit that referenced this issue Nov 30, 2020
Capture newlines in the stdlib adapter messages. Avoids cutting off
multi-line log messages.

See: prometheus/node_exporter#1886

Signed-off-by: Ben Kochie <[email protected]>
peterbourgon pushed a commit to go-kit/log that referenced this issue May 9, 2021
Capture newlines in the stdlib adapter messages. Avoids cutting off
multi-line log messages.

See: prometheus/node_exporter#1886

Signed-off-by: Ben Kochie <[email protected]>
SuperQ added a commit that referenced this issue Jun 3, 2021
Fix the error logging of the promhttp handler by connecting it to the
promlog setup.
* Switch to go-kit/log.
* Cleanup CHANGELOG.

Fixes: #1886

Signed-off-by: Ben Kochie <[email protected]>
muscliary pushed a commit to muscliary/kit that referenced this issue Sep 12, 2023
Capture newlines in the stdlib adapter messages. Avoids cutting off
multi-line log messages.

See: prometheus/node_exporter#1886

Signed-off-by: Ben Kochie <[email protected]>
oblitorum pushed a commit to shatteredsilicon/node_exporter that referenced this issue Apr 9, 2024
Fix the error logging of the promhttp handler by connecting it to the
promlog setup.
* Switch to go-kit/log.
* Cleanup CHANGELOG.

Fixes: prometheus#1886

Signed-off-by: Ben Kochie <[email protected]>
oblitorum pushed a commit to shatteredsilicon/node_exporter that referenced this issue Apr 9, 2024
Fix the error logging of the promhttp handler by connecting it to the
promlog setup.
* Switch to go-kit/log.
* Cleanup CHANGELOG.

Fixes: prometheus#1886

Signed-off-by: Ben Kochie <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants