Skip to content
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

Change format of listen on vsock #219

Closed
wants to merge 3 commits into from

Conversation

Leonid99
Copy link

@Leonid99 Leonid99 commented May 3, 2024

Before this change, the expected format of listening on
vsock was "--web.listen-address=vsock://:9100".
However, this URI-inspired format is probably confusing,
especially given that it doesn't really support the host
portion of the URI and that listening on TCP doesn't
support the "http://:9100" syntax.
This commit changes the expected format for vsock to be
--web.listen-address=vsock:9100.

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>
@Leonid99 Leonid99 force-pushed the leonid/vsock branch 2 times, most recently from 122811e to 5eed0a8 Compare May 3, 2024 23:19
@Leonid99
Copy link
Author

Leonid99 commented May 4, 2024

Found #202 after publishing this PR. Should I just close this one? (They are identical, except for the expected format of the command line flag).

@SuperQ
Copy link
Member

SuperQ commented May 4, 2024

I think the other PR is mostly ready to go. Would you be interested in adding to the tests as mentioned in the comments after I merge it?

@SuperQ
Copy link
Member

SuperQ commented May 4, 2024

If you have any comments on the other PR, feel free to contribute there.

@Leonid99
Copy link
Author

Leonid99 commented May 5, 2024

If you have any comments on the other PR, feel free to contribute there.

Left a few comments.

Signed-off-by: Leonid Podolny <leonid@podolny.net>
@Leonid99 Leonid99 changed the title Support listen on vsock Change format of listen on vsock May 5, 2024
@Leonid99
Copy link
Author

Leonid99 commented May 5, 2024

@SuperQ I rebased this branch on top of the one by @jeffrey4l to address my own remarks in his #202. Would you be interested in me rebasing this branch to resolve conflicts, so that we can merge it? I will obviously keep @jeffrey4l 's commits untouched to keep proper credit.

@Leonid99 Leonid99 force-pushed the leonid/vsock branch 2 times, most recently from 6c78537 to 163145b Compare May 5, 2024 03:25
Before this change, the expected format of listening on
vsock was "--web.listen-address=vsock://:9100".
However, this URI-inspired format is probably confusing,
especially given that it doesn't really support the host
portion of the URI and that listening on TCP doesn't
support the "http://:9100" syntax.
This commit changes the expected format for vsock to be
"--web.listen-address=vsock:9100".

Signed-off-by: Leonid Podolny <leonid@podolny.net>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants