chore(logger): Drop logger generic #2460

abi87 · 2025-02-01T04:45:54Z

Cleaned up the logger generics.
Apologies to @fridrik01 for stealing this

Signed-off-by: aBear <abear@berachain.com>

codecov · 2025-02-01T04:47:47Z

Codecov Report

Attention: Patch coverage is 0% with 112 lines in your changes missing coverage. Please review.

Project coverage is 32.36%. Comparing base (2309cc5) to head (25104bc).
Report is 1 commits behind head on main.

Files with missing lines	Patch %	Lines
cmd/beacond/defaults.go	0.00%	16 Missing ⚠️
consensus/cometbft/service/options.go	0.00%	12 Missing ⚠️
consensus/cometbft/service/service.go	0.00%	10 Missing ⚠️
cli/builder/options.go	0.00%	8 Missing ⚠️
node-core/builder/baseapp_options.go	0.00%	8 Missing ⚠️
cli/builder/builder.go	0.00%	7 Missing ⚠️
cmd/beacond/main.go	0.00%	5 Missing ⚠️
consensus/cometbft/service/log/cmt_logger.go	0.00%	5 Missing ⚠️
consensus/cometbft/service/log/sdk_logger.go	0.00%	5 Missing ⚠️
node-core/builder/builder.go	0.00%	5 Missing ⚠️
... and 26 more

Additional details and impacted files

@@           Coverage Diff           @@
##             main    #2460   +/-   ##
=======================================
  Coverage   32.36%   32.36%           
=======================================
  Files         350      350           
  Lines       15594    15592    -2     
  Branches       20       20           
=======================================
  Hits         5047     5047           
+ Misses      10184    10182    -2     
  Partials      363      363

Files with missing lines	Coverage Δ
cli/commands/setup.go	`0.00% <ø> (ø)`
consensus/cometbft/service/abci.go	`0.00% <ø> (ø)`
consensus/cometbft/service/prepare_proposal.go	`0.00% <ø> (ø)`
consensus/cometbft/service/process_proposal.go	`0.00% <ø> (ø)`
cli/commands/server/rollback.go	`0.00% <0.00%> (ø)`
cli/commands/server/start.go	`0.00% <0.00%> (ø)`
cli/config/server.go	`0.00% <0.00%> (ø)`
consensus/cometbft/cli/commands.go	`0.00% <0.00%> (ø)`
consensus/cometbft/service/commit.go	`0.00% <0.00%> (ø)`
consensus/cometbft/service/finalize_block.go	`0.00% <0.00%> (ø)`
... and 30 more

abi87 · 2025-02-01T05:29:03Z

consensus/cometbft/service/log/sdk_logger.go

 )

-type SDKLogger[LoggerT log.AdvancedLogger[LoggerT]] struct {
-	log.AdvancedLogger[LoggerT]
+type SDKLogger struct {


Todo: check if we can drop this type and what interface it is implementing (to assert it)

abi87 · 2025-02-01T05:29:09Z

consensus/cometbft/service/log/cmt_logger.go

 	cmtlog "github.com/cometbft/cometbft/libs/log"
 )

-type CometLogger[LoggerT log.AdvancedLogger[LoggerT]] struct {
-	log.AdvancedLogger[LoggerT]
+type CometLogger struct {


Todo: check if we can drop this type and what interface it is implementing (to assert it)

yebb I don't think CometLogger and SDKLogger are doing much as is

fridrik01 · 2025-02-01T15:02:25Z

@abi87 I already did this as part of https://github.com/berachain/beacon-kit-internal/pull/59, I didn't want to merge that in so soon before mainnet launch as I combined it with changing logger to use the standard slog logger

abi87 · 2025-02-01T15:13:48Z

@abi87 I already did this as part of berachain/beacon-kit-internal#59, I didn't want to merge that in so soon before mainnet launch as I combined it with changing logger to use the standard slog logger

I recall that. I think that PR is doing too much. I am down merging in the log generics cleanup quickly, I would take more care in switching the log library we use. Would you consider splitting your PR like this? I can close this one

abi87 · 2025-02-02T14:46:05Z

@abi87 I already did this as part of berachain/beacon-kit-internal#59, I didn't want to merge that in so soon before mainnet launch as I combined it with changing logger to use the standard slog logger

@fridrik01 here you are an interesting post about the use of interfaces in our logger!!

fridrik01 · 2025-02-02T16:45:12Z

@abi87 I already did this as part of berachain/beacon-kit-internal#59, I didn't want to merge that in so soon before mainnet launch as I combined it with changing logger to use the standard slog logger

@fridrik01 here you are an interesting post about the use of interfaces in our logger!!

lol, this makes the code so less readable.

fridrik01 · 2025-02-02T16:51:11Z

@abi87 I already did this as part of berachain/beacon-kit-internal#59, I didn't want to merge that in so soon before mainnet launch as I combined it with changing logger to use the standard slog logger

I recall that. I think that PR is doing too much. I am down merging in the log generics cleanup quickly, I would take more care in switching the log library we use. Would you consider splitting your PR like this? I can close this one

Nah, I think since you already did this again in a separate PR we go with that. You are right, these should be separate PRs anyway. Btw, the change from phuslu to slog is still something that would be nice to do eventually, but not the right time before mainnet launch

fridrik01

lgtm

fridrik01 · 2025-02-02T16:57:35Z

consensus/cometbft/service/log/cmt_logger.go

 	cmtlog "github.com/cometbft/cometbft/libs/log"
 )

-type CometLogger[LoggerT log.AdvancedLogger[LoggerT]] struct {
-	log.AdvancedLogger[LoggerT]
+type CometLogger struct {


yebb I don't think CometLogger and SDKLogger are doing much as is

rezbera

nice

Signed-off-by: aBear <abear@berachain.com>

abi87 added 14 commits January 31, 2025 21:45

dropped ctx from provide node api handlers

3f8af83

dropped ctx from provide node api beaconHandler

d2bb7d1

dropped ctx from provide node api builder handler

df8e2c2

dropped ctx from provide node api config handler

4d3bd1d

dropped ctx from provide debug api handler

5c8773f

dropped ctx from provide node api events handler

523163c

dropped ctx from provide node api handler

a706246

dropped ctx from provide node api proof handler

3626d32

dropped ctx from provide node api server

5cf537c

dropped ctx from server.Server

635801e

moved echo.Context to handlers package

7ade486

simplified handlers.Context

f91e75e

Signed-off-by: aBear <abear@berachain.com>

some more cleanup

7bedccc

Merge branch 'main' into drop-node-api-context-generic

2197831

abi87 self-assigned this Feb 1, 2025

abi87 had a problem deploying to test-e2e February 1, 2025 04:46 — with GitHub Actions Error

abi87 force-pushed the drop-logger-generic branch from b2b82ae to 416da88 Compare February 1, 2025 04:57

abi87 had a problem deploying to test-e2e February 1, 2025 04:57 — with GitHub Actions Error

abi87 force-pushed the drop-logger-generic branch from 416da88 to 9ee8e15 Compare February 1, 2025 05:01

abi87 had a problem deploying to test-e2e February 1, 2025 05:01 — with GitHub Actions Error

abi87 force-pushed the drop-logger-generic branch from 9ee8e15 to 08b5b09 Compare February 1, 2025 05:10

abi87 had a problem deploying to test-e2e February 1, 2025 05:10 — with GitHub Actions Error

wip: dropping logger generic

37893d9

abi87 force-pushed the drop-logger-generic branch from 08b5b09 to 37893d9 Compare February 1, 2025 05:19

abi87 temporarily deployed to test-e2e February 1, 2025 05:20 — with GitHub Actions Inactive

abi87 commented Feb 1, 2025

View reviewed changes

abi87 marked this pull request as ready for review February 1, 2025 05:39

abi87 requested a review from a team as a code owner February 1, 2025 05:39

abi87 mentioned this pull request Feb 2, 2025

fix: remove redundant *LoggerConfig type from function signature #2450

Closed

Base automatically changed from drop-node-api-context-generic to main February 2, 2025 15:02

Merge branch 'main' into drop-logger-generic

f589625

abi87 temporarily deployed to test-e2e February 2, 2025 15:06 — with GitHub Actions Inactive

fridrik01 approved these changes Feb 2, 2025

View reviewed changes

rezbera approved these changes Feb 2, 2025

View reviewed changes

Merge branch 'main' into drop-logger-generic

25104bc

abi87 temporarily deployed to test-e2e February 3, 2025 02:43 — with GitHub Actions Inactive

abi87 merged commit 2271084 into main Feb 3, 2025
19 checks passed

abi87 deleted the drop-logger-generic branch February 3, 2025 02:56

shotes pushed a commit that referenced this pull request Feb 3, 2025

chore(logger): Drop logger generic (#2460)

3369db1

Signed-off-by: aBear <abear@berachain.com>

shotes pushed a commit that referenced this pull request Feb 3, 2025

chore(logger): Drop logger generic (#2460)

dd5975e

Signed-off-by: aBear <abear@berachain.com>

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

chore(logger): Drop logger generic #2460

chore(logger): Drop logger generic #2460

abi87 commented Feb 1, 2025 •

edited

Loading

codecov bot commented Feb 1, 2025 •

edited

Loading

abi87 Feb 1, 2025

abi87 Feb 1, 2025

fridrik01 Feb 2, 2025

fridrik01 commented Feb 1, 2025 •

edited

Loading

abi87 commented Feb 1, 2025

abi87 commented Feb 2, 2025

fridrik01 commented Feb 2, 2025 •

edited

Loading

fridrik01 commented Feb 2, 2025

fridrik01 left a comment

fridrik01 Feb 2, 2025

rezbera left a comment

chore(logger): Drop logger generic #2460

chore(logger): Drop logger generic #2460

Conversation

abi87 commented Feb 1, 2025 • edited Loading

codecov bot commented Feb 1, 2025 • edited Loading

Codecov Report

abi87 Feb 1, 2025

Choose a reason for hiding this comment

abi87 Feb 1, 2025

Choose a reason for hiding this comment

fridrik01 Feb 2, 2025

Choose a reason for hiding this comment

fridrik01 commented Feb 1, 2025 • edited Loading

abi87 commented Feb 1, 2025

abi87 commented Feb 2, 2025

fridrik01 commented Feb 2, 2025 • edited Loading

fridrik01 commented Feb 2, 2025

fridrik01 left a comment

Choose a reason for hiding this comment

fridrik01 Feb 2, 2025

Choose a reason for hiding this comment

rezbera left a comment

Choose a reason for hiding this comment

abi87 commented Feb 1, 2025 •

edited

Loading

codecov bot commented Feb 1, 2025 •

edited

Loading

fridrik01 commented Feb 1, 2025 •

edited

Loading

fridrik01 commented Feb 2, 2025 •

edited

Loading