fix(inputs.upsd): Enforce string type for vendor and product IDs by majiayu000 · Pull Request #18626 · influxdata/telegraf

majiayu000 · 2026-03-28T12:52:17Z

Summary

The go.nut library auto-detects numeric-looking values using a regex and converts them to int64. This means a vendorid like "0764" becomes int64(764), but a non-numeric vendorid like "ABCD" stays a string. When both types of UPS devices report to the same InfluxDB bucket, you get type conflicts on the vendorid/productid fields.

This adds a stringFieldSet that forces ups.vendorid, ups.productid, driver.parameter.vendorid, and driver.parameter.productid to always be emitted as strings, following the same internal.ToString pattern already used for ups.firmware. The string check runs before the ForceFloat conversion so these fields aren't accidentally cast to float.

Also adds a test case with non-numeric vendor/product IDs to confirm type consistency.

Checklist

No AI generated code was used in this PR

Related issues

resolves #18308

The go.nut library auto-detects numeric-looking values and converts them to integers, stripping leading zeros (e.g. "0764" becomes 764). This causes type conflicts in InfluxDB when different UPS vendors report non-numeric IDs for the same fields. Convert ups.vendorid, ups.productid, driver.parameter.vendorid, and driver.parameter.productid to strings before writing, following the same pattern used for ups.firmware. Signed-off-by: majiayu000 <1835304752@qq.com>

Signed-off-by: majiayu000 <1835304752@qq.com>

srebhan

Thanks for your contribution @majiayu000!

skartikey · 2026-04-17T15:51:38Z

Hi @majiayu000, thanks for this fix. After discussion with @srebhan, we'd like to land this behind a config option so users can migrate gracefully rather than getting a silent type change on upgrade. Here's the plan:

This PR (step 1): Add a config option (e.g. stringify_ids bool, defaulting to false to preserve current behavior). When the option is unset, keep the old numeric behavior but emit a warning in Init() noting that the default will flip in a future release and users should set the option explicitly. When set, apply the string-enforcement logic this PR already implements. Please also add a short note to the plugin README and a changelog entry flagging the upcoming default change.

Future PRs (for our tracking, not this one):

Step 2: flip the default to string, emit an info log for users still not setting the option, mark the option deprecated for removal >= v1.x+5.
Step 3: remove the option, keep only the enforce-string behavior.

One small thing worth knowing: because go.nut parses "0764" into int64(764) before we see it, the emitted string will be "764" rather than "0764". Leading zeros are lost and not recoverable at this layer. Probably worth a line in the README so users know.

Let me know if you have questions on the option naming or wording of the warning.

Per maintainer feedback, introduce the fix behind an opt-in config option so users get a migration path rather than a silent type change on upgrade. This is step 1 of a 3-step migration: Step 1 (this commit): Add `stringify_ids` bool, default false to preserve the legacy numeric behavior. Emit a warning in Init() when the option is unset noting the default will flip in a future release. Step 2 (future): flip the default to true, deprecate the option. Step 3 (future): remove the option. When `stringify_ids = true`, ups.vendorid, ups.productid, driver.parameter.vendorid and driver.parameter.productid are forced to string before the ForceFloat conversion, so the numeric-looking values that go.nut auto-casts to int64 (e.g. "0764" -> 764) and the non-numeric values ("ABCD") are all emitted with the same field type. Note: because go.nut parses "0764" into int64(764) before Telegraf sees it, the emitted string is "764"; leading zeros are not recoverable at this layer. Documented in the plugin README. Revert CyberPowerSystems_CP900EPFCLCD_full/expected.out to the pre-PR integer form so the default-off behavior is covered, and add a CyberPowerSystems_CP900EPFCLCD_full_stringify_ids testcase that exercises the opt-in path. Signed-off-by: majiayu000 <1835304752@qq.com>

majiayu000 · 2026-04-17T19:38:45Z

Thanks @skartikey, updated the PR to follow the 3-step migration plan. Pushed as 8f42f14.

Changes in this push:

Added stringify_ids *bool config option, defaulting to unset. When unset, the legacy numeric behavior is preserved and a warning is emitted in Init() noting the default will flip in a future release. Setting it explicitly (either true or false) silences the warning.
Gated the stringFieldSet enforcement behind stringify_ids = true. The check still runs before the ForceFloat branch so IDs aren't accidentally cast to float.
Reverted CyberPowerSystems_CP900EPFCLCD_full/expected.out to the pre-PR integer form so it covers the default-off path.
Added a CyberPowerSystems_CP900EPFCLCD_full_stringify_ids testcase with the same variables but stringify_ids = true in the conf, expecting strings.
Documented the option in sample.conf and the README, including the "0764" → "764" leading-zero note you called out (good catch, thanks).

Happy to adjust the warning wording or the option name if you'd prefer something else. Also let me know if you'd rather have stringify_ids be a plain bool (default false, no warning) — I went with *bool so "unset" is distinguishable from an explicit false, which is what makes the "please set me explicitly" warning possible. If you'd rather keep the struct simpler I can switch to plain bool and drop the warning.

telegraf-tiger · 2026-04-17T19:57:35Z

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

🥳 This pull request decreases the Telegraf binary size by -1.05 % for linux amd64 (new size: 300.7 MB, nightly size 303.9 MB)

📦 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_arm64.zip
armel.deb	armv6hl.rpm	freebsd_amd64.tar.gz	windows_i386.zip
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

skartikey

@majiayu000 some more comments.

skartikey · 2026-04-20T06:21:57Z

+behavior, but will flip to `true` in a future release. If the option is left
+unset, a warning is logged on startup.
+
+> **Note:** the NUT library parses `"0764"` into `int64(764)` before Telegraf


Please switch the > **Note:** block to GitHub alert syntax (> [!NOTE]). Matches the style guide and renders as a proper callout on GitHub.

skartikey · 2026-04-20T06:23:25Z

+unset, a warning is logged on startup.
+
+> **Note:** the NUT library parses `"0764"` into `int64(764)` before Telegraf
+> sees it, so the stringified value will be `"764"` — leading zeros are lost


nit: replace the em-dash in "764" — leading zeros are lost with a semicolon or period.

skartikey · 2026-04-20T06:29:01Z

 	u.dumped = make(map[string]bool)

+	if u.StringifyIDs == nil {
+		u.Log.Warn("'stringify_ids' is not set explicitly; vendor/product IDs may be emitted as int64 " +


Would you mind adding a small test asserting the warning fires when stringify_ids is unset (and stays silent when set to true/false)? The warning is load-bearing for the migration; a regression test will keep it honest across future refactors.

skartikey · 2026-04-20T06:33:32Z

+		u.Log.Warn("'stringify_ids' is not set explicitly; vendor/product IDs may be emitted as int64 " +
+			"when the underlying value looks numeric, causing type conflicts between UPS devices. " +
+			"The default will change to 'true' in a future release. " +
+			"Set 'stringify_ids = true' to opt into the new behavior now, or 'stringify_ids = false' to keep the legacy behavior and silence this warning.")


The warning string is a 4-line concat; consider tightening the wording

Suggested change

u.Log.Warn("'stringify_ids' is not set explicitly; vendor/product IDs may be emitted as int64 " +

"when the underlying value looks numeric, causing type conflicts between UPS devices. " +

"The default will change to 'true' in a future release. " +

"Set 'stringify_ids = true' to opt into the new behavior now, or 'stringify_ids = false' to keep the legacy behavior and silence this warning.")

u.Log.Warn("Option 'stringify_ids' is not set; the default will flip from 'false' to 'true' " +

"in a future release to fix vendor/product ID type conflicts. " +

"Set it explicitly to lock in behavior and silence this warning.")

skartikey · 2026-04-20T06:35:31Z

+			if err == nil {
+				v = str
+			}
+		} else if u.ForceFloat {


please keep the stringify/force-float comments in the same position relative to their if blocks (both above).

skartikey · 2026-04-20T06:38:10Z

+  ## Force vendor/product IDs to always be emitted as strings to avoid type
+  ## conflicts between UPS devices with numeric-looking IDs (e.g. "0764",
+  ## auto-converted to int64 by the NUT client library) and UPS devices with
+  ## non-numeric IDs (e.g. "ABCD"). Affected fields: ups_vendorid, ups_productid,
+  ## driver_parameter_vendorid, driver_parameter_productid.
+  ## Note: leading zeros are lost on the wire (e.g. "0764" becomes "764").
+  ## If left unset, a warning is logged. The default will change to true in a
+  ## future release.
+  # stringify_ids = false


Could you trim the comment block down to 2-3 lines? The detailed rationale is already in the README section. The sample.conf should stay scannable.

Suggested change

## Force vendor/product IDs to always be emitted as strings to avoid type

## conflicts between UPS devices with numeric-looking IDs (e.g. "0764",

## auto-converted to int64 by the NUT client library) and UPS devices with

## non-numeric IDs (e.g. "ABCD"). Affected fields: ups_vendorid, ups_productid,

## driver_parameter_vendorid, driver_parameter_productid.

## Note: leading zeros are lost on the wire (e.g. "0764" becomes "764").

## If left unset, a warning is logged. The default will change to true in a

## future release.

# stringify_ids = false

## Emit vendor/product IDs as strings regardless of their value. Avoids

## type conflicts when some UPS devices report numeric-looking IDs and

## others report alphanumeric. See README for migration notes.

# stringify_ids = false

skartikey · 2026-04-20T06:40:52Z

+driver.parameter.vendor: NUMBER
+driver.parameter.vendorid: NUMBER
+driver.state: NUMBER
+driver.version:


types.dev inconsistency: the sibling _full_stringify_ids/types.dev:27 has driver.version: (trailing space, no type) while this file has driver.version: (no trailing space). Make them identical, pick one and apply everywhere.

telegraf-tiger Bot added fix pr to fix corresponding bug plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins labels Mar 28, 2026

fix(inputs.upsd): fix gofmt alignment in stringFieldSet map

7c6cd34

Signed-off-by: majiayu000 <1835304752@qq.com>

majiayu000 marked this pull request as ready for review March 29, 2026 04:39

srebhan approved these changes Apr 16, 2026

View reviewed changes

srebhan changed the title ~~fix(inputs.upsd): force vendorid and productid fields to string type~~ fix(inputs.upsd): Avoiding type conflicts by enforcing vendor and product IDs to string Apr 16, 2026

srebhan changed the title ~~fix(inputs.upsd): Avoiding type conflicts by enforcing vendor and product IDs to string~~ fix(inputs.upsd): Enforce string type for vendor and product IDs Apr 16, 2026

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 Apr 16, 2026

srebhan assigned skartikey Apr 16, 2026

skartikey reviewed Apr 20, 2026

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix(inputs.upsd): Enforce string type for vendor and product IDs#18626

fix(inputs.upsd): Enforce string type for vendor and product IDs#18626
majiayu000 wants to merge 3 commits intoinfluxdata:masterfrom
majiayu000:fix/upsd-vendorid-productid-string-type

majiayu000 commented Mar 28, 2026

Uh oh!

srebhan left a comment

Uh oh!

skartikey commented Apr 17, 2026

Uh oh!

majiayu000 commented Apr 17, 2026

Uh oh!

telegraf-tiger Bot commented Apr 17, 2026

Artifact URLs

Uh oh!

skartikey left a comment

Uh oh!

skartikey Apr 20, 2026

Uh oh!

skartikey Apr 20, 2026

Uh oh!

skartikey Apr 20, 2026

Uh oh!

skartikey Apr 20, 2026

Uh oh!

skartikey Apr 20, 2026

Uh oh!

skartikey Apr 20, 2026

Uh oh!

skartikey Apr 20, 2026 •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Conversation

majiayu000 commented Mar 28, 2026

Summary

Checklist

Related issues

Uh oh!

srebhan left a comment

Choose a reason for hiding this comment

Uh oh!

skartikey commented Apr 17, 2026

Uh oh!

majiayu000 commented Apr 17, 2026

Uh oh!

telegraf-tiger Bot commented Apr 17, 2026

Artifact URLs

Uh oh!

skartikey left a comment

Choose a reason for hiding this comment

Uh oh!

skartikey Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

skartikey Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

skartikey Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

skartikey Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

skartikey Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

skartikey Apr 20, 2026

Choose a reason for hiding this comment

Uh oh!

skartikey Apr 20, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

skartikey Apr 20, 2026 •

edited

Loading