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

prometheus api and +/-Inf values #10490

Closed
datamuc opened this issue Nov 16, 2018 · 3 comments
Closed

prometheus api and +/-Inf values #10490

datamuc opened this issue Nov 16, 2018 · 3 comments
Assignees
Milestone

Comments

@datamuc
Copy link

datamuc commented Nov 16, 2018

System info: [Include InfluxDB version, operating system name, and other relevant details

InfluxDB v1.7.0 (git: 1.7 dac4c6f)

Steps to reproduce:

Send +/-Inf values with prometheus' remote_write.

Expected behavior: [What you expected to happen]

Ignore +/-Inf values

Actual behavior: [What actually happened]

The whole batch fails.

Additional info: [Include gist of relevant config, logs, etc.]

Here is an example setup with docker-compose:

https://gist.github.com/datamuc/d6085cc4c4190619796f0ea5a8661dce

influxdb_1       | [httpd] 192.168.208.3 - admin [16/Nov/2018:16:34:55 +0000] "POST /api/v1/prom/write?db=prom HTTP/1.1" 400 59 "-" "Go-http-client/1.1" 8ce96825-e9bd-11e8-8001-0242c0a8d002 88584
prometheus_1     | level=warn ts=2018-11-16T16:34:55.788646973Z caller=queue_manager.go:531 component=remote queue="0:http://influxdb:8086/api/v1/prom/write?db=prom" msg="Error sending samples to remote storage" count=375 err="server returned HTTP status 400 Bad Request: {\"error\":\"+/-Inf is an unsupported value for field value\"}"

All metrics sent in this POST request are lost because of a single +Inf value.

@datamuc datamuc changed the title +Inf prometheus api and +/-Inf values Nov 16, 2018
@datamuc
Copy link
Author

datamuc commented Nov 17, 2018

QuickFix, at least identifying the culprit:

diff --git a/prometheus/converters.go b/prometheus/converters.go
index f3f438587..3cb9a5064 100644
--- a/prometheus/converters.go
+++ b/prometheus/converters.go
@@ -57,6 +57,7 @@ func WriteRequestToPoints(req *remote.WriteRequest) ([]models.Point, error) {
 		for _, s := range ts.Samples {
 			// skip NaN values, which are valid in Prometheus
 			if math.IsNaN(s.Value) {
+				// FIXME: Add some logging here?
 				droppedNaN = ErrNaNDropped
 				continue
 			}
@@ -66,7 +67,8 @@ func WriteRequestToPoints(req *remote.WriteRequest) ([]models.Point, error) {
 			fields := map[string]interface{}{fieldName: s.Value}
 			p, err := models.NewPoint(measurement, models.NewTags(tags), fields, t)
 			if err != nil {
-				return nil, err
+				// FIXME: Add some logging here?
+				continue
 			}
 
 			points = append(points, p)

@marlon-nc
Copy link

This is tripping me up in 1.7.4 as well

e-dard added a commit that referenced this issue Mar 21, 2019
This commit extends the Prometheus remote write endpoint to drop
unsupported Prometheus values, rather than reject the entire batch.

InfluxDB does not support NaN, -Inf or +Inf, but Prometheus does. The
remote write endpoint will now drop these and write valid values in the
provided batch.

If the user enabled write trace logging (`[http] write-tracing = true`)
then summaries of any dropped values within a batch will be logged.

If a batch of values contains any values that are subsequently dropped,
the returned status code will be `204`.
@e-dard e-dard added this to the 1.7.5 milestone Mar 21, 2019
@e-dard
Copy link
Contributor

e-dard commented Apr 1, 2019

Fixed via #12813

@e-dard e-dard closed this as completed Apr 1, 2019
e-dard added a commit that referenced this issue Apr 4, 2019
This commit extends the Prometheus remote write endpoint to drop
unsupported Prometheus values, rather than reject the entire batch.

InfluxDB does not support NaN, -Inf or +Inf, but Prometheus does. The
remote write endpoint will now drop these and write valid values in the
provided batch.

If the user enabled write trace logging (`[http] write-tracing = true`)
then summaries of any dropped values within a batch will be logged.

If a batch of values contains any values that are subsequently dropped,
the returned status code will be `204`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants