-
Notifications
You must be signed in to change notification settings - Fork 5.6k
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
Fixed json serialization for OpenTSDB http output #1793
Conversation
…enTSDB are sent and made sure we send numbers un-quoted event though OpenTSDB API accepts them as this is not clean json.
case uint64: | ||
case float64: | ||
default: | ||
fmt.Printf("OpenTSDB does not support metric value: [%s] of type [%T].\n", value, value) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
don't print every single time this happens
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sparrc Would printing once every metrics drop be ok ?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
do we really need to be logging why non-number fields aren't being sent? wouldn't an OpenTSDB user know that they can't use strings/bools?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess not. I'll just log in debug then.
@@ -16,7 +16,7 @@ import ( | |||
type HttpMetric struct { | |||
Metric string `json:"metric"` | |||
Timestamp int64 `json:"timestamp"` | |||
Value string `json:"value"` | |||
Value interface{} `json:"value"` |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
can you explain this? how was this working before if every value was being sent as a string?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sparrc The OpenTSDB API is a bit loose. It says it doesn't accept string values but it does accept numbers quoted or not. So that is why it is working. Although the API was returning errors for string(characters) values it did process all metrics compared to the telnet API which just bail out at the first error. When you use tools between telegraf and OpenTSDB and they are more strict, it can cause problems so this is why I am making the change.
Required for all PRs:
Fixed json serialization to make sure only value types supported by OpenTSDB are sent and made sure we send numbers un-quoted event though OpenTSDB API accepts them as this is not clean json.