feat(input): add upsd implementation #9890

Malinskiy · 2021-10-08T02:37:41Z

Required for all PRs:

Updated associated README.md.
Wrote appropriate unit tests.
Pull request title or commits are in conventional commit format

Added an input plugin to gather upsd metrics using a forked golang library Malinskiy/go.nut from robbiet480/go.nut. Fork is needed to support network timeouts. The metrics are mimicking the apcupsd input plugin as close as possible to allow users to reuse existing dashboards.

telegraf-tiger · 2021-10-08T02:37:47Z

Thanks so much for the pull request!
🤝 ✒️ Just a reminder that the CLA has not yet been signed, and we'll need it before merging. Please sign the CLA when you get a chance, then post a comment here saying !signed-cla

Malinskiy · 2021-10-08T02:39:53Z

!signed-cla

srebhan

Hey @Malinskiy, thanks for this nice plugin! I have some comments in the code... Please think about querying only a single server with the plugin as this will reduce the complexity quite dramatically. You can still collect multiple servers by duplicating the plugin entry in the config file.

go.mod

plugins/inputs/upsd/upsd.go

plugins/inputs/upsd/upsd_test.go

srebhan

Thanks for the quick adaptation! Some more comments from my side, the biggest question being why you are not using the upstream version of go.nut?

plugins/inputs/upsd/README.md

plugins/inputs/upsd/upsd.go

srebhan · 2021-10-12T11:11:37Z

plugins/inputs/upsd/upsd.go

+		"serial":   fmt.Sprintf("%v", metrics["device.serial"]),
+		"ups_name": name,
+		//"variables": variables.Status not sure if it's a good idea to provide this
+		"model": fmt.Sprintf("%v", metrics["device.model"]),


Please don't use fmt.Sprintf if not necessary. This way we will notice if the protocol changes (which should never happen actually).

plugins/inputs/upsd/upsd.go

plugins/inputs/upsd/upsd_test.go

srebhan

Nice update @Malinskiy! This only leaves us with the fmt.Sprintfs and the restructuring of the tests. Furthermore, I want to ask you to split the status flags and add them as separate tags instead of a field...

plugins/inputs/upsd/upsd.go

srebhan · 2021-10-18T15:37:50Z

@Malinskiy any chance to work on this? I'm eagerly waiting for this plugin to use with my UPS... :-)

Malinskiy · 2021-10-18T23:06:39Z

I will resume work on this when I have more time: have a conference talk to prepare to.
Might have time during the weekend (fingers crossed)

srebhan · 2021-10-19T07:25:02Z

@Malinskiy perfect. Just wanted to let you know your PR is appreciated. :-)

Malinskiy · 2021-11-08T00:29:02Z

With the fmt.Sprintf - it's either this or a type cast. If you really want to break in case there is a protocol change - let me know, otherwise I'd leave the fmt.Sprintf call and the breaking behaviour becomes just an incorrect metric

telegraf-tiger · 2021-11-08T00:48:41Z

☺️ This pull request doesn't significantly change the Telegraf binary size (less than 1% increase)

📦 Looks like new artifacts were built from this PR.

Expand this list to get them here! 🐯

Artifact URLs

DEB	RPM	TAR GZ	ZIP
amd64.deb	aarch64.rpm	darwin_amd64.tar.gz	windows_amd64.zip
arm64.deb	armel.rpm	freebsd_amd64.tar.gz	windows_i386.zip
armel.deb	armv6hl.rpm	freebsd_armv7.tar.gz
armhf.deb	i386.rpm	freebsd_i386.tar.gz
i386.deb	ppc64le.rpm	linux_amd64.tar.gz
mips.deb	s390x.rpm	linux_arm64.tar.gz
mipsel.deb	x86_64.rpm	linux_armel.tar.gz
ppc64el.deb		linux_armhf.tar.gz
s390x.deb		linux_i386.tar.gz
		linux_mips.tar.gz
		linux_mipsel.tar.gz
		linux_ppc64le.tar.gz
		linux_s390x.tar.gz
		static_linux_amd64.tar.gz

srebhan

Hey @Malinskiy thanks for the nice update. I think the code is pretty close now, however, I think the remaining fmt.Sprintfs should be removed and replace by proper "casting".

Furthermore, depending on your fork for this plugin is not something we should do as telegraf would then rely on a single person maintaining an external library. Therefore we try to avoid using forked version of libraries wherever possible. I want to ask you to either upstream your changes or to not rely on your modifications and switch to the upstream go.nut library.

plugins/inputs/upsd/upsd.go

Malinskiy · 2022-06-30T02:49:33Z

Removed timeout handling as requested, upstream version is used. Keep in mind that upstream version does 2 additional network requests each time it connects, namely VER and NETVER, the result of those requests is completely ignored by the upstream library so it's an unnecessary overhead.

Sidenote: I am running the forked version of go.nut since October without any issues.

AdamLeyshon · 2022-06-30T09:58:54Z

Exciting! 🎉

kiwimato · 2022-07-01T20:20:10Z

@srebhan @Malinskiy any update on this one ? I can't wait to get rid of the python scripts I currently use and send UPS data with Telegraf instead :)
Thanks for all the work btw, super cool stuff.

srebhan

Looks good. I only have one comment about the naming of the ups.status field. Can you please solve this and I think we are ready to go...

plugins/inputs/upsd/upsd.go

telegraf-tiger · 2022-07-05T06:13:11Z

Download PR build artifacts for linux_amd64.tar.gz, darwin_amd64.tar.gz, and windows_amd64.zip.
Downloads for additional architectures and packages are available below.

☺️ This pull request doesn't significantly change the Telegraf binary size (less than 1%)

📦 Click here to get additional PR build artifacts

Artifact URLs

DEB	RPM	TAR GZ	ZIP
amd64.deb	aarch64.rpm	darwin_amd64.tar.gz	windows_amd64.zip
arm64.deb	armel.rpm	darwin_arm64.tar.gz	windows_i386.zip
armel.deb	armv6hl.rpm	freebsd_amd64.tar.gz
armhf.deb	i386.rpm	freebsd_armv7.tar.gz
i386.deb	ppc64le.rpm	freebsd_i386.tar.gz
mips.deb	riscv64.rpm	linux_amd64.tar.gz
mipsel.deb	s390x.rpm	linux_arm64.tar.gz
ppc64el.deb	x86_64.rpm	linux_armel.tar.gz
riscv64.deb		linux_armhf.tar.gz
s390x.deb		linux_i386.tar.gz
		linux_mips.tar.gz
		linux_mipsel.tar.gz
		linux_ppc64le.tar.gz
		linux_riscv64.tar.gz
		linux_s390x.tar.gz
		static_linux_amd64.tar.gz

srebhan

Looks good to me. Thank you very much @Malinskiy for keeping this PR alive!

I will fix the sample.conf once this is merged...

reimda · 2022-07-06T19:09:41Z

Thanks @Malinskiy!

ostueker · 2022-07-07T00:38:17Z

@srebhan

Isn't da5fdad missing the

[[inputs.upsd]]

Header?

#11463

srebhan · 2022-07-07T07:29:03Z

@ostueker you are right. This is due to the changed way of how the sample configuration is handled now. Can you please open an issue and assign it to me or at least mention me? I'll fix the plugin today then...

srebhan · 2022-07-07T14:21:53Z

@ostueker please check #11471 and let me know (there) if this fixes the issue!

karelkryda · 2022-08-02T12:10:48Z

Hi,
I'm using docker version 1.23.3. According to the date of merging this PR, the plugin should already be available, but it is not.
Am I doing something wrong, or will the plugin only be available in the next version?

Thank you in advance

AdamLeyshon · 2022-08-02T13:16:15Z

It doesn't look like it's been released yet unless you build your own from master Aug 2, 2022 13:11:06 Karel Krýda ***@***.***>:

…

Hi, I'm using docker version 1.23.3. According to the date of merging this PR, the plugin should already be available, but it is not. Am I doing something wrong, or will the plugin only be available in the next version? Thank you in advance — Reply to this email directly, view it on GitHub[#9890 (comment)], or unsubscribe[https://github.com/notifications/unsubscribe-auth/ABXBTVQDLAFPQV6OUEOLBN3VXEF5HANCNFSM5FSTHYKA]. You are receiving this because you were mentioned.[Tracking image][https://github.com/notifications/beacon/ABXBTVSD7QJ4FWIPTMEOZM3VXEF5HA5CNFSM5FSTHYKKYY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGOI6VTXFQ.gif]

karelkryda · 2022-08-02T13:21:20Z

It doesn't look like it's been released yet unless you build your own from master

Aug 2, 2022 13:11:06 Karel Krýda @.***>:

Hi,
I'm using docker version 1.23.3. According to the date of merging this PR, the plugin should already be available, but it is not.
Am I doing something wrong, or will the plugin only be available in the next version?

Thank you in advance

the sample telegraf configuration contains a sample of this plugin, but for some reason the plugin itself is not part of the current version 1.23.3 despite the fact that it was released 8 days ago and this plugin was merged 28 days ago.

amigthea · 2022-08-02T13:24:00Z

It doesn't look like it's been released yet unless you build your own from master
Aug 2, 2022 13:11:06 Karel Krýda @.***>:

Hi,
I'm using docker version 1.23.3. According to the date of merging this PR, the plugin should already be available, but it is not.
Am I doing something wrong, or will the plugin only be available in the next version?
Thank you in advance

the sample telegraf configuration contains a sample of this plugin, but for some reason the plugin itself is not part of the current version 1.23.3 despite the fact that it was released 8 days ago and this plugin was merged 28 days ago.

If I have to guess the documentation is updated between minor release and plugins like this between main release like a 1.24 maybe? I'm waiting too for this to be implemented

powersj · 2022-08-02T13:29:29Z

New plugins are generally not added to bug-fix releases. This will be released in v1.24.0.

We have nightly packages, archives, and docker images for users who want to jump-start on what is available. We appreciate when users do run these nightly images to catch issues earlier, but the typical warnings about the fact that they are nightlies apply.

calebj · 2022-11-16T04:18:17Z

I'm having an issue with mixed types on battery_charge_percent and load_percent. It would seem my APC rack UPSes (Smart-UPS X 2000) which are polled via SNMP report both as float, but the USB HID driver for my Eatons (5S LCD) pass them as integers. Not exactly sure where to report this since it seems to be a NUT behavior that go.nut just passes on to the plugin. But honestly, I don't see a reason not to cast most of the parameters pulled by this plugin to float.

Also, I'm considering a patch to report power draw, but since both vendors also report that differently, I want to test the waters here for whether that's in the scope of the plugin. The APCs give me output voltage and current, and don't distinguish between reactive and apparent power, even though load% for both is available in the web UI. The Eaton, however, gives ups.power, ups.realpower and output voltage, but no current. I'm curious if anyone has different models to compare, because this already seems like a mess.

srebhan · 2022-11-16T08:59:31Z

@calebj can you please open an issue (best is one per topic) to discuss on how to go on from here!?

Malinskiy and others added 5 commits August 25, 2021 20:12

feat(input): add basic upsd implementation

9df94af

fix(upsd): fix conversion from s to ns

03eaed6

feat(upsd): add tests

af259b0

feat(upsd): simplify

8488455

feat(upsd): gather metrics concurrently

4f96a9b

telegraf-tiger bot added fix pr to fix corresponding bug new plugin plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins labels Oct 8, 2021

srebhan requested changes Oct 11, 2021

View reviewed changes

srebhan added area/system and removed fix pr to fix corresponding bug labels Oct 11, 2021

srebhan self-assigned this Oct 11, 2021

Malinskiy added 2 commits October 12, 2021 19:28

feat(upsd): address review feedback

a7d47e1

Merge branch 'master' into feature/upsd

156c8c5

Malinskiy requested a review from srebhan October 12, 2021 08:59

srebhan requested changes Oct 12, 2021

View reviewed changes

Malinskiy added 2 commits October 13, 2021 10:13

feat(upsd): address review feedback

6e0897b

fix(upsd): add "ups.status" field to the test expectation

c0208b4

srebhan requested changes Oct 13, 2021

View reviewed changes

plugins/inputs/upsd/upsd.go Outdated Show resolved Hide resolved

plugins/inputs/upsd/upsd.go Outdated Show resolved Hide resolved

plugins/inputs/upsd/upsd.go Outdated Show resolved Hide resolved

Malinskiy added 2 commits November 8, 2021 11:25

fix(upsd): add status tags and address style feedback

50066f1

fix(upsd): split tests

a6a50c9

Malinskiy requested a review from srebhan November 11, 2021 01:13

srebhan reviewed Nov 17, 2021

View reviewed changes

plugins/inputs/upsd/upsd.go Outdated Show resolved Hide resolved

Malinskiy added 6 commits June 30, 2022 12:51

fix(upsd): fix readme linter

79af9bc

fix(upsd): fix LICENSE_OF_DEPENDENCIES.md

8b45ca1

fix(upsd): more markdown linter fixes

5e25dc1

fix(upsd): return deferred error during disconnect

f37865c

fix(upsd): code linting fix

a55f15b

fix(upsd): fix readme

cc16ad1

srebhan reviewed Jul 4, 2022

View reviewed changes

plugins/inputs/upsd/upsd.go Outdated Show resolved Hide resolved

fix(upsd): field ups.status -> ups_status

7d92070

srebhan approved these changes Jul 5, 2022

View reviewed changes

srebhan added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Jul 5, 2022

reimda approved these changes Jul 6, 2022

View reviewed changes

reimda merged commit fbccc71 into influxdata:master Jul 6, 2022

MyaLongmire pushed a commit that referenced this pull request Jul 6, 2022

feat(input): add upsd implementation (#9890)

a81eeba

srebhan mentioned this pull request Jul 7, 2022

fix(inputs.upsd): Move to new sample.conf style #11471

Merged

3 tasks

sjwang90 mentioned this pull request Sep 7, 2022

Telegraf 1.24 influxdata/docs-v2#4422

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(input): add upsd implementation #9890

feat(input): add upsd implementation #9890

Malinskiy commented Oct 8, 2021

telegraf-tiger bot commented Oct 8, 2021

Malinskiy commented Oct 8, 2021

srebhan left a comment

srebhan left a comment

srebhan Oct 12, 2021

srebhan left a comment

srebhan commented Oct 18, 2021

Malinskiy commented Oct 18, 2021

srebhan commented Oct 19, 2021

Malinskiy commented Nov 8, 2021

telegraf-tiger bot commented Nov 8, 2021

Artifact URLs

srebhan left a comment

Malinskiy commented Jun 30, 2022

AdamLeyshon commented Jun 30, 2022

kiwimato commented Jul 1, 2022 •

edited

Loading

srebhan left a comment

telegraf-tiger bot commented Jul 5, 2022

Artifact URLs

srebhan left a comment

reimda commented Jul 6, 2022

ostueker commented Jul 7, 2022 •

edited

Loading

srebhan commented Jul 7, 2022

srebhan commented Jul 7, 2022

karelkryda commented Aug 2, 2022

AdamLeyshon commented Aug 2, 2022 via email

karelkryda commented Aug 2, 2022 •

edited

Loading

amigthea commented Aug 2, 2022

powersj commented Aug 2, 2022

calebj commented Nov 16, 2022

srebhan commented Nov 16, 2022

feat(input): add upsd implementation #9890

feat(input): add upsd implementation #9890

Conversation

Malinskiy commented Oct 8, 2021

Required for all PRs:

telegraf-tiger bot commented Oct 8, 2021

Malinskiy commented Oct 8, 2021

srebhan left a comment

Choose a reason for hiding this comment

srebhan left a comment

Choose a reason for hiding this comment

srebhan Oct 12, 2021

Choose a reason for hiding this comment

srebhan left a comment

Choose a reason for hiding this comment

srebhan commented Oct 18, 2021

Malinskiy commented Oct 18, 2021

srebhan commented Oct 19, 2021

Malinskiy commented Nov 8, 2021

telegraf-tiger bot commented Nov 8, 2021

Artifact URLs

srebhan left a comment

Choose a reason for hiding this comment

Malinskiy commented Jun 30, 2022

AdamLeyshon commented Jun 30, 2022

kiwimato commented Jul 1, 2022 • edited Loading

srebhan left a comment

Choose a reason for hiding this comment

telegraf-tiger bot commented Jul 5, 2022

Artifact URLs

srebhan left a comment

Choose a reason for hiding this comment

reimda commented Jul 6, 2022

ostueker commented Jul 7, 2022 • edited Loading

srebhan commented Jul 7, 2022

srebhan commented Jul 7, 2022

karelkryda commented Aug 2, 2022

AdamLeyshon commented Aug 2, 2022 via email

karelkryda commented Aug 2, 2022 • edited Loading

amigthea commented Aug 2, 2022

powersj commented Aug 2, 2022

calebj commented Nov 16, 2022

srebhan commented Nov 16, 2022

kiwimato commented Jul 1, 2022 •

edited

Loading

ostueker commented Jul 7, 2022 •

edited

Loading

karelkryda commented Aug 2, 2022 •

edited

Loading