-
Notifications
You must be signed in to change notification settings - Fork 1.2k
Switch logging from using prefixes to the new package path format #16059
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
base: develop
Are you sure you want to change the base?
Conversation
3b4668a to
6df53d0
Compare
6df53d0 to
a3a9f8c
Compare
a3a9f8c to
b0c7f11
Compare
.github/workflows/check-logs.yml
Outdated
|
|
||
| - name: Check log.go files | ||
| run: | | ||
| chmod +x hack/gen-logs.sh hack/check-logs.sh |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You shouldn't need to do this every time. Can you make the file executable in the git repo?
| # ---------- config ---------- | ||
| # Paths (relative to repo root) to exclude. | ||
| # Each entry excludes that directory AND all its subdirectories. | ||
| EXCLUDED_PATH_PREFIXES=( |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If this could also exclude paths listed in gitignore, then it would be somewhat better positioned for future directories to ignore
This PR sets the foundation for advanced log control: per package visibility and verbosity.
The goal of this big PR is the following:
Adding a log.go file to every package: commit
packageto the full path of that package.log_helpers.gofile within each package.Create a CI rule which verifies that: commit
packagefield of each log.go variable, has the correct path. (to detect when we move a package or change it's name)Alter the logging system to read the prefix from this
packagefield for every log while outputing: commitkv). This can be solved by keeping a map of package paths to prefix names somewhere.Some notes:
prefixReplacementmap 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.packagefield 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.hack/gen-logs.shand checks that the git diff is zero. that script ishack/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:
internal"
List of excluded directories (their subdirectories are also excluded):