API V1 types, improved logging

[denisshevchenko]

#315

Overview

• I have improved logging format for API V1 types.

Comments

[uroboros]

mostly LGTM

[uroboros]

is this ok given that there can be many addrs?

[denisshevchenko]

Of course it's ok.

[uroboros]

but what if you log an account with 50k addresses?

[denisshevchenko]

Well, in this case we'll get a very big log. But we have a lot of different types with lists, and some of these lists can theoretically be really big. And now they are well-formatted, with indentation and newlines.

[KtorZ]

Do we have any example of the logs produced by those changes? I am skeptic about some of them to be honest by looking at the implementation 🤔

Beside, it'd be nice to make more smaller PRs for this kind of things. Showing the rendered output with each iteration.

[KtorZ]

cf comment above ^^^

[denisshevchenko]

Do we have any example of the logs produced by those changes?

I tested logging of all these API V1 types in ghci, just compare old Buildable-result with the new one. It's much better.

it'd be nice to make more smaller PRs for this kind of things.

But all these changes are in one single module, and they are very similar. I thought it's not bad to put it in one PR...

[KtorZ]

I tested logging of all these API V1 types in ghci, just compare old Buildable-result with the new one. It's much better.

What is much better ? This is the kind of elements we'd like to see in the PR's comments or, in the ticket. From a reviewer perspective, you get all the details you need to assess the change :)
So, what does it look like exactly?

But all these changes are in one single module, and they are very similar. I thought it's not bad to put it in one PR...

Well, yes and no 😕 ... It's still good to proceed by steps. I am not saying making one PR per type, that's too much. But we can focus on getting nice logs for the wallet operations for instance. And then, move on to a different set of operations. etc. Then, with each PR, we actually bring value to one particular set of endpoint, and it's much easier to review and integrate. We don't have to go from A to Z directly, every step in-between is worth taking.

[denisshevchenko]

These are some examples, from old log and from new one.

Logs for payment, old:

{ source={ walletId=<wallet id> accountIndex=2147483648 } destinations=[... (1 item(s))] groupingPolicty= spendingPassword=<wallet passphrase> }

and new:

Payment
  source - wallet id: Ae2tdPwUPEZKhRPWAiCodrtkGuSgZozxTGKRmvijxswR1zLvfYSSdLAad2c, account ix: 2147483648
  destinations:
  - payment distribution
      address: DdzFFzCqrhse61s45YYRVdhEDfpC3NTQpKukNipktpWt1i1pzdch7oSiu24Gvwx65jp56Sh5GJratddjor9akzWWdimt9WFETiJgRJvg
      amount: 1000111 coin(s)

---

Logs for new EOS-wallet creation, old:

{ accounts=[{ publicKey=pub:b573ec63 index=2147483648 }, { publicKey=pub:ff19e03a index=2147483649 }, { publicKey=pub:43a0c21f index=2147483650 }] addressPoolGap=Address pool gap 14 assuranceLevel=normal name=<text> }

and new:

Request for new EOS-wallet
  name: Fixture Wallet
  assurance level: normal
  accounts:
  - account public key: pub:dad4b752, account ix: 2147483648
  - account public key: pub:fc5c0e1e, account ix: 2147483649
  - account public key: pub:d8937bc4, account ix: 2147483650

---

Logs for new wallet creation, old:

{ backupPhrase=<backup phrase> spendingPassword=<wallet passphrase> assuranceLevel=normal name=<text> operationcreate }

and new:

New wallet
  operation: create
  name: Fixture Wallet
  assurance level: normal

---

Logs for new address creation, old:

{ spendingPassword=<wallet passphrase> accountIndex=2147483648 walletId=<wallet id> }

and new:

Request for new address
  wallet id: Ae2tdPwUPEZ29HBhX8D1DAv5bJhbGpxm1TW8tCc38hpL2W7wbuThBRm4Bwt
  account ix: 2147483648

[denisshevchenko]

Closed in favor of few smaller PRs.