Upgrade to github.com/grpc-ecosystem/go-grpc-middleware/v2 #1370
Conversation
WalkthroughThe changes involve updating the gRPC middleware from version 1 to version 2 across multiple files in the codebase. This includes modifications to import paths and function calls related to authentication and metadata handling. Additionally, logging functionality in the gRPC server implementation has been streamlined, with the removal of several logging helper functions and the introduction of a new logging structure. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant AuthMiddleware
participant Logger
Client->>Server: gRPC Request
Server->>AuthMiddleware: Extract Token
AuthMiddleware->>Server: Token Validated
Server->>Logger: Log Request Details
Logger-->>Server: Log Confirmation
Server-->>Client: gRPC Response
Tip New featuresWalkthrough comment now includes:
Notes:
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
go.modis excluded by!**/*.modgo.sumis excluded by!**/*.sum,!**/*.sum
Files selected for processing (4)
- pkg/net/grpc/auth.go (3 hunks)
- pkg/net/grpc/authFromOutgoingMD.go (3 hunks)
- pkg/net/grpc/interceptor.go (1 hunks)
- pkg/net/grpc/server/makeDefaultOptions.go (5 hunks)
Additional comments not posted (15)
pkg/net/grpc/interceptor.go (3)
Line range hint
14-21: Verify the use ofWrapServerStreamunder the new middleware version.The function
StreamServerInterceptorusesWrapServerStreamfrom the new middleware version. Ensure that this method's behavior hasn't changed in a way that affects the interceptor's functionality.Run the following script to verify the specific use of
WrapServerStream:
Line range hint
23-31: Verify the use of unary interceptor under the new middleware version.The function
UnaryServerInterceptoruses the new middleware version. Ensure that the unary interceptor's behavior hasn't changed in a way that affects its functionality.Run the following script to verify the specific use of the unary interceptor:
6-6: Verify compatibility with the new middleware version.The import statement has been updated to use version 2 of the
go-grpc-middleware. It's crucial to ensure that the new version's features and breaking changes are compatible with the existing code.Run the following script to verify the compatibility:
pkg/net/grpc/authFromOutgoingMD.go (3)
7-8: Verify compatibility with the new middleware version.The import paths for authentication and metadata have been updated to use version 2 of the
go-grpc-middleware. It's crucial to ensure that the new version's features and breaking changes are compatible with the existing code.Run the following script to verify the compatibility:
40-40: Verify the use ofAuthFromMDunder the new middleware version.The function
TokenFromMDusesAuthFromMDfrom the new middleware version. Ensure that this method's behavior hasn't changed in a way that affects the function's functionality.Run the following script to verify the specific use of
AuthFromMD:
24-24: Verify the use ofExtractOutgoingunder the new middleware version.The function
TokenFromOutgoingMDusesExtractOutgoingfrom the new middleware version. Ensure that this method's behavior hasn't changed in a way that affects the function's functionality.Run the following script to verify the specific use of
ExtractOutgoing:pkg/net/grpc/auth.go (4)
52-52: Verify the use ofAuthFromMDunder the new middleware version.The function
ValidateJWTWithValidatorusesAuthFromMDfrom the new middleware version. Ensure that this method's behavior hasn't changed in a way that affects the function's functionality.Run the following script to verify the specific use of
AuthFromMD:
73-73: Verify the use ofExtractIncomingunder the new middleware version.The function
CtxWithIncomingTokenusesExtractIncomingfrom the new middleware version. Ensure that this method's behavior hasn't changed in a way that affects the function's functionality.Run the following script to verify the specific use of
ExtractIncoming:
66-66: Verify the use ofExtractOutgoingunder the new middleware version.The function
CtxWithTokenusesExtractOutgoingfrom the new middleware version. Ensure that this method's behavior hasn't changed in a way that affects the function's functionality.Run the following script to verify the specific use of
ExtractOutgoing:
8-9: Verify compatibility with the new middleware version.The import paths for authentication and metadata have been updated to use version 2 of the
go-grpc-middleware. It's crucial to ensure that the new version's features and breaking changes are compatible with the existing code.Run the following script to verify the compatibility:
pkg/net/grpc/server/makeDefaultOptions.go (5)
9-9: Approved: Import changes and new constant introduction.The new imports support the updated logging functionality, and the introduction of
loggerCtxMarkerKeyis crucial for the new context handling strategy.Also applies to: 29-32
99-138: Approved: Refactoring ofdefaultMessageProducer.The refactoring enhances clarity and efficiency by consolidating the logic into fewer functions. Verify the correct implementation of the new context handling strategy, particularly the use of
loggerCtxMarkerKey.Run the following script to verify the correct implementation of the new context handling strategy:
Verification successful
Verified: Correct implementation of context handling strategy using
loggerCtxMarkerKey.The
loggerCtxMarkerKeyis correctly used to store and retrieve the logger from the context within thepkg/net/grpc/server/makeDefaultOptions.gofile. The context handling strategy is implemented as intended. No further issues were found.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct implementation of the new context handling strategy. # Test: Search for the usage of `loggerCtxMarkerKey`. Expect: Correct implementation across the codebase. rg --type go -A 5 $'loggerCtxMarkerKey'Length of output: 1531
220-221: Approved: Update toMakeDefaultOptions.The updated interceptor chaining improves modularity and efficiency. Verify the correct implementation of this setup.
Run the following script to verify the correct implementation of the interceptor chaining:
142-142: Approved: Update toMakeDefaultMessageProducer.The use of
context.WithValueto store the logger simplifies context management and improves integration. Verify the correct usage of this method across the codebase.Run the following script to verify the correct usage of
context.WithValue:Verification successful
Verification Successful: Correct Usage of
context.WithValueThe keys used with
context.WithValueare defined uniquely and are not exported, ensuring proper encapsulation and usage. The implementation across the codebase adheres to best practices. No issues found.
oauth2.HTTPClient: Used in multiple files, related to HTTP client configuration.&authCtxKeyand&log.StartTimeKey: Defined as pointers incoap-gateway/service/service.go.loggerCtxMarkerKey: Defined as a struct type inpkg/net/grpc/server/makeDefaultOptions.go.authorizationKey: Defined inpkg/net/grpc/auth.goandpkg/net/http/jwt/token.go.lablelerContextKey: Defined as a custom type inpkg/opentelemetry/otelhttp/labeler.go.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the correct usage of `context.WithValue`. # Test: Search for the usage of `context.WithValue`. Expect: Correct implementation across the codebase. rg --type go -A 5 $'context.WithValue'Length of output: 4470
Script:
#!/bin/bash # Description: Verify the uniqueness and visibility of keys used with `context.WithValue`. # Extract keys used with `context.WithValue` and search for their definitions. rg --type go -o 'context.WithValue\([^,]+, ([^,]+),' | awk -F', ' '{print $2}' | sort | uniq | while read -r key; do echo "Searching for key definition: $key" rg --type go -A 3 -B 3 "$key" doneLength of output: 257
Script:
#!/bin/bash # Description: Re-verify the uniqueness and visibility of keys used with `context.WithValue`. # Extract keys used with `context.WithValue` and search for their definitions. rg --type go 'context.WithValue\([^,]+, ([^,]+),' -o -r '$1' | sort | uniq | while read -r key; do echo "Searching for key definition: $key" rg --type go -A 3 -B 3 "$key" doneLength of output: 381
Script:
#!/bin/bash # Description: Re-verify the uniqueness and visibility of keys used with `context.WithValue`. # Extract keys used with `context.WithValue` and search for their definitions. rg --type go 'context.WithValue\([^,]+, ([^,]+),' -o -r '$1' | sort | uniq | while read -r key; do # Remove any file path prefix from the key clean_key=$(echo "$key" | sed 's/.*://') echo "Searching for key definition: $clean_key" rg --type go -A 3 -B 3 "$clean_key" doneLength of output: 830
Script:
#!/bin/bash # Description: Search for the definitions of keys used with `context.WithValue`. # List of keys to search for keys=("oauth2.HTTPClient" "&authCtxKey" "&log.StartTimeKey" "loggerCtxMarkerKey" "authorizationKey" "lablelerContextKey") # Search for the definition of each key for key in "${keys[@]}"; do echo "Searching for definition of key: $key" rg --type go -A 3 -B 3 "$key" doneLength of output: 11214
62-78: Approved: Implementation oftoZapFields.The function efficiently converts logging fields into a format compatible with the Zap logger. Ensure that all necessary data types are covered in the switch-case structure.
Run the following script to verify the coverage of data types:
Danielius1922
force-pushed
the
adam/feature/grpc-middleware-v2
branch
from
465951b
to
518da0f
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (2)
go.modis excluded by!**/*.modgo.sumis excluded by!**/*.sum,!**/*.sum
Files selected for processing (4)
- pkg/net/grpc/auth.go (3 hunks)
- pkg/net/grpc/authFromOutgoingMD.go (3 hunks)
- pkg/net/grpc/interceptor.go (1 hunks)
- pkg/net/grpc/server/makeDefaultOptions.go (5 hunks)
Files skipped from review as they are similar to previous changes (3)
- pkg/net/grpc/auth.go
- pkg/net/grpc/authFromOutgoingMD.go
- pkg/net/grpc/interceptor.go
Additional comments not posted (6)
pkg/net/grpc/server/makeDefaultOptions.go (6)
9-9: Approved import addition.The addition of
github.com/grpc-ecosystem/go-grpc-middleware/v2/interceptors/loggingaligns with the changes in logging functionality described in the PR.
29-32: Approved context marker changes.The introduction of
loggerCtxMarkerandloggerCtxMarkerKeysimplifies the context management for logging, aligning with the PR's objectives to improve logger integration.
81-114: Approved streamlined logging function.The function
defaultMessageFieldseffectively consolidates logging field extraction and construction, enhancing both clarity and efficiency in the logging process.
136-143: Approved refactored message producer function.The refactoring of
defaultMessageProducerto use the new logging structure and context handling improves the integration of the logger with the gRPC context, aligning with the PR's objectives.
147-147: Approved function wrapper for message production.The function
MakeDefaultMessageProducereffectively wraps thedefaultMessageProducerwith a custom context, simplifying the integration of the logger with the gRPC context.
225-226: Approved updates to interceptor chaining.The updates to the chaining of stream and unary interceptors align with the PR's objectives to improve the gRPC server configuration.
|
No description provided.