-
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(inputs.gnmi): Register connection statistics before creating client #16171
Conversation
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.
Thanks @Mrflatt! Just one question if we still need the address
in the handler... Otherwise the code looks good.
plugins/inputs/gnmi/gnmi.go
Outdated
address: addr, | ||
host: host, |
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 need the address
in the handler then?
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.
No if I add port also to handler, is that better? To drop address and add host + port, as client, err := grpc.NewClient(h.address, opts...)
still needs host + port
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.
dropped address and added port to handler and joining address from it, before connecting
4e23111
to
bd0f82a
Compare
Now selfstat does not report status of connection if setup of subscription fails. This makes harder to check all hosts statuses, when host count per telegraf is high.
bd0f82a
to
6fd28c1
Compare
Download PR build artifacts for linux_amd64.tar.gz, darwin_arm64.tar.gz, and windows_amd64.zip. 📦 Click here to get additional PR build artifactsArtifact URLs |
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.
Thanks @Mrflatt!
Summary
Now gnmi selfstat does not report status of connection if setup of subscription fails. This makes harder to check all hosts statuses, when they are missing from selfstat. We are using internal plugin to see failures on grpc status, and some host are missing cause subscription fails and self stat is never reported.
Also changed selfstat source to be same as in other metrics, don't know if this is ok to do :)
Checklist
Related issues