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

Refactor logging component #696

Closed
cthulhu-rider opened this issue Jul 13, 2021 · 4 comments
Closed

Refactor logging component #696

cthulhu-rider opened this issue Jul 13, 2021 · 4 comments
Assignees
Labels
enhancement Improving existing functionality
Milestone

Comments

@cthulhu-rider
Copy link
Contributor

cthulhu-rider commented Jul 13, 2021

We use zap logger as a basis of app logger. Currently Logger type is an alias, but this does not seem desirable from the side of the application code. I propose to provide a specific API built on the built-in-Go and NeoFS-specific types, and close zap in the implementation. This will make it possible to complete the use of a wide range of zap features to those that are required specifically for our applications. Also, when changing zap API or switching to a new fundamental basis, the application code will not be affected.

The similar approach was performed for the config types based on viper.

@cthulhu-rider cthulhu-rider added enhancement Improving existing functionality triage labels Jul 13, 2021
@cthulhu-rider
Copy link
Contributor Author

Also:

  1. underline meaningful log messages (object lifetime, etc.)
  2. per-component log (in config)

cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Oct 13, 2022
There is a need to dynamically parameterize `Logger` instances. For
example, change severity level.

Make `Logger` to encapsulate `zap.Logger` in order to prevent direct
assignments. Declare `Level` enumeration for message priorities. Replace
`Prm` type with `Config`. Add `Init` method which binds dynamic
configuration component which can be later used to modify parameters on
the fly.

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
@cthulhu-rider
Copy link
Contributor Author

In #1832 I was visited by the idea to unify some widely-used fields of the context. For example, provide

func FieldContainer(cnr cid.ID) Field {
  return FieldStringer("container", cnr)
}

func FieldEpoch(epoch uint64) Field {
  return FieldUint("epoch", epoch)
}

We can also add optional name string parameter to get field like "epoch (previous)": 100.

Also we can cover widely-used formats, e.g.

func FieldStringHex(name string, val string) Field {
  return FieldString(name, hex.EncodeToString(val))
}

func FieldStringBase58(name string, val string) Field {
  return FieldString(name, base58(val))
}

cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Oct 20, 2022
There is a need to dynamically parameterize `Logger` instances. For
example, change severity level.

Make `Logger` to encapsulate `zap.Logger` in order to prevent direct
assignments. Declare `Level` enumeration for message priorities. Replace
`Prm` type with `Config`. Add `Init` method which binds dynamic
configuration component which can be later used to modify parameters on
the fly.

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
cthulhu-rider pushed a commit to cthulhu-rider/neofs-node that referenced this issue Oct 20, 2022
There is a need to dynamically parameterize `Logger` instances. For
example, change severity level.

Make `Logger` to encapsulate `zap.Logger` in order to prevent direct
assignments. Declare `Level` enumeration for message priorities. Replace
`Prm` type with `Config`. Add `Init` method which binds dynamic
configuration component which can be later used to modify parameters on
the fly.

Signed-off-by: Leonard Lyubich <ctulhurider@gmail.com>
@roman-khimov
Copy link
Member

Another proposal: drop pkg/util/logger, use zap and be done with it.

@roman-khimov roman-khimov added this to the v0.38.0 milestone May 11, 2023
@cthulhu-rider
Copy link
Contributor Author

lets use zap everywhere until other needs

@cthulhu-rider cthulhu-rider self-assigned this Jul 10, 2023
cthulhu-rider added a commit to cthulhu-rider/neofs-node that referenced this issue Jul 21, 2023
Nothing foreshadows leaving zap logger in the near future.

Closes nspcc-dev#696.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
cthulhu-rider added a commit to cthulhu-rider/neofs-node that referenced this issue Jul 21, 2023
Nothing foreshadows leaving zap logger in the near future.

Closes nspcc-dev#696.

Signed-off-by: Leonard Lyubich <leonard@morphbits.io>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement Improving existing functionality
Projects
None yet
Development

No branches or pull requests

2 participants