Logger module #164 #301

okdas · 2022-10-11T23:43:47Z

Description

Logger module addition. The module will allow us to utilize structured logs for better observability.

Issue

Fixes #164

Type of change

Please mark the relevant option(s):

List of changes

New module addition

Testing

make develop_test
LocalNet w/ all of the steps outlined in the README

Required Checklist

I have performed a self-review of my own code
I have commented my code, particularly in hard-to-understand areas
I have tested my changes using the available tooling
I have updated the corresponding CHANGELOG

If Applicable Checklist

I have updated the corresponding README(s); local and/or global
I have added tests that prove my fix is effective or that my feature works
I have added, or updated, mermaid.js diagrams in the corresponding README(s)
I have added, or updated, documentation and mermaid.js diagrams in shared/docs/* if I updated shared/*README(s)

build/config/config1.json

logger/proto/logger_config.proto

deblasis

I like where this is going, just a few comments and happy to help if you have questions

app/client/main.go

build/config/config1.json

shared/modules/logger_module.go

codecov-commenter · 2022-10-18T01:31:22Z

Codecov Report

Base: 38.53% // Head: 38.53% // No change to project coverage 👍

Coverage data is based on head (4131d40) compared to base (fbb14a4).
Patch has no changes to coverable lines.

Additional details and impacted files

@@           Coverage Diff           @@
##             main     #301   +/-   ##
=======================================
  Coverage   38.53%   38.53%           
=======================================
  Files          33       33           
  Lines        2629     2629           
=======================================
  Hits         1013     1013           
  Misses       1556     1556           
  Partials       60       60

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

Olshansk · 2022-10-18T21:09:51Z

@okdas Let me know if/when you need another set of 👀 at this

okdas · 2022-10-19T01:19:43Z

@okdas Let me know if/when you need another set of 👀 at this

I wanted to ask Alessandro for a second review, but if you're available then sure, I'd love to get your feedback!

Olshansk

Left a couple small comments, but overall it's looking really clean!

logger/proto/logger_config.proto

shared/modules/logger_module.go

shared/modules/module.go

logger/module.go

consensus/consensus_tests/utils_test.go

shared/modules/logger_module.go

shared/modules/module.go

logger/module.go

logger/proto/logger_config.proto

app/client/cli/debug.go

Olshansk

@deblasis Could you take a look at this when you have a chance as well?

shared/modules/types.go

runtime/config.go

logger/proto/logger_config.proto

app/client/cli/debug.go

logger/module.go

Olshansk · 2022-11-07T18:03:15Z

@okdas Please re-request a review when this is ready again.

Also, please update the description of title in this PR since it's in review and not just a WIP anymore.

okdas · 2022-11-08T18:33:57Z

@deblasis please take a look at this if you've got a moment. I want to make sure I'm utilizing the foundation you've built correctly.

deblasis

Solid work, just a question that potentially is not an action item, please let me know if you think that I am getting it wrong.

Everything else, I believe that when the rubber hits the road in #288 there might be (or not) some small changes to this but the foundation looks pretty good to me.

shared/modules/logger_module.go

Olshansk

👀 👌

Really appreciate the scoping of this so we could extend it elsewhere afterwards!

okdas added 3 commits October 6, 2022 15:55

only complain about issues related to the pr

95da48f

logger module wip

6da4ec7

wip - remove or fix enums

af77cd8

okdas requested a review from a team as a code owner October 11, 2022 23:43

okdas self-assigned this Oct 11, 2022

okdas added core Core infrastructure - protocol related code health Nice to have code improvement priority:medium tooling tooling to support development, testing et al observability labels Oct 11, 2022

okdas added this to the M2: Pocket DoS (Devnet of Servicers) milestone Oct 11, 2022

can run client now

ccc1904

okdas commented Oct 12, 2022

View reviewed changes

build/config/config1.json Show resolved Hide resolved

okdas commented Oct 12, 2022

View reviewed changes

logger/proto/logger_config.proto Show resolved Hide resolved

deblasis self-requested a review October 12, 2022 07:13

deblasis reviewed Oct 12, 2022

View reviewed changes

app/client/main.go Outdated Show resolved Hide resolved

build/config/config1.json Show resolved Hide resolved

shared/modules/logger_module.go Show resolved Hide resolved

okdas and others added 2 commits October 17, 2022 17:20

Merge branch 'main' into issues/164/observability-logs

3d8f32e

utilizing runtimemgr

8dd13b1

🎉 client works!

afb2b75

okdas force-pushed the issues/164/observability-logs branch from e3d682c to afb2b75 Compare October 19, 2022 01:17

okdas requested a review from Olshansk October 19, 2022 01:19

Olshansk requested changes Oct 20, 2022

View reviewed changes

okdas added 2 commits October 24, 2022 10:41

Merge branch 'main' into issues/164/observability-logs

39268e5

add logger to CreateBus

0ed85d4

okdas force-pushed the issues/164/observability-logs branch from 8dc7c6b to 0ed85d4 Compare October 25, 2022 00:41

okdas changed the title ~~[WIP] logging module #164~~ [WIP] logger module #164 Oct 25, 2022

okdas requested a review from Olshansk October 25, 2022 00:42

disable golangci even for new code changes until we fix existing issues

b683af9

Olshansk requested a review from deblasis October 25, 2022 17:46

Olshansk requested changes Oct 25, 2022

View reviewed changes

okdas mentioned this pull request Nov 3, 2022

[Shared][CodeHealth] Allow to create only Initializable modules #333

Closed

6 tasks

okdas and others added 3 commits November 2, 2022 17:59

Follow protobuf enum best practices, map values into zerolog levels

2fb7aeb

Merge branch 'main' into issues/164/observability-logs

cc2e106

fix client after merge

8e5adc6

okdas commented Nov 4, 2022

View reviewed changes

app/client/cli/debug.go Show resolved Hide resolved

okdas requested a review from Olshansk November 4, 2022 00:58

Olshansk requested changes Nov 4, 2022

View reviewed changes

okdas force-pushed the issues/164/observability-logs branch 2 times, most recently from 0ecbe4d to cc465fa Compare November 4, 2022 21:36

okdas changed the title ~~[WIP] logger module #164~~ Logger module #164 Nov 7, 2022

resolved requested changes

137ca8e

okdas force-pushed the issues/164/observability-logs branch from cc465fa to 137ca8e Compare November 8, 2022 18:28

okdas requested a review from Olshansk November 8, 2022 18:32

deblasis approved these changes Nov 8, 2022

View reviewed changes

shared/modules/logger_module.go Show resolved Hide resolved

jessicadaugherty mentioned this pull request Nov 9, 2022

[Telemetry] Apply shared logging module #288

Closed

9 tasks

Olshansk approved these changes Nov 9, 2022

View reviewed changes

Merge branch 'main' into issues/164/observability-logs

dfce8fa

okdas merged commit a53a104 into main Nov 21, 2022

okdas deleted the issues/164/observability-logs branch November 21, 2022 22:23

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Logger module #164 #301

Logger module #164 #301

okdas commented Oct 11, 2022 •

edited

Loading

deblasis left a comment

codecov-commenter commented Oct 18, 2022 •

edited

Loading

Olshansk commented Oct 18, 2022

okdas commented Oct 19, 2022

Olshansk left a comment

Olshansk left a comment

Olshansk commented Nov 7, 2022

okdas commented Nov 8, 2022

deblasis left a comment

Olshansk left a comment

Logger module #164 #301

Logger module #164 #301

Conversation

okdas commented Oct 11, 2022 • edited Loading

Description

Issue

Type of change

List of changes

Testing

Required Checklist

If Applicable Checklist

deblasis left a comment

Choose a reason for hiding this comment

codecov-commenter commented Oct 18, 2022 • edited Loading

Codecov Report

Olshansk commented Oct 18, 2022

okdas commented Oct 19, 2022

Olshansk left a comment

Choose a reason for hiding this comment

Olshansk left a comment

Choose a reason for hiding this comment

Olshansk commented Nov 7, 2022

okdas commented Nov 8, 2022

deblasis left a comment

Choose a reason for hiding this comment

Olshansk left a comment

Choose a reason for hiding this comment

okdas commented Oct 11, 2022 •

edited

Loading

codecov-commenter commented Oct 18, 2022 •

edited

Loading