mirror of
https://github.com/OffchainLabs/prysm.git
synced 2026-01-06 22:23:56 -05:00
6b5ba5ad01
#### This PR sets the foundation for the new logging features. --- The goal of this big PR is the following: 1. Adding a log.go file to every package: [_commit_](54f6396d4c) - Writing a bash script that adds the log.go file to every package that imports logrus, except the excluded packages, configured at the top of the bash script. - the log.go file creates a log variable and sets a field called `package` to the full path of that package. - I have tried to fix every error/problem that came from mass generation of this file. (duplicate declarations, different prefix names, etc...) - some packages had the log.go file from before, and had some helper functions in there as well. I've moved all of them to a `log_helpers.go` file within each package. 2. Create a CI rule which verifies that: [_commit_](b799c3a0ef) - every package which imports logrus, also has a log.go file, except the excluded packages. - the `package` field of each log.go variable, has the correct path. (to detect when we move a package or change it's name) - I pushed a commit with a manually changed log.go file to trigger the ci check failure and it worked. 3. Alter the logging system to read the prefix from this `package` field for every log while outputing: [_commit_](b0c7f1146c) - some packages have/want/need a different log prefix than their package name (like `kv`). This can be solved by keeping a map of package paths to prefix names somewhere. --- **Some notes:** - Please review everything carefully. - I created the `prefixReplacement` map and populated the data that I deemed necessary. Please check it and complain if something doesn't make sense or is missing. I attached at the bottom, the list of all the packages that used to use a different name than their package name as their prefix. - I have chosen to mark some packages to be excluded from this whole process. They will either not log anything, or log without a prefix, or log using their previously defined prefix. See the list of exclusions in the bottom. - I fixed all the tests that failed because of this change. These were failing because they were expecting the old prefix to be in the generated logs. I have changed those to expect the new `package` field instead. This might not be a great solution. Ideally we might want to remove this from the tests so they only test for relevant fields in the logs. but this is a problem for another day. - Please run the node with this config, and mention if you see something weird in the logs. (use different verbosities) - The CI workflow uses a script that basically runs the `hack/gen-logs.sh` and checks that the git diff is zero. that script is `hack/check-logs.sh`. This means that if one runs this script locally, it will not actually _check_ anything, rather than just regenerate the log.go files and fix any mistake. This might be confusing. Please suggest solutions if you think it's a problem. --- **A list of packages that used a different prefix than their package names for their logs:** - beacon-chain/cache/depositsnapshot/ package depositsnapshot, prefix "cache" - beacon-chain/core/transition/log.go — package transition, prefix "state" - beacon-chain/db/kv/log.go — package kv, prefix "db" - beacon-chain/db/slasherkv/log.go — package slasherkv, prefix "slasherdb" - beacon-chain/db/pruner/pruner.go — package pruner, prefix "db-pruner" - beacon-chain/light-client/log.go — package light_client, prefix "light-client" - beacon-chain/operations/attestations/log.go — package attestations, prefix "pool/attestations" - beacon-chain/operations/slashings/log.go — package slashings, prefix "pool/slashings" - beacon-chain/rpc/core/log.go — package core, prefix "rpc/core" - beacon-chain/rpc/eth/beacon/log.go — package beacon, prefix "rpc/beaconv1" - beacon-chain/rpc/eth/validator/log.go — package validator, prefix "beacon-api" - beacon-chain/rpc/prysm/v1alpha1/beacon/log.go — package beacon, prefix "rpc" - beacon-chain/rpc/prysm/v1alpha1/validator/log.go — package validator, prefix "rpc/validator" - beacon-chain/state/stategen/log.go — package stategen, prefix "state-gen" - beacon-chain/sync/checkpoint/log.go — package checkpoint, prefix "checkpoint-sync" - beacon-chain/sync/initial-sync/log.go — package initialsync, prefix "initial-sync" - cmd/prysmctl/p2p/log.go — package p2p, prefix "prysmctl-p2p" - config/features/log.go -- package features, prefix "flags" - io/file/log.go — package file, prefix "fileutil" - proto/prysm/v1alpha1/log.go — package eth, prefix "protobuf" - validator/client/beacon-api/log.go — package beacon_api, prefix "beacon-api" - validator/db/kv/log.go — package kv, prefix "db" - validator/db/filesystem/db.go — package filesystem, prefix "db" - validator/keymanager/derived/log.go — package derived, prefix "derived-keymanager" - validator/keymanager/local/log.go — package local, prefix "local-keymanager" - validator/keymanager/remote-web3signer/log.go — package remote_web3signer, prefix "remote-keymanager" - validator/keymanager/remote-web3signer/internal/log.go — package internal, prefix "remote-web3signer- internal" - beacon-chain/forkchoice/doubly... prefix is "forkchoice-doublylinkedtree" **List of excluded directories (their subdirectories are also excluded):** ``` EXCLUDED_PATH_PREFIXES=( "testing" "validator/client/testutil" "beacon-chain/p2p/testing" "beacon-chain/rpc/eth/config" "beacon-chain/rpc/prysm/v1alpha1/debug" "tools" "runtime" "monitoring" "io" "cmd" ".well-known" "changelog" "hack" "specrefs" "third_party" "bazel-out" "bazel-bin" "bazel-prysm" "bazel-testlogs" "build" ".github" ".jj" ".idea" ".vscode" ) ```
Prysm Feature Flags
Part of Prysm's feature development often involves use of feature flags which serve as a way to toggle new features as they are introduced. Using this methodology, you are assured that your feature can be safely tested in production with a fall back option if any regression were to occur. This reduces the likelihood of an emergency release or rollback of a given feature due to unforeseen issues.
When to use a feature flag?
It is best to use a feature flag any time you are adding or removing functionality in an existing critical application path.
Examples of when to use a feature flag:
- Adding a caching layer to an expensive RPC call.
- Optimizing a particular algorithm or routine.
- Introducing a temporary public testnet configuration.
Examples of when not to use a feature flag:
- Adding a new gRPC endpoint. (Unless dangerous or expensive to expose).
- Adding new logging statements.
- Removing dead code.
- Adding any trivial feature with no risk of regressions to existing functionality.
How to use a feature flag?
Once it has been decided that you should use a feature flag. Follow these steps to safely releasing your feature. In general, try to create a single PR for each step of this process.
- Add your feature flag to
shared/featureconfig/flags.go, use the flag to toggle a boolean in the feature config in shared/featureconfig/config.go. It is a good idea to use theenableprefix for your flag since you're going to invert the flag in a later step. i.e you will usedisableprefix later. For example,--enable-my-feature. Additionally, create a feature flag tracking issue for your feature using the appropriate issue template. - Use the feature throughout the application to enable your new functionality and be sure to write tests carefully and thoughtfully to ensure you have tested all of your new functionality without losing coverage on the existing functionality. This is considered an opt-in feature flag. Example usage:
func someExistingMethod(ctx context.Context) error {
if features.Get().MyNewFeature {
return newMethod(ctx)
}
// Otherwise continue with the existing code path.
}
- Add the flag to the end to end tests. This set of flags can also be found in shared/featureconfig/flags.go.
- Test the functionality locally and safely in production. Once you have enough confidence that your new function works and is safe to release then move onto the next step.
- Move your existing flag to the deprecated section of
shared/featureconfig/flags.go. It is important NOT to delete your existing flag outright. Deleting a flag can be extremely frustrating to users as it may break their existing workflow! Marking a flag as deprecated gives users time to adjust their start scripts and workflow. Add another feature flag to represent the inverse of your flag from step 1. For example--disable-my-feature. Read the value of this flag to appropriately the config value in shared/featureconfig/config.go. - After your feature has been included in a release as "opt-out" and there are no issues, deprecate the opt-out feature flag, delete the config field from shared/featureconfig/config.go, delete any deprecated / obsolete code paths.
Deprecated flags are deleted upon each major semver point release. Ex: v1, v2, v3.