chore(apple): Remove connlib_client_apple log filters because they have no effect #4182
Conversation
|
The latest updates on your projects. Learn more about Vercel for Git ↗︎ 1 Ignored Deployment
|
Terraform Cloud Plan Output |
jamilbk
changed the title
Performance Test ResultsTCP
UDP
|
There was a problem hiding this comment.
ACK modulo one comment.
We probably just don't have any logging code anymore in that gluecode crate. That is why they don't have any effect.
cc @conectado Should we use explicit log targets everywhere do that the crate name doesn't matter? Like, use connlib everywhere?
| @@ -31,15 +31,14 @@ struct AdvancedSettings: Equatable { | |||
| authBaseURLString: "https://app.firez.one/", | |||
| apiURLString: "wss://api.firez.one/", | |||
| connlibLogFilterString: | |||
| "connlib_client_apple=debug,firezone_tunnel=trace,phoenix_channel=debug,connlib_shared=debug,connlib_client_shared=debug,info" | |||
| "firezone_tunnel=trace,phoenix_channel=debug,connlib_shared=debug,connlib_client_shared=debug,str0m=info,debug" | |||
There was a problem hiding this comment.
If you are setting an overall debug level, you could remove the others. I've found that too noise though because of the TLS stuff with the websocket.
I usually use overall info and narrow them down to debug / trace for others.
There was a problem hiding this comment.
So the issue is that only the global filter applies to this glue module. Should we start at debug for dev builds and tune out the noisy ones with info on an adhoc basis?
That might be better than logging info globally in order to see things during development, because those might end up accidentally making their way to release.
The issue is that I added some in the other PR and it wasn't showing up which caused a few minutes of confusion. The global Maybe it's the crate type? |
That is odd! |
Tried to organize this PR into commits so that it's a bit easier to review. 1. Involves simplifying the logic in Adapter.swift so that us mortals can maintain it confidently: - The `.stoppingTunnel`, `.stoppedTunnelTemporarily`, and `.stoppingTunnelTemporarily` states have been removed. - I also removed the `self.` prefix from local vars when it's not necessary to use it, to be more consistent. - `onTunnelReady` and `getSystemDefaultResolvers` has been removed, and `onUpdateRoutes` wired up, along with cleanup necessary to support that. 2. Involves adding the `reconnect` and `set_dns` stubs in the FFI and fixing the log filter so that we can log them (see #4182 ) 3. Involves getting the path update handler working well on macOS using `SystemConfiguration` to read DNS servers. 4. Involves getting the path update handler working well on iOS by employing careful trickery to prevent path update cycles by detecting if `path.gateways` has changed, and avoid setting new DNS if it hasn't. Refs #4028 Fixes #4297 Fixes #3565 Fixes #3429 Fixes #4175 Fixes #4176 Fixes #4309 --------- Signed-off-by: Jamil <jamilbk@users.noreply.github.com> Co-authored-by: Reactor Scram <ReactorScram@users.noreply.github.com>
Discovered that tracing doesn't respect the
connlib_client_applelog filter, which makes it a bit confusing when logging debug in the Apple FFI module.Removing to prevent further casualties.
Extracted out of #4133