View Issue Details

IDProjectCategoryView StatusLast Update
0006557Talerwallet (Android App)public2020-10-03 12:37
ReporterFlorian Dold Assigned Togrote  
PrioritynormalSeveritytweakReproducibilityhave not tried
Status resolvedResolutionfixed 
Product Versiongit (master) 
Target Version0.8Fixed in Version0.8 
Summary0006557: implement "log view" for notifications from wallet-core
DescriptionThe wallet's notifications are likely rather useful in diagnosing problems. We should consider having a *very* simple view that allows us to view the last (~hundred?) notifications.

Each row should be expandable to show the full JSON. Since this feature is mainly meant for diagnostics, it can be hidden either in the settings or behind the developer mode. Also, we should *not* spend any effort on semantically rendering the notifications.

The only "nicety" we might want to have is a filter based on the notification type.
TagsNo tags attached.

Activities

grote

2020-09-07 13:59

developer   ~0016899

What's the use-case envisioned for this feature? Wouldn't an adb logcat be able to achieve the same without need to modify the app?

Florian Dold

2020-09-07 14:07

manager   ~0016901

We won't be able to logcat into the devices where an error might occur during the first BFH demo run. These will be students' devices, not our own!

Having a way to quickly view issues would be be helpful.

grote

2020-09-07 14:10

developer   ~0016902

And these issues are expected to not be reproducible with a device where we can do logcat?

Florian Dold

2020-09-07 14:15

manager   ~0016903

Yes! The wallet's (database) state is rather complex, and the "interesting" issues likely won't be that easy to reproduce. In most situations we wouldn't even exactly know what operation failed, so how could we reproduce it?

Christian Grothoff

2020-09-07 14:49

manager   ~0016908

I like it. I think it should only be accessible via developer mode *or* by clicking an active error notification snack bar. So invisible to users where everything just works, but if a user gets an error, they can pull the full diagnostics out at a single click.

Christian Grothoff

2020-09-07 14:49

manager   ~0016909

Alas, probably 0.8.1 unless Torsten is really fast ;-).

grote

2020-09-07 19:04

developer   ~0016913

I added a log viewer to the wallet (visible in dev mode) in 663d264. There don't seem to be any good libraries for that, so I used the most usable one. Not entirely happy with it, but instead of writing my own library, this one should give us what we need for now.

Please let me know if this works for you.

Florian Dold

2020-09-08 10:56

manager   ~0016915

Being able to export the logs from within is definitely useful. I'm happily surprised that this is actually possible from within the Android app itself! However, after playing around with a bit, I don't think it's that helpful in diagnosing wallet issues on the spot: There are a lot of log entries, many of them not relevant to the wallet directly, and due to the limited screen space and log headers the relevant JSON is not readable at all.

In order to get more actionable information for what went wrong, what I actually wanted to see is a list view with messages (responses, errors, notifications) collected from the wallet backend. To do this, two things are required:

(1) The WalletBackendApi instance should keep the last N messages it received in a Deque (reads/writes must obtain a lock first).
(2) The UI should display a *copy* (again obtained while holding the lock) of that list in a list view (maybe with the JSON expanded/collapsed on touch as it tends to be large). It should *not* automatically update.

[1] https://git.taler.net/taler-android.git/tree/wallet/src/main/java/net/taler/wallet/backend/WalletBackendApi.kt#n80

grote

2020-09-08 13:41

developer   ~0016917

> There are a lot of log entries, many of them not relevant to the wallet directly

Here, I only see akono notification retry spam which I don't think has any actual purpose, so they could be removed. But there's also a filter option to find specific things.

> and due to the limited screen space and log headers the relevant JSON is not readable at all.

Yeah, I tried to shorten the strings at the beginning, but the library doesn't offer that option. It helped here to rotate the phone into landscape mode.

> In order to get more actionable information for what went wrong, what I actually wanted to see is a list view with messages (responses, errors, notifications) collected from the wallet backend.

Isn't that exactly what this already does? To cut down on spam (which I don't like either in the log), you can enter "taler-wallet-backend" in the filter field for example.

> The WalletBackendApi instance should keep the last N messages it received in a Deque (reads/writes must obtain a lock first).

I don't get why we need a separate logging infrastructure when we already have one that was made exactly for that purpose and has several readers available (in-app, command-line, Android-Studio). This is what devs are used to work with and now it can even be shared by normal users.

> It should *not* automatically update.

Yeah I don't like that part either, but there isn't an off switch. I could try to increase the sampling frequency to something really large.

Florian Dold

2020-09-08 14:27

manager   ~0016919

Does this log facility help us to quickly see what issues the wallet is having? As far as I can see, it doesn't.

We're not building new 'logging infrastructure' here, we're just surfacing notifications from the wallet-core to the advanced users. Advanced users here are mostly people setting up their own infrastructure. Having the notifications+errors view is a cheap way to allow them to see potential errors in their setup.

grote

2020-09-09 19:03

developer   ~0016951

the 0.8 rc2 release comes with an improved logger, Christian please test this in landscape orientation, if it aids debugging

Christian Grothoff

2020-09-09 20:58

manager   ~0016953

Well, of course _now_ everything works fine here ;-). Maybe call it fixed and we open a new one if we find this is inadequate?

Issue History

Date Modified Username Field Change
2020-09-03 11:50 Florian Dold New Issue
2020-09-03 11:50 Florian Dold Status new => assigned
2020-09-03 11:50 Florian Dold Assigned To => grote
2020-09-05 13:53 Christian Grothoff Severity minor => tweak
2020-09-06 12:39 Florian Dold Target Version => 0.8
2020-09-06 12:39 Florian Dold Description Updated View Revisions
2020-09-06 13:07 Florian Dold Summary consider "log view" for notifications from wallet-core => implement "log view" for notifications from wallet-core
2020-09-07 13:59 grote Note Added: 0016899
2020-09-07 14:07 Florian Dold Note Added: 0016901
2020-09-07 14:10 grote Note Added: 0016902
2020-09-07 14:15 Florian Dold Note Added: 0016903
2020-09-07 14:49 Christian Grothoff Note Added: 0016908
2020-09-07 14:49 Christian Grothoff Target Version 0.8 => 0.8.1
2020-09-07 14:49 Christian Grothoff Note Added: 0016909
2020-09-07 19:04 grote Note Added: 0016913
2020-09-08 10:56 Florian Dold Note Added: 0016915
2020-09-08 13:41 grote Note Added: 0016917
2020-09-08 14:27 Florian Dold Note Added: 0016919
2020-09-09 19:03 grote Note Added: 0016951
2020-09-09 20:58 Christian Grothoff Note Added: 0016953
2020-09-24 15:08 grote Status assigned => resolved
2020-09-24 15:08 grote Resolution open => fixed
2020-10-03 12:37 Christian Grothoff Product Version => git (master)
2020-10-03 12:37 Christian Grothoff Fixed in Version => 0.8
2020-10-03 12:37 Christian Grothoff Target Version 0.8.1 => 0.8