[CBR-275] stack logging ontop of 'katip', provides structured logging #3533

CodiePP · 2018-09-01T08:15:43Z

Description

we adapt our abstract interface Pos.Util.Wlog and direct output to 'katip'.
this should allow a transition from 'log-warper' to 'katip' with minimal changes to the overall codebase.

this PR only contains changes to logging related code in 'util' and changes to other modules which need to be applied at the same time
especially, startup code and logging initialization in tests
PR [CBR-275] cleanup interface required for structured logging #3534 alrady adapted "minimally" the codebase
all references to 'log-warper' library have been removed; now, we only use 'katip' for logging

Linked issue

CBR-97 (user story)
CBR-275

Type of change

[~] 🐞 Bug fix (non-breaking change which fixes an issue)
[~] 🛠 New feature (non-breaking change which adds functionality)
[~] ⚠️ Breaking change (fix or feature that would cause existing functionality to change)
🏭 Refactoring that does not change existing functionality but does improve things like code readability, structure etc
🔨 New or improved tests for existing code
[~] ⛑ git-flow chore (backport, hotfix, etc)

Developer checklist

I have read the style guide document, and my code follows the code style of this project.
If my code deals with exceptions, it follows the guidelines.
[~] I have updated any documentation accordingly, if needed. Documentation changes can be reflected in opening a PR on cardanodocs.com, amending the inline Haddock comments, any relevant README file or one of the document listed in the docs directory.
[~] CHANGELOG entry has been added and is linked to the correct PR on GitHub.

Testing checklist

I have added tests to cover my changes.
All new and existing tests passed.

QA Steps

all tests in CI
benchmark in scripts/bench/runbench.sh
block syncing test

njd42

Can we split this into a "clean up interface" PR and a "first steps to Katip" PR?

This makes the changes (and importantly the potential interactions) clearer - the "clean up" should just be reducing the "surface area" - the replacement should just be the interface module (plus the essential changes to things like logging initialisation) as the PR sequence progresses

CodiePP · 2018-09-02T09:56:26Z

yes, moved interface adaptation to PR #3534

util/src/Pos/Util/Wlog.hs

erikd

Approved as is, but I do think it can be improved by removing the use of RecordWildCards there.

util/src/Pos/Util/Wlog.hs

util/test/Test/Pos/Util/LogSpec.hs

Large number of commits added since my review.

util/src/Pos/Util/Wlog/Compatibility.hs

CodiePP · 2018-09-07T18:15:24Z

next steps:

cleanup this PR by squashing commits
testing, testing

erikd · 2018-09-14T05:30:11Z

lib/test/Test/Pos/Diffusion/BlockSpec.hs

        , fdcConvEstablishTimeout = 15000000 -- us
        , fdcStreamWindow = streamWindow
-        , fdcTrace = wlogTrace ("client" <> "diffusion")
+        , fdcTrace = wlogTrace ("client.diffusion")


erikd

LGTM!

CodiePP · 2018-09-14T08:47:19Z

thank you @erikd for reviewing
rebased on develop again

Signed-off-by: Alexander Diemand <[email protected]>

… to 'cardano-sl' Signed-off-by: Alexander Diemand <[email protected]>

Signed-off-by: Alexander Diemand <[email protected]>

CodiePP · 2018-09-15T12:18:38Z

rebased on develop

Signed-off-by: Alexander Diemand <[email protected]>

…logging-interface [CBR-275] stack logging ontop of 'katip', provides structured logging

…hk/adiemand/CBR-275/mimic-logging-interface [CBR-275] stack logging ontop of 'katip', provides structured logging

CodiePP added wip DO NOT MERGE labels Sep 1, 2018

CodiePP assigned CodiePP and 365andreas Sep 1, 2018

CodiePP requested review from erikd and njd42 September 1, 2018 08:16

CodiePP force-pushed the adiemand/CBR-275/mimic-logging-interface branch 3 times, most recently from 1f63bbf to badf129 Compare September 2, 2018 09:41

njd42 reviewed Sep 2, 2018

View reviewed changes

CodiePP force-pushed the adiemand/CBR-275/mimic-logging-interface branch 3 times, most recently from 7d35904 to 9fea606 Compare September 2, 2018 14:15

erikd reviewed Sep 3, 2018

View reviewed changes

util/src/Pos/Util/Wlog.hs Outdated Show resolved Hide resolved

erikd reviewed Sep 3, 2018

View reviewed changes

util/src/Pos/Util/Wlog.hs Outdated Show resolved Hide resolved

erikd previously approved these changes Sep 3, 2018

View reviewed changes

CodiePP force-pushed the adiemand/CBR-275/mimic-logging-interface branch 2 times, most recently from 954c5ea to e8c06c0 Compare September 5, 2018 15:37

erikd reviewed Sep 6, 2018

View reviewed changes

util/src/Pos/Util/Wlog.hs Outdated Show resolved Hide resolved

erikd reviewed Sep 6, 2018

View reviewed changes

util/src/Pos/Util/Wlog.hs Outdated Show resolved Hide resolved

CodiePP force-pushed the adiemand/CBR-275/mimic-logging-interface branch 2 times, most recently from 5809c0a to 0bd6ee0 Compare September 6, 2018 00:53

CodiePP commented Sep 6, 2018

View reviewed changes

util/test/Test/Pos/Util/LogSpec.hs Outdated Show resolved Hide resolved

CodiePP commented Sep 6, 2018

View reviewed changes

util/src/Pos/Util/Wlog/Compatibility.hs Outdated Show resolved Hide resolved

365andreas force-pushed the adiemand/CBR-275/mimic-logging-interface branch 3 times, most recently from b9c0fdb to 5b208bb Compare September 7, 2018 12:48

erikd reviewed Sep 14, 2018

View reviewed changes

erikd approved these changes Sep 14, 2018

View reviewed changes

CodiePP force-pushed the adiemand/CBR-275/mimic-logging-interface branch from c26567e to acd8c64 Compare September 14, 2018 08:46

CodiePP force-pushed the adiemand/CBR-275/mimic-logging-interface branch 2 times, most recently from d684388 to 468b27d Compare September 14, 2018 14:24

Andreas Triantafyllos and others added 16 commits September 15, 2018 10:50

[CBR-207] Edit export list of Pos.Util.Wlog.

3c820e5

[CBR-275] counting call sites for logging functions

1d4f3ea

Signed-off-by: Alexander Diemand <[email protected]>

[CBR-275] mimicking the old logging interface with stubs; compiles up…

863668a

… to 'cardano-sl' Signed-off-by: Alexander Diemand <[email protected]>

[CBR-275] Pos.Util.Wlog testing

25acfdd

Signed-off-by: Alexander Diemand <[email protected]>

[CBR-275] Removed RecordWildCards.

1fe41be

[CBR-345] benchmark and wallet-integration-test adapted to new logging

a1e7e80

Signed-off-by: Alexander Diemand <[email protected]>

[CBR-275] Added compatible instances

bd838fe

[CBR-275] Changes for wallet-new & tools

787e1c1

[CBR-275] Pos.Util.Wlog cleanup in 'util'

7ffae84

Signed-off-by: Alexander Diemand <[email protected]>

[CBR-275] remove reference to log-warper

c928d72

Signed-off-by: Alexander Diemand <[email protected]>

[CBR-275] simplified LoggerName

8d398ab

Signed-off-by: Alexander Diemand <[email protected]>

[CBR-275] Revive tests and startup code

abd38c3

[CBR-275] wallet integration test: grep in new log files

22b12ad

Signed-off-by: Alexander Diemand <[email protected]>

[CBR-275] logging config in networking benchmarks

a079e6b

Signed-off-by: Alexander Diemand <[email protected]>

[CBR-275] modifyLoggerName reqs 'dot' when adding name to context

043d186

Signed-off-by: Alexander Diemand <[email protected]>

[CBR-275] default severity is 'Debug'

49000f3

Signed-off-by: Alexander Diemand <[email protected]>

CodiePP force-pushed the adiemand/CBR-275/mimic-logging-interface branch from 468b27d to 77f2e11 Compare September 15, 2018 12:17

[CBR-275] cleanup; revisit tests in 'util'

9365bcb

Signed-off-by: Alexander Diemand <[email protected]>

CodiePP force-pushed the adiemand/CBR-275/mimic-logging-interface branch from 77f2e11 to 9365bcb Compare September 15, 2018 12:25

CodiePP merged commit c1c3729 into develop Sep 15, 2018

CodiePP deleted the adiemand/CBR-275/mimic-logging-interface branch October 26, 2018 08:55

KtorZ pushed a commit that referenced this pull request Nov 9, 2018

Merge pull request #3533 from input-output-hk/adiemand/CBR-275/mimic-…

b6a5e31

…logging-interface [CBR-275] stack logging ontop of 'katip', provides structured logging

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

[CBR-275] stack logging ontop of 'katip', provides structured logging #3533

[CBR-275] stack logging ontop of 'katip', provides structured logging #3533

Uh oh!

CodiePP commented Sep 1, 2018 •

edited

Loading

Uh oh!

njd42 left a comment

Uh oh!

CodiePP commented Sep 2, 2018

Uh oh!

Uh oh!

Uh oh!

erikd left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CodiePP commented Sep 7, 2018

Uh oh!

erikd Sep 14, 2018

Uh oh!

erikd left a comment

Uh oh!

CodiePP commented Sep 14, 2018

Uh oh!

CodiePP commented Sep 15, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

[CBR-275] stack logging ontop of 'katip', provides structured logging #3533

[CBR-275] stack logging ontop of 'katip', provides structured logging #3533

Uh oh!

Conversation

CodiePP commented Sep 1, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Linked issue

Type of change

Developer checklist

Testing checklist

QA Steps

Uh oh!

njd42 left a comment

Choose a reason for hiding this comment

Uh oh!

CodiePP commented Sep 2, 2018

Uh oh!

Uh oh!

Uh oh!

erikd left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

CodiePP commented Sep 7, 2018

Uh oh!

erikd Sep 14, 2018

Choose a reason for hiding this comment

Uh oh!

erikd left a comment

Choose a reason for hiding this comment

Uh oh!

CodiePP commented Sep 14, 2018

Uh oh!

CodiePP commented Sep 15, 2018

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

CodiePP commented Sep 1, 2018 •

edited

Loading