Skip to content

fix(outputs.prometheus_client)!: Bind the configured vsock context ID - #19854

Open
Vivek1898 wants to merge 1 commit into
influxdata:masterfrom
Vivek1898:fix/19775-prometheus-client-vsock-cid
Open

Vivek1898 wants to merge 1 commit into
influxdata:masterfrom
Vivek1898:fix/19775-prometheus-client-vsock-cid

Conversation

@Vivek1898

Copy link
Copy Markdown

Summary

outputs.prometheus_client split the CID off a vsock://2:9273 listen address and threw it away, then called vsock.Listen(port, nil), which binds the context ID of the machine Telegraf runs on. The listener came up on a different CID than the configuration asked for, nothing warned, and a scraper addressing the configured CID never reached it.

This is the same defect #19774 fixed for inputs.socket_listener, and that PR named this plugin as the follow-up: "outputs.prometheus_client has the same vsock.Listen call with its own parsing and is left for a follow-up." That plugin goes through plugins/common/socket; prometheus_client has its own listener, so the fix did not reach it. This completes it.

The change

// The documented address carries no CID (vsock://:9273) and binding the
// local context ID is right for it. An address that does name one had it
// discarded, so the listener came up on this machine's CID instead and a
// scraper addressing the configured one never reached it.
if cidStr == "" {
    return vsock.Listen(uint32(port), nil)
}

cid, err := strconv.ParseUint(cidStr, 10, 32)
if err != nil {
    return nil, fmt.Errorf("failed to parse CID %s: %w", cidStr, err)
}
listener, err := vsock.ListenContextID(uint32(cid), uint32(port), nil)
if err != nil {
    return nil, fmt.Errorf("listening on CID %d failed: %w", cid, err)
}

Error wording matches #19774 so the two plugins fail the same way, and the README gets the same note yours did.

One deliberate difference from #19774

prometheus_client documents the address as listen = "vsock://:9273" — no CID — and for that form binding the local context ID is correct. inputs.socket_listener always carries a CID, so #19774 had no such case and routes everything through ListenContextID.

Doing that here would have broken the documented default, so the empty host keeps using vsock.Listen and only an explicit CID goes to ListenContextID. There is a test pinning that, because it is the part of this change most likely to be undone by someone later making the two plugins "consistent".

Behaviour change

As in #19774, a configuration naming a CID that does not belong to this machine now fails at startup with EADDRNOTAVAIL instead of quietly binding a different one. That is the point — the previous behaviour was unreachable-by-design — but it is a breaking change for anyone who wrote a CID that was never honoured. CHANGELOG entry added under the existing Important Changes section, next to #19774's.

Testing

Four cases in plugins/outputs/prometheus_client/vsock_test.go, in the shape #19774 used:

Test Pins
TestListenVsockBindsTheConfiguredContextID an explicit CID is the one bound — asserts listener.Addr().(*vsock.Addr).ContextID
TestListenVsockWithoutContextIDBindsLocal :9274 still binds the local CID — the documented form, and the difference from #19774
TestListenVsockRejectsForeignContextID 4294967294 is refused with listening on CID 4294967294 failed rather than silently rebinding
TestListenVsockRejectsNonNumericContextID a non-numeric CID gives failed to parse CID host

gofmt is clean, go vet ./plugins/outputs/prometheus_client/ passes, and go test ./plugins/outputs/prometheus_client/... -count=1 is green: every pre-existing test passes, the non-numeric case passes, and the three that need a vsock-capable host skip.

The skip guard is tighter than #19774's, for a reason I hit

I tried to actually run the binding assertions rather than assume them, in a privileged Linux container with /dev/vsock present. vsock.ContextID() succeeds there and returns 4294967295 — VMADDR_CID_ANY, the wildcard — because Docker Desktop's VM has no guest transport. A guard on the error alone does not fire, and the bind then fails:

listening on CID 4294967295 failed: listen vsock vm(4294967295):9273: socket: function not implemented

That is not specific to my tests. Your merged test fails the same way in the same container:

=== RUN   TestVsockAddressParsing
    socket_test.go:936: listening on CID 4294967295 failed:
        listen vsock vm(4294967295):8790: socket: function not implemented
--- FAIL: TestVsockAddressParsing
--- PASS: TestVsockForeignContextID

So these tests skip through requireVsock, which also skips when the reported context ID is the wildcard. I have observed the skip path firing; the non-skip path is a uint32 comparison, and a real guest or host CID is never 0xffffffff.

plugins/common/socket/socket_test.go has the same gap. I did not touch it — it is not what this PR is about and I would rather you decide — but say the word and I will send the same two-line guard there.

What that leaves unverified

The two positive binding assertions did not execute anywhere I have access to, so the claim that ListenContextID binds the CID you give it rests on the same upstream contract #19774 already relies on, not on a run I can show you. go test ./plugins/outputs/prometheus_client/ -run Vsock -v on a vsock-capable guest would turn those two skips into real coverage.

Checklist

Related issues

resolves #19775

@telegraf-tiger

telegraf-tiger Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

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

@telegraf-tiger telegraf-tiger Bot added fix pr to fix corresponding bug plugin/output 1. Request for new output plugins 2. Issues/PRs that are related to out plugins labels Oct 7, 2026
listenVsock split the CID out of the listen address and discarded it, then
called vsock.Listen, which binds the context ID of the machine Telegraf runs
on. An address like vsock://2:9273 came up on a different CID, nothing warned,
and a scraper addressing the configured one never reached it.

Bind the given CID via vsock.ListenContextID so the address decides what gets
bound, and fail on startup when the CID is not ours instead of silently
binding something else. An address with no CID (vsock://:9273), the documented
form, keeps binding the local context ID.

This completes influxdata#19774, which fixed the same defect for inputs.socket_listener
through plugins/common/socket and left this plugin's own listener as a
follow-up.

resolves influxdata#19775
@Vivek1898
Vivek1898 force-pushed the fix/19775-prometheus-client-vsock-cid branch from 6f63a3c to 31309fe Compare October 7, 2026 08:26
@telegraf-tiger

telegraf-tiger Bot commented Oct 7, 2026

Copy link
Copy Markdown
Contributor

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 increases the Telegraf binary size by 5.84 % for linux amd64 (new size: 326.2 MB, nightly size 308.2 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

@Vivek1898

Copy link
Copy Markdown
Author

Two jobs are red and I do not think either is from this change, so here is the evidence rather than an assertion — I cannot re-run them from my side.

test-go-windows — 8904 tests, 420 skipped, 2 failures, both in plugins/inputs/nvidia_smi:

--- FAIL: plugins/inputs/nvidia_smi TestProbe/probe_success (5.03s)
    Error: Received unexpected error:
      calling "C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe" failed: command timed out
--- FAIL: plugins/inputs/nvidia_smi TestProbe (8.38s)

A PowerShell invocation timing out on the runner. The same log shows this PR's own tests taking the skip path on Windows, which is what should happen:

SKIP: plugins/outputs/prometheus_client TestListenVsockWithoutContextIDBindsLocal
    vsock_test.go:52: vsock not available on this host: vsock: not implemented on windows
SKIP: plugins/outputs/prometheus_client TestListenVsockRejectsForeignContextID
    vsock_test.go:67: vsock not available on this host: vsock: not implemented on windows

test-integration — two failures, neither in a package this PR touches:

--- FAIL: TestGatherUDPCertIntegration
FAIL  github.com/influxdata/telegraf/plugins/inputs/x509_cert   0.515s
--- FAIL: TestIntegration (42.76s)
FAIL  github.com/influxdata/telegraf/plugins/outputs/graphite   42.824s

I checked the one thing that could have connected them to this PR: scripts/check-plugin-changes.sh falls back to ./... as soon as a file outside plugins/{inputs,outputs,aggregators,processors} changes, and this PR touches CHANGELOG.md. But #19853 (CHANGELOG.md, plugins/common/sudo/) and #19852 (config/) select ./... for the same reason and both are green, so this is not a wider test selection than theirs — just three flaky tests.

The change itself is confined to plugins/outputs/prometheus_client, which nothing in nvidia_smi, x509_cert or graphite imports. Happy to push an empty commit to re-trigger if that is easier than a re-run.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fix pr to fix corresponding bug plugin/output 1. Request for new output plugins 2. Issues/PRs that are related to out plugins

Projects

None yet

Development

Successfully merging this pull request may close these issues.

outputs.prometheus_client: vsock listener silently ignores a CID in the listen address

1 participant