-
Notifications
You must be signed in to change notification settings - Fork 6
Databricks bazel 7.6.1 logging #22
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: databricks-bazel-7.6.1
Are you sure you want to change the base?
Conversation
| if (DB_LOG_CALL_RESULT) { | ||
| sb.append(" result: ").append(result).append("\n"); | ||
| } | ||
| System.err.print(sb); |
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.
I know that this is dubious because of many things, but I didn't want to start messing with Reportable events. Just wanted to do something small that'll help me with debugging Starlark
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.
While I believe that this is useful, I'd rather not merge this as-is:
- the feature uses environment variables for configuration instead of command line flags
- the information is printed in an unstructured or at least non-standard format which will make it hard to reuse
- the implementation creates a custom language for filtering the log events rather than use something standard
I believe this can (and should) be implemented outside of Bazel with a custom client to the Bazel Starlark Debugging Protocol. That would also allow extending the log without having to make a new Bazel release. The debugging protocol already supports using Starlark expressions for conditional breakpoints which could be used for filtering.
There also appears to be an additional feature enabled with the DB_LOG_TOOLCHAIN_FEATURES environment variable that isn't documented in the PR description. It also prints to stderr. If the information is generally useful, it should probably go to the profile instead. Of course, this should also be a command line flag instead of an environment variable.
Added logging of Starlark function calls with very basic filters:
params- log all passed parameterscallstack- log callstackresult- log call result;as a separatorand_set_of_paramspresent in the callAn example:
This means that
cc_toolchaincalls_is_enabledcalls which havemodule_mapsorheader_modulesvalues among parametersAn example of the output: