-
Notifications
You must be signed in to change notification settings - Fork 13
chore: log requests upon retrieval #1140
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
Conversation
Images are ready for the commit at 1ccc31e. To use the images, use the tag |
524c2ce
to
679c526
Compare
f371323
to
fbb7c9c
Compare
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 see the logs in the artifacts of the associated CI, made a few general comments.
Certainly an improvement.
imageName := "unknown" | ||
if req.GetImage() != "" { | ||
imageName = req.GetImage() | ||
} | ||
logrus.Infof("Fetching vulnerabilities for %s", imageName) |
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.
Should we add to the backlog modifying the invocations from Central/Sensor to add the image name?
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.
api/grpc/logging_interceptor.go
Outdated
return func(ctx context.Context, req interface{}, info *grpc.UnaryServerInfo, handler grpc.UnaryHandler) (interface{}, error) { | ||
peerInfo, exists := peer.FromContext(ctx) | ||
if !exists { | ||
return nil, status.Error(codes.InvalidArgument, "unable to parse peer information from request context") | ||
} | ||
|
||
logrus.WithFields(map[string]interface{}{ | ||
"Method": info.FullMethod, | ||
}).Infof("Received gRPC request from %v", peerInfo.Addr) |
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'm not an expert on grpc-gateway - but looks like it supports adding X-Forwarded-For
header to the context
Guessing should be able to extract the Metadata
from the context, something like:
addrs := metadata.ValueFromIncomingContext(ctx, "X-Forwarded-For")
And then append the addrs
to this log
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.
doesn't seem like we can rely on this:
{"Event":"Received gRPC request from unknown","HEADERS":{":authority":["localhost:8443"],"content-type":["application/grpc"],"user-agent":["grpc-go/1.57.0"]},"Level":"info","Location":"logging_interceptor.go:28","Method":"/scannerV1.ImageScanService/GetImageComponents","Time":"2023-08-23 22:25:36.923706"}
Going to revert back to the original but leave out the peer's address because it added no value
fbb7c9c
to
32262f0
Compare
604ba3f
to
1ccc31e
Compare
1ccc31e
to
526adf6
Compare
This came up when looking into potential network issues between Central and Scanner. These logs will let us know about all incoming traffic to the HTTP and gRPC endpoints which are exposed by Scanner.