Skip to content
This repository has been archived by the owner on Mar 1, 2019. It is now read-only.

Improve logging #315

Open
11 tasks
denisshevchenko opened this issue Feb 6, 2019 · 1 comment
Open
11 tasks

Improve logging #315

denisshevchenko opened this issue Feb 6, 2019 · 1 comment
Labels
ACCEPTED ADR has been accepted

Comments

@denisshevchenko
Copy link
Contributor

denisshevchenko commented Feb 6, 2019

Context

Our logging situation is terrible and not helpful. A lot of Buildable-instances are not as useful as we'd want. So we will need to fix these instances for pretty much every type we use to make sure that we get the most information we can out of our logging.That also probably mean getting rid of this BuildableSafe we did a while ago and just, not output password and private keys in the Buildable-instance at all.

Decision

  1. Review all Buildable-instances and check what outputs we get from the logs.
  2. (Probably) get rid of BuildableSafe-instances by removing them with Buildable-instances.
  3. Add a few actual log lines in the wallet code, apart from the API calls.

Acceptance Criterias

  • All Buildable-instances must provide all information we need in logs.
  • Information which is printed in logs must be well-formatted.

Development Plan

Proposal - open a separate PR for each layer:

  1. Buildable-instances in API-layer.
  2. Buildable-instances in Wallet-layer.
  3. Buildable-instances in Kernel-layer.
  4. Buildable-instances in DB-layer.
  • Make sure all Buildable-instances in cardano-wallet produce useful information in logs.
    • Re-write existing Buildable-instances using utils from Cardano.Wallet.Util module.
  • Replace BuildableSafeGen-instances by corresponding Buildable-instances and make sure we don't print out passwords and private keys. BuildableSafeGen-instances must be removed from these modules:
    • Cardano.Wallet.API.Request
    • Cardano.Wallet.API.Request.Filter
    • Cardano.Wallet.API.Request.Sort
    • Cardano.Wallet.API.V1.Types
    • Cardano.Wallet.Types.UtxoStatistics
  • Add additional log messages we need.

PR

Number Base
#316 develop
#345 develop
#348 develop
#352 develop
#358 develop
#361 develop
#363 develop

QA

Criteria Coverage
? -

Retrospective

@akegalj
Copy link
Contributor

akegalj commented Feb 20, 2019

Replace BuildableSafeGen-instances by corresponding Buildable-instances and make sure we don't print out passwords and private keys. BuildableSafeGen-instances must be removed from these modules:

I am not sure this is what we want? We are going to lose privacy option for our logs which we do want in production setting. Some people wouldn't want their account names being leaked into logs

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
ACCEPTED ADR has been accepted
Projects
None yet
Development

No branches or pull requests

2 participants