[BUG] Restore increase max payload size of log service (Go) #4546
Conversation
Reviewer ChecklistPlease leverage this checklist to ensure your code review is thorough before approving Testing, Bugs, Errors, Logs, Documentation
System Compatibility
Quality
|
|
This PR fixes a bug by reintroducing increased gRPC payload size in the Go log service without using the methods from the grpcutils package. The fix hard-codes the maxGrpcFrameSize constant (256MB) and applies it directly to the gRPC server configuration to maintain metrics collection functionality. This summary was automatically generated by @propel-code-bot |
![propel-code-bot[bot]](https://avatars.githubusercontent.com/in/1179004?s=60&v=4){kind=small}
| // hard-coding this here despite it also being in pkg/grpcutils/service.go because | ||
| // using the methods from grpcutils results in breaking our metrics collection for | ||
| // some reason. This service is being deprecated soon, so this is just a quick fix. | ||
| const maxGrpcFrameSize = 256 * 1024 * 1024 |
There was a problem hiding this comment.
[BestPractice]
Although the comment explains the reason for hardcoding maxGrpcFrameSize, consider adding a TODO or issue number reference to track the proper fix later. This helps future maintainers understand that this is a temporary solution that should be addressed once the service is refactored.
| grpc.MaxRecvMsgSize(maxGrpcFrameSize), | ||
| grpc.UnaryInterceptor(sharedOtel.ServerGrpcInterceptor), |
There was a problem hiding this comment.
[BestPractice]
Consider adding MaxSendMsgSize similar to MaxRecvMsgSize to ensure symmetrical message size limits for both directions. Without this, the service may be able to receive large messages but fail when trying to send responses of similar size.
Description of changes
This restores the core functionality we were hoping to add in #4534 which is increasing the gRPC payload size. Instead of using the methods from the
grpcutilspackage, this just sets the option in the existing setup so that we hopefully don't break whatever voodoo is keeping metrics collection working.Test plan
I ran
make testlocally, the build succeeds, but our local tests are broken for some reason. Relying on CI so that I don't have to debug yet another orthogonal issue.