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.

Sign up for GitHub

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

Jump to bottom

tracing-journald: Do not connect socket #1745

Closed
swsnr opened this issue Nov 23, 2021 · 3 comments · Fixed by #1758
Closed

tracing-journald: Do not connect socket #1745

swsnr opened this issue Nov 23, 2021 · 3 comments · Fixed by #1758

Comments

@swsnr
Copy link
Contributor

swsnr commented Nov 23, 2021

Bug Report

Currently tracing-journald subscribers immediately connect to the systemd journal socket.

I believe this implies that the subscriber will permanently fail if the connection to the socket ever drops, e.g. if journald crashes. I think the subscriber should use an unconnected socket and then use send_to to specify the target socket explicitly for every message. That'd make a subscriber "survive" a journald crash, and it's also what upstream does

I can make a pull request (based on #1744) to change this.

Crates

tracing-journald
@swsnr
Copy link
Contributor Author

swsnr commented Nov 23, 2021

/cc @Ralith

@Ralith
Copy link
Collaborator

Ralith commented Nov 23, 2021

SGTM!

@Ralith
Copy link
Collaborator

Ralith commented Nov 23, 2021

Actually, one drawback to this: if we don't connect at startup, we lose the immediate, actionable feedback on journald being missing/misconfigured/whatever. This isn't a problem for upstream because they propagate errors to the user, but a tracing subscriber can't. Surviving a journald restart is still a huge win, but we should probably at least check that /run/systemd/journal/socket exists and is writable in new.

@swsnr swsnr mentioned this issue Nov 30, 2021
Merged
hawkw pushed a commit that referenced this issue Dec 16, 2021
@swsnr
journald: use unconnected socket (#1758)
2e84c0b 
Lets journald subscribers survive a journald restart.

Closes #1745

## Motivation

Currently the journald subscriber immediately connects to the journald
socket.  As such I understand it'd not survive a full restart of
journald.

## Solution

Do not connect the client socket immediately; instead pass the socket
pathname every time we send a message.  This is also what upstream does.
hawkw pushed a commit that referenced this issue Dec 19, 2021
@swsnr @hawkw
journald: use unconnected socket (#1758)
c2e2737 
Lets journald subscribers survive a journald restart.

Closes #1745

## Motivation

Currently the journald subscriber immediately connects to the journald
socket.  As such I understand it'd not survive a full restart of
journald.

## Solution

Do not connect the client socket immediately; instead pass the socket
pathname every time we send a message.  This is also what upstream does.
hawkw pushed a commit that referenced this issue Dec 19, 2021
@swsnr @hawkw
journald: use unconnected socket (#1758)
89de8f7 
Lets journald subscribers survive a journald restart.

Closes #1745

## Motivation

Currently the journald subscriber immediately connects to the journald
socket.  As such I understand it'd not survive a full restart of
journald.

## Solution

Do not connect the client socket immediately; instead pass the socket
pathname every time we send a message.  This is also what upstream does.
hawkw pushed a commit that referenced this issue Dec 20, 2021
@swsnr @hawkw
journald: use unconnected socket (#1758)
f8d8ea3 
Lets journald subscribers survive a journald restart.

Closes #1745

## Motivation

Currently the journald subscriber immediately connects to the journald
socket.  As such I understand it'd not survive a full restart of
journald.

## Solution

Do not connect the client socket immediately; instead pass the socket
pathname every time we send a message.  This is also what upstream does.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging a pull request may close this issue.

Use unconnected socket
2 participants
@Ralith @swsnr