-
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
Fix GNMI TypedValue to scalar value conversion and update supported platforms #7173
Conversation
@@ -324,23 +323,9 @@ func (c *CiscoTelemetryGNMI) handleTelemetryField(update *gnmi.Update, tags map[ | |||
return aliasPath, nil | |||
} | |||
|
|||
value, _ := gnmivalue.ToScalar(update.Val) |
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.
Check the error and also there are some value types we should probably skip, such as []byte. The field needs to be one of int64, float64, uint64, string, bool, there is some massaging that is done when acc.AddField is called but that was done mostly to avoid errors with older plugins, we try do it in the plugin now.
Is this going to work with the JsonVal/JsonIetfVal types? Maybe we should do the ToScaler as the default case of the switch?
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 see your points, I therefore reverted those changes and just changed the handling for the decimalval type. I think in hindsight it would be more trouble switching to the native casting function since we would then need to deal with the error cases as it also does not deal with the Json types which we would need to handle manually anyway.
This uses the GNMI library native ToScalar function to convert GNMI TypedValues to scalar Go values.
Also adds an updated note on tested device platforms.
Fixes #7167