Standardize logging when publishing a checkpoint#555
Conversation
| if err := a.s.overwrite(layout.CheckpointPath, cpRaw); err != nil { | ||
| return fmt.Errorf("overwrite(%s): %v", layout.CheckpointPath, err) | ||
| } | ||
| klog.Infof("Published latest checkpoint") |
There was a problem hiding this comment.
Rather than remove it entirely, how about making it a klog.V(1).Infof (or even V(2)) and include the tree size in the logged line?
There was a problem hiding this comment.
There was a problem hiding this comment.
The same info logging cannot be found in AWS and GCP storage implementation of publishCheckpoint. If this info log stays, should we add the same to the rest of the storage implementations?
There was a problem hiding this comment.
^ was the reason I chose to delete it, and there's already logging for when a new tree state is published with the new tree size - https://github.com/transparency-dev/trillian-tessera/blob/a60c8d23e801e11eb7511a623d144d6a7da3d0d4/storage/posix/files.go#L346
There was a problem hiding this comment.
Integration != Publishing in Tessera, and in fact these are asynchronous processes.
Tessera is not yet at a point in its maturity cycle where I'm happy for debug logging to be entirely removed, if there's a disparity between events logged in different storage backends I'd argue for adding similar logging in the other storage implementations as opposed to removing, as @roger2hk's comment above also suggests.
I am happy for debug logging to have its level moved up so that it can be enabled/disabled, which was the suggestion.
There was a problem hiding this comment.
@AlCutter Added logging for checkpoint publishing for all backends.
All storage backends will log when a new checkpoint is published. Tiny change to standardize logging message for integration as well. Signed-off-by: Hayden B <8418760+haydentherapper@users.noreply.github.com>
Hayden-IO
changed the title
| if err := a.s.overwrite(layout.CheckpointPath, cpRaw); err != nil { | ||
| return fmt.Errorf("overwrite(%s): %v", layout.CheckpointPath, err) | ||
| } | ||
| klog.Infof("Published latest checkpoint") |
3 participants
All storage backends will log when a new checkpoint is published.
Tiny change to standardize logging message for integration as well.