More debug output #190
Merged
More debug output #190
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
It's useful to be able to run with debug logging, which you can do now
with:
make run LOG_LEVEL=debug
As a companion change, logging from `make run` is now explicitly set
to console format, since the expectation is it will be run from a
console.
Signed-off-by: Michael Bridgen <[email protected]>
This commit finesses the use of the debug log a little, and introduces a trace log. The trace log gets threaded through calls to utility procedures -- it's a little awkward putting loggers into func parameters and structs, but it always is. Signed-off-by: Michael Bridgen <[email protected]>
squaremo
force-pushed
the
more-debug-output
branch
from
a42038a
to
f011f23
Compare
hiddeco
reviewed
Jun 23, 2021
| @@ -229,7 +239,7 @@ func (r *ImageUpdateAutomationReconciler) Reconcile(ctx context.Context, req ctr | |||
| } | |||
| } | |||
|
|
|||
| log.V(debug).Info("cloned git repository", "gitrepository", originName, "ref", ref, "working", tmp) | |||
| debuglog.Info("cloned git repository", "gitrepository", originName, "ref", ref, "working", tmp) | |||
There was a problem hiding this comment.
Wouldn't it be better to log this before the attempt, so that it automatically adds context to any error that may happen during the attempt?
Reason I think this is a better approach, is because reconcilers generally do 3 things while performing operations for a resource:
- Logging (to tell what is about to happen, or highlight important information during an action).
- Events, to make certain things that have happened easily accessible to users and machines for observation.
- Conditions, to advertise observed state. Most often recorded after a reconciler has run an action that changed state, or when it inspected it.
There is some overlap in 1 and 2, but what distinguishes it for me is that 2 in all scenarios I have seen thus far, it never happens before something has taken place, which means that it would be a better task for logging.
There was a problem hiding this comment.
Yes it would, and that is a well-reasoned insight 👍
- trace different code paths, e.g., how the push branch is chosen - move debug output so it records things not already covered by e.g., errors, events Signed-off-by: Michael Bridgen <[email protected]>
Closed
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This adds trace-level messages to the log, in particular for