-
Notifications
You must be signed in to change notification settings - Fork 1k
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
Revise blob sidecar not found log #13571
Conversation
@@ -80,7 +80,7 @@ func (s *Service) blobSidecarByRootRPCHandler(ctx context.Context, msg interface | |||
sc, err := s.cfg.blobStorage.Get(root, idx) | |||
if err != nil { | |||
if db.IsNotFound(err) { | |||
log.WithError(err).Debugf("BlobSidecar not found in db, root=%x, index=%d", root, idx) | |||
log.WithError(err).Debugf("Serving blob sidecar request, not found in db, root=%x, index=%d", root, idx) |
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 the goal is to make it clear that this is untrusted input, we could make that a little more clear like "Peer requested blob sidecar by root which was not found in db".
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.
Done. Thanks!
92466e6
to
426e2df
Compare
@@ -80,7 +80,7 @@ func (s *Service) blobSidecarByRootRPCHandler(ctx context.Context, msg interface | |||
sc, err := s.cfg.blobStorage.Get(root, idx) | |||
if err != nil { | |||
if db.IsNotFound(err) { | |||
log.WithError(err).Debugf("BlobSidecar not found in db, root=%x, index=%d", root, idx) | |||
log.WithError(err).Debugf("Peer requested blob sidecar by root which was not found in db, root=%x, index=%d", root, idx) |
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.
Suggestion: This would be better to remove the templating and use fields. Easier to collect and measure this log
Revised log messaging to clarify scenarios where a node, despite having the blob sidecar locally, encounters an error serving it to a peer due to the sidecar not being found in the database