Support listen on vsock by jeffrey4l · Pull Request #202 · prometheus/exporter-toolkit

jeffrey4l · 2024-02-26T08:22:25Z

metrics can be retrive without nic between host and qemu guest through vsock.

more info check

SuperQ

Interesting idea. This also needs to be added to the Go documentation and probably the flag help string in web/kingpinflag/flag.go.

web/tls_config.go

SuperQ · 2024-02-26T09:02:05Z

Please add documentation to the ListenAndServe func docs (before like 285). A link to https://wiki.qemu.org/Features/VirtioVsock would be useful there.

SuperQ

Thanks, looks good.

With this patch, node_exporter can run with `--web.listen-address=vsock: //:9100` to listen on vsock in qemu guest. Then host can get the metrics. Signed-off-by: Jeffrey Zhang <zhang.lei.fly@gmail.com>

bwplotka · 2024-04-22T07:29:47Z

👍🏽

SuperQ · 2024-04-22T07:36:50Z

Would you please rebase to fix the conflict?

jesusvazquez

LGTM

I'd like some testing for ListenAndServe here

exporter-toolkit/web/tls_config_test.go

Lines 479 to 492 in 48431d7

    
           go func() { 
        
           	defer func() { 
        
           		if recover() != nil { 
        
           			recordConnectionError(errors.New("Panic starting server")) 
        
           		} 
        
           	}() 
        
           	flags := FlagConfig{ 
        
           		WebListenAddresses: &([]string{port}), 
        
           		WebSystemdSocket:   OfBool(false), 
        
           		WebConfigFile:      &test.YAMLConfigPath, 
        
           	} 
        
           	err := ListenAndServe(server, &flags, testlogger) 
        
           	recordConnectionError(err) 
        
           }()

To make sure it listens both on tcp and vsock if specified.

But I won't block on this if other people don't think this is necessary 👍

Leonid99 · 2024-05-05T02:38:31Z

web/tls_config.go

+		} else {
+			listener, err = net.Listen("tcp", address)
+			if err != nil {
+				return nil


Shouldn't this be return err? It silently swallows an error condition.

web/kingpinflag/flag.go

Leonid99 · 2024-05-05T18:09:27Z

Would you please rebase to fix the conflict?

For your consideration, I rebased this branch as #220 to allow it to be merged (with original authorship of @jeffrey4l). The only conflict is go.mod.

septatrix

This behavior can already be archived using the existing systemd socket use case as those are not only able to listen on TCP/IP (AF_INET/AF_INET6) and Unix Domain Sockets (AF_UNIX), but also on Vsock (AF_VSOCK) - among some others.

SuperQ · 2024-08-01T09:05:21Z

@septatrix

Interesting, so you're saying that this isn't needed? Do we just need to update the documentation?

septatrix · 2024-08-01T09:13:50Z

@septatrix

Interesting, so you're saying that this isn't needed? Do we just need to update the documentation?

Yes, at least for systems using systemd (or a compatible implementation of the listener interface). See man:systemd.socket:

If the address string is a string in the format "vsock:x:y", it is read as CID x on a port y address in the AF_VSOCK family. The CID is a unique 32-bit integer identifier in AF_VSOCK analogous to an IP address. Specifying the CID is optional, and may be set to the empty string. "vsock" may be replaced with "vsock-stream", "vsock-dgram" or "vsock-seqpacket" to force usage of the corresponding socket type.

Leonid99 · 2024-08-02T03:51:03Z

that this isn't needed

@SuperQ
I would advocate to merge this change anyway. Using systemd for sockets translation isn't different from other methods of socket translation, such as socat, i.e. it's always possible, for example, expose a socket that listens on a TCP port to local domain sockets, etc., but most network services support listening on both natively, without socat or systemd.

Also, another reminder that a rebased version of this PR exists as #220. (I am not trying to claim credit, I'm just interested in having this merged).

SuperQ · 2024-08-02T07:18:14Z

Closing in favor of #220

jeffrey4l force-pushed the master branch 3 times, most recently from 322c7e8 to cc3c96a Compare February 26, 2024 08:26

SuperQ requested changes Feb 26, 2024

View reviewed changes

web/tls_config.go Outdated Show resolved Hide resolved

jeffrey4l force-pushed the master branch from cc3c96a to f9a6d94 Compare February 26, 2024 08:45

jeffrey4l requested a review from SuperQ February 26, 2024 08:47

jeffrey4l force-pushed the master branch from f9a6d94 to 41e73c7 Compare February 26, 2024 09:11

SuperQ approved these changes Feb 26, 2024

View reviewed changes

Support listen on vsock

48431d7

With this patch, node_exporter can run with `--web.listen-address=vsock: //:9100` to listen on vsock in qemu guest. Then host can get the metrics. Signed-off-by: Jeffrey Zhang <zhang.lei.fly@gmail.com>

jeffrey4l force-pushed the master branch from 41e73c7 to 48431d7 Compare February 27, 2024 00:39

jeffrey4l requested a review from SuperQ February 27, 2024 00:39

SuperQ requested a review from roidelapluie February 27, 2024 10:43

jesusvazquez approved these changes Apr 22, 2024

View reviewed changes

Leonid99 mentioned this pull request May 4, 2024

Change format of listen on vsock #219

Closed

Leonid99 reviewed May 5, 2024

View reviewed changes

Leonid99 mentioned this pull request May 5, 2024

Support listen on vsock #220

Merged

septatrix reviewed Jun 26, 2024

View reviewed changes

SuperQ closed this Aug 2, 2024

	go func() {
	defer func() {
	if recover() != nil {
	recordConnectionError(errors.New("Panic starting server"))
	}
	}()
	flags := FlagConfig{
	WebListenAddresses: &([]string{port}),
	WebSystemdSocket: OfBool(false),
	WebConfigFile: &test.YAMLConfigPath,
	}
	err := ListenAndServe(server, &flags, testlogger)
	recordConnectionError(err)
	}()

Conversation

jeffrey4l commented Feb 26, 2024

Uh oh!

SuperQ left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

SuperQ commented Feb 26, 2024

Uh oh!

SuperQ left a comment

Choose a reason for hiding this comment

Uh oh!

bwplotka commented Apr 22, 2024

Uh oh!

SuperQ commented Apr 22, 2024

Uh oh!

jesusvazquez left a comment • 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!

Leonid99 May 5, 2024

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Leonid99 commented May 5, 2024

Uh oh!

septatrix left a comment

Choose a reason for hiding this comment

Uh oh!

SuperQ commented Aug 1, 2024

Uh oh!

septatrix commented Aug 1, 2024

Uh oh!

Leonid99 commented Aug 2, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

SuperQ commented Aug 2, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

jesusvazquez left a comment •

edited

Loading

Leonid99 commented Aug 2, 2024 •

edited

Loading