-
Notifications
You must be signed in to change notification settings - Fork 905
chore(vpn): send info, debug logs over tunnel #18240
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
Conversation
This stack of pull requests is managed by Graphite. Learn more about stacking. |
@@ -65,7 +65,6 @@ func (r *RootCmd) vpnDaemonRun() *serpent.Command { | |||
logger.Info(ctx, "starting tunnel") | |||
tunnel, err := vpn.NewTunnel(ctx, logger, pipe, vpn.NewClient(), | |||
vpn.UseOSNetworkingStack(), | |||
vpn.UseAsLogger(), |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The Windows app drops logs received over the tunnel, so this can be removed. It adds it's own sink to stderr
.
select { | ||
case <-t.updater.ctx.Done(): | ||
return | ||
case <-t.ctx.Done(): | ||
return | ||
case t.sendCh <- msg: |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
We deliberately never cancel the tunnel ctx, due to the somewhat precarious teardown procedure. We do however cancel the updater ctx to ensure we don't tick network updates after shutdown, so we'll check for that here too.
logMu sync.Mutex | ||
logs []*TunnelMessage |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not seeing any value in this buffer, as we don't do any disk I/O - the pipe is entirely in-memory. With pings every 5 seconds and debug logs on, we produce around 150~ logs a minute, or about 35 kilobytes. The in-memory pipe can handle gigabytes a second. I think buffering would make sense were we relaying the logs to a terminal or a file, but that's not the case here.
Since only Error and above logs are flushed immediately, the alternative is adding the complexity of flushing the logs from the buffer periodically. One downside of this is that the macOS timestamps on the logs once they reach the Swift code aren't actually representative of the time the log was produced.
c65ebb2
to
8d8937d
Compare
8d8937d
to
c5c1ab5
Compare
Merge activity
|
c5c1ab5
to
8535d64
Compare
Review request can be ignored - graphite touched proto files for a moment doing it's branch switching + rebase (why?). |
Closes coder/internal#397