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

android: send Android logs to logz #515

Merged
merged 1 commit into from
Sep 24, 2024
Merged

android: send Android logs to logz #515

merged 1 commit into from
Sep 24, 2024

Conversation

kari-ts
Copy link
Collaborator

@kari-ts kari-ts commented Sep 19, 2024

ExtendedLog sends log messages to Android's logcat and Tailscale's logger Libtailscale wrapper is a Kotlin wrapper that allows us to get around the problems with mocking a native library

Fixes tailscale/corp#23191

@agottardo
Copy link
Collaborator

Can we rename ExtendedLog to TSLog or TsLog? It is less stuff to type, and would match what we use in Swift :)

@kari-ts kari-ts force-pushed the kari/androidlog branch 2 times, most recently from 8040c04 to 95ce37f Compare September 19, 2024 21:40
@@ -59,7 +60,7 @@ object Notifier {

@OptIn(ExperimentalSerializationApi::class)
fun start(scope: CoroutineScope) {
Log.d(TAG, "Starting")
TSLog.d(TAG, "Starting")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
TSLog.d(TAG, "Starting")
TSLog.d(TAG, "Starting Notifier")

@@ -89,7 +90,7 @@ object Notifier {
}

fun stop() {
Log.d(TAG, "Stopping")
TSLog.d(TAG, "Stopping")
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
TSLog.d(TAG, "Stopping")
TSLog.d(TAG, "Stopping Notifier")

import libtailscale.Libtailscale

object TSLog {
var libtailscaleWrapper = LibtailscaleWrapper()
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

private?

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this can't be private because we use it in TimeUtilTest

import android.util.Log
import libtailscale.Libtailscale

object TSLog {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's document why this exists and when it should be used in place of the util.Log.

@@ -169,3 +171,13 @@ func RequestVPN(service IPNService) {
func ServiceDisconnect(service IPNService) {
onDisconnect <- service
}

func SendLog(logstr string) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be more efficient to use a []byte or byteArray here similar to what we did going the other way for localAPI? That might avoid a bunch of object copies.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

very good idea. done.

Log.e(tag, message, throwable)
libtailscaleWrapper.sendLog(tag, "$message ${throwable?.localizedMessage}")
}
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In swift, we have the notion of debug logs (which only show up when you do local debug builds) and verbose logs, which sit behind a flag you can toggle at build or runtime.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh nice, this is a good idea. I think we can use Proguard to strip out logs for release builds, so maybe we could use that to strip verbose/debug logs? or we can specify in build flavors in our build.gradle. I'll do this in a follow up

TSLog sends log messages to Android's logcat and Tailscale's logger
Libtailscale wrapper is a Kotlin wrapper that allows us to get around the problems with mocking a native library

Fixes tailscale/corp#23191

Signed-off-by: kari-ts <[email protected]>
@kari-ts kari-ts merged commit 08ae018 into main Sep 24, 2024
4 checks passed
@kari-ts kari-ts deleted the kari/androidlog branch September 24, 2024 18:25
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.

3 participants