Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

upgrade package log to 2024-11-04 #734

Merged
merged 37 commits into from
Nov 15, 2024
Merged

Conversation

gzliudan
Copy link
Collaborator

@gzliudan gzliudan commented Nov 15, 2024

Proposed changes

This PR upgrade the package to latest(2024-11-04). It includes the following PRs from geth:

The most important change is: all: replace log15 with slog (#28187). It introduces the following new flags:

name usage
log-vmodule Per-module verbosity: comma-separated list of <pattern>=<level> (e.g. eth/*=5,p2p=4)
log-json Format logs with JSON
log-format Log format to use (json|logfmt| terminal)
log-file Write logs to a file
log-rotate Enables log file rotation
log-maxsize Maximum size in MBs of a single log file (default: 100)
log-maxbackups Maximum number of log files to retain (default: 10)
log-maxage Maximum number of days to retain a log file (default: 30)
log-compress Compress the log files
log-debug Prepends log messages with call-site location (deprecated)
log-backtrace Request a stack trace at a specific logging statement (deprecated)

Types of changes

What types of changes does your code introduce to XDC network?
Put an in the boxes that apply

  • Bugfix (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 not work as expected)
  • Documentation Update (if none of the other choices apply)
  • Regular KTLO or any of the maintaince work. e.g code style
  • CICD Improvement

Impacted Components

Which part of the codebase this PR will touch base on,

Put an in the boxes that apply

  • Consensus
  • Account
  • Network
  • Geth
  • Smart Contract
  • External components
  • Not sure (Please specify below)

Checklist

Put an in the boxes once you have confirmed below actions (or provide reasons on not doing so) that

  • This PR has sufficient test coverage (unit/integration test) OR I have provided reason in the PR description for not having test coverage
  • Provide an end-to-end test plan in the PR description on how to manually test it on the devnet/testnet.
  • Tested the backwards compatibility.
  • Tested with XDC nodes running this version co-exist with those running the previous version.
  • Relevant documentation has been updated as part of this PR
  • N/A

gzliudan and others added 30 commits November 15, 2024 10:02
- Keep the tailing zeros.
- Limit precision to milliseconds.
* log: Do not pad too long values

* log: gofmt
docs: change http to https on links in log/README.md
* log: fix formatting of big.Int

The implementation of formatLogfmtBigInt had two issues: it crashed when
the number was actually large enough to hit the big integer case, and
modified the big.Int while formatting it.

* log: don't call FormatLogfmtInt64 for int16

* log: separate from decimals back, not front

Co-authored-by: Péter Szilágyi <[email protected]>
This modifies the order of Lock() defer Unlock() to follow the more
typically used pattern.
* log: allow tabs in log messages

This fixes a regression where panic reports in RPC handlers were quoted
because they contain tab characters.

* Update format.go
* log/format.go : invalid string cast fix

* log: some polish

---------

Co-authored-by: Martin Holst Swende <[email protected]>
These changes improves the performance of the non-coloured terminal formatting, _quite a lot_.

```
name               old time/op    new time/op    delta
TerminalHandler-8    10.2µs ±15%     5.4µs ± 9%  -47.02%  (p=0.008 n=5+5)

name               old alloc/op   new alloc/op   delta
TerminalHandler-8    2.17kB ± 0%    0.40kB ± 0%  -81.46%  (p=0.008 n=5+5)

name               old allocs/op  new allocs/op  delta
TerminalHandler-8      33.0 ± 0%       5.0 ± 0%  -84.85%  (p=0.008 n=5+5)
```

I tried to _somewhat_ organize the commits, but the it might still be a bit chaotic. Some core insights:

- The function `terminalHandler.Handl` uses a mutex, and writes all output immediately to 'upstream'. Thus, it can reuse a scratch-buffer every time.
- This buffer can be propagated internally, making all the internal formatters either write directly to it,
- OR, make  use of the `tmp := buf.AvailableBuffer()` in some cases, where a byte buffer "extra capacity" can be temporarily used.
- The `slog` package  uses `Attr` by value. It makes sense to minimize operating on them, since iterating / collecting into a new slice, iterating again etc causes copy-on-heap. Better to operate on them only once.
- If we want to do padding, it's better to copy from a constant `space`-buffer than to invoke `bytes.Repeat` every single time.
…m#28622)

This change

- Removes interface `log.Format`,
- Removes method `log.FormatFunc`,
- unexports `TerminalHandler.TerminalFormat` formatting methods (renamed to `TerminalHandler.format`)
- removes the notion of `log.Lazy` values

The lazy handler was useful in the old log package, since it
could defer the evaluation of costly attributes until later in the
log pipeline: thus, if the logging was done at 'Trace', we could
skip evaluation if logging only was set to 'Info'.

With the move to slog, this way of deferring evaluation is no longer
needed, since slog introduced 'Enabled': the caller can thus do
the evaluate-or-not decision at the callsite, which is much more
straight-forward than dealing with lazy reflect-based evaluation.

Also, lazy evaluation would not work with 'native' slog, as in, these
two statements would be evaluated differently:

```golang
  log.Info("foo", "my lazy", lazyObj)
  slog.Info("foo", "my lazy", lazyObj)
```
slog.SetDefault has undesirable side effects. It also sets the default logger destination,
for example. So we should not call it by default in init.
log: Add Handler getter to Logger interface
@gzliudan gzliudan merged commit 77b0c0b into XinFinOrg:dev-upgrade Nov 15, 2024
17 checks passed
@gzliudan gzliudan deleted the upgrade-log branch November 15, 2024 07:41
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Nov 21, 2024
gzliudan added a commit to gzliudan/XDPoSChain that referenced this pull request Nov 21, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants