feat: OTEL wiring in grpcserver interceptor #21029
Conversation
* OTEL wiring in grpcserver * updates * updates * grpc headers * updates * updates * updates * go 1.21 * updates * test go1.21 * update lint go * go mod for tests * more updates * bump linter version * updates * remove nakedret * codeql go version
WalkthroughWalkthroughThe recent changes focus on upgrading the Go version in Continuous Integration (CI) configurations and enhancing the gRPC server's observability through OpenTelemetry integration. New logging and instrumentation libraries have been added to improve monitoring capabilities. These updates emphasize improvements in performance and logging practices by adopting newer versions of dependencies while maintaining the overall functionality. Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant gRPC Server
participant OpenTelemetry
Client->>gRPC Server: Send gRPC Request
gRPC Server->>OpenTelemetry: Start Span
gRPC Server->>gRPC Server: Process Request
alt Success
gRPC Server-->>OpenTelemetry: Mark Span as Successful
else Error
gRPC Server-->>OpenTelemetry: Log Error and Update Span
end
gRPC Server-->>Client: Send Response
sequenceDiagram
participant Developer
participant CI/CD Pipeline
participant Build Environment
Developer->>CI/CD Pipeline: Trigger Build
CI/CD Pipeline->>Build Environment: Use Go 1.21
Build Environment-->>CI/CD Pipeline: Build Successful
CI/CD Pipeline-->>Developer: Notify Build Success
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 as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
github-actions
bot
added
C:x/staking
C:x/gov
C:x/slashing
C:x/distribution
p0mvn
changed the title
|
I think that the failing CI test stems from the go cache as it passes for me locally. Would appreciate pointers if I am wrong. |
This comment has been minimized.
This comment has been minimized.
.github/workflows/test.yml
Outdated
| go-version: "1.20" | ||
| go-version: "1.21" |
There was a problem hiding this comment.
Note to reviewer - I had to bump these up because some GRPC dependencies are supported starting from Go 1.21
There was a problem hiding this comment.
I don't believe you had to bump CI for those. Those are standalone components that do not import the SDK. It should stay on 1.20, and do not required a go version bump
![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
Outside diff range, codebase verification and nitpick comments (1)
server/grpc/server.go (1)
39-39: EnsureNewGRPCServeris adequately testedThe addition of
grpc.StatsHandler(otelgrpc.NewServerHandler())enhances observability and monitoring capabilities. However, there are no direct tests found forNewGRPCServer. It is recommended to add tests to verify the new functionality and ensure it works correctly.
- Add tests for
NewGRPCServerto verify the integration ofotelgrpc.NewServerHandler.Analysis chain
LGTM! Integration of OpenTelemetry StatsHandler.
The addition of
grpc.StatsHandler(otelgrpc.NewServerHandler())is appropriate and enhances observability and monitoring capabilities.However, ensure that all function calls to
NewGRPCServerare tested with the new functionality.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify all function calls to `NewGRPCServer` are tested with the new functionality. # Test: Search for the function usage. Expect: Function calls are tested with the new functionality. rg --type go -A 5 $'NewGRPCServer'Length of output: 1234
Script:
#!/bin/bash # Description: Verify that `NewGRPCServer` is tested. # Search for test functions related to `NewGRPCServer` in the codebase. rg --type go -A 5 'func Test' | grep -i 'grpcserver'Length of output: 52
Script:
#!/bin/bash # Description: Verify that `NewGRPCServer` is tested. # Search for test files in the directories where `NewGRPCServer` is called. fd --type f --extension go --search-path testutil/network --search-path server --exec rg 'NewGRPCServer'Length of output: 434
Script:
#!/bin/bash # Description: Verify that `NewGRPCServer` is tested. # Search for test functions across the entire codebase that might be testing `NewGRPCServer`. rg --type go -l 'NewGRPCServer' | xargs rg --type go 'func Test'Length of output: 64
Script:
#!/bin/bash # Description: Verify that `NewGRPCServer` is tested. # Search for test files in the entire codebase and look for references to `NewGRPCServer`. fd --type f --extension go --exec rg 'NewGRPCServer'Length of output: 382
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files ignored due to path filters (27)
client/v2/go.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.sumserver/v2/cometbft/go.sumis excluded by!**/*.sumsimapp/go.sumis excluded by!**/*.sumsimapp/v2/go.sumis excluded by!**/*.sumtests/go.sumis excluded by!**/*.sumx/accounts/defaults/lockup/go.sumis excluded by!**/*.sumx/accounts/defaults/multisig/go.sumis excluded by!**/*.sumx/accounts/go.sumis excluded by!**/*.sumx/auth/go.sumis excluded by!**/*.sumx/authz/go.sumis excluded by!**/*.sumx/bank/go.sumis excluded by!**/*.sumx/circuit/go.sumis excluded by!**/*.sumx/consensus/go.sumis excluded by!**/*.sumx/distribution/go.sumis excluded by!**/*.sumx/epochs/go.sumis excluded by!**/*.sumx/evidence/go.sumis excluded by!**/*.sumx/feegrant/go.sumis excluded by!**/*.sumx/gov/go.sumis excluded by!**/*.sumx/group/go.sumis excluded by!**/*.sumx/mint/go.sumis excluded by!**/*.sumx/nft/go.sumis excluded by!**/*.sumx/params/go.sumis excluded by!**/*.sumx/protocolpool/go.sumis excluded by!**/*.sumx/slashing/go.sumis excluded by!**/*.sumx/staking/go.sumis excluded by!**/*.sumx/upgrade/go.sumis excluded by!**/*.sum
Files selected for processing (30)
- .github/workflows/test.yml (8 hunks)
- baseapp/grpcserver.go (3 hunks)
- client/v2/go.mod (1 hunks)
- go.mod (3 hunks)
- server/grpc/server.go (2 hunks)
- server/v2/cometbft/go.mod (2 hunks)
- simapp/go.mod (1 hunks)
- simapp/v2/go.mod (1 hunks)
- tests/go.mod (1 hunks)
- x/accounts/defaults/lockup/go.mod (1 hunks)
- x/accounts/defaults/multisig/go.mod (2 hunks)
- x/accounts/go.mod (1 hunks)
- x/auth/go.mod (2 hunks)
- x/authz/go.mod (1 hunks)
- x/bank/go.mod (1 hunks)
- x/circuit/go.mod (2 hunks)
- x/consensus/go.mod (2 hunks)
- x/distribution/go.mod (1 hunks)
- x/epochs/go.mod (1 hunks)
- x/evidence/go.mod (2 hunks)
- x/feegrant/go.mod (1 hunks)
- x/gov/go.mod (1 hunks)
- x/group/go.mod (2 hunks)
- x/mint/go.mod (1 hunks)
- x/nft/go.mod (2 hunks)
- x/params/go.mod (2 hunks)
- x/protocolpool/go.mod (2 hunks)
- x/slashing/go.mod (2 hunks)
- x/staking/go.mod (1 hunks)
- x/upgrade/go.mod (1 hunks)
Files skipped from review due to trivial changes (3)
- .github/workflows/test.yml
- simapp/v2/go.mod
- x/upgrade/go.mod
Additional context used
Path-based instructions (3)
server/grpc/server.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.baseapp/grpcserver.go (1)
Pattern
**/*.go: Review the Golang code for conformity with the Uber Golang style guide, highlighting any deviations.tests/go.mod (1)
Pattern
tests/**/*: "Assess the integration and e2e test code assessing sufficient code coverage for the changes associated in the pull request"
Additional comments not posted (129)
server/grpc/server.go (1)
8-8: LGTM! Import statement for OpenTelemetry.The import statement for
otelgrpcis correct and necessary for the new functionality.baseapp/grpcserver.go (3)
11-15: LGTM! Import statements for OpenTelemetry.The import statements for OpenTelemetry packages (
otel,attribute,otelcodes,propagation,trace) are correct and necessary for the new functionality.
29-29: LGTM! Addition oftracerNameconstant.The addition of the
tracerNameconstant improves code readability and maintainability.
Line range hint
33-109:
LGTM! Integration of OpenTelemetry tracing interceptor.The addition of the tracing interceptor that captures and manages tracing spans for each gRPC request is appropriate and enhances observability and monitoring capabilities.
However, ensure that all function calls to
RegisterGRPCServerare tested with the new functionality.x/auth/go.mod (2)
82-83: LGTM! Addition of logging dependencies.The addition of
github.com/go-logr/lograndgithub.com/go-logr/stdrdependencies is appropriate for enhanced logging capabilities.
159-162: LGTM! Addition of OpenTelemetry dependencies.The addition of
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc,go.opentelemetry.io/otel,go.opentelemetry.io/otel/metric, andgo.opentelemetry.io/otel/tracedependencies is appropriate for integrating OpenTelemetry tracing and metrics collection.x/consensus/go.mod (2)
74-75: New Logging Dependencies AddedThe new logging dependencies
github.com/go-logr/lograndgithub.com/go-logr/stdrwill enhance the logging capabilities of the project.
153-156: New OpenTelemetry Dependencies AddedThe new OpenTelemetry dependencies
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc,go.opentelemetry.io/otel,go.opentelemetry.io/otel/metric, andgo.opentelemetry.io/otel/tracewill enhance the observability and tracing capabilities of the project.x/staking/go.mod (2)
175-176: New Logging Dependencies AddedThe new logging dependencies
github.com/go-logr/lograndgithub.com/go-logr/stdrwill enhance the logging capabilities of the project.
178-181: New OpenTelemetry Dependencies AddedThe new OpenTelemetry dependencies
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc,go.opentelemetry.io/otel,go.opentelemetry.io/otel/metric, andgo.opentelemetry.io/otel/tracewill enhance the observability and tracing capabilities of the project.x/bank/go.mod (2)
173-174: New Logging Dependencies AddedThe new logging dependencies
github.com/go-logr/lograndgithub.com/go-logr/stdrwill enhance the logging capabilities of the project.
176-179: New OpenTelemetry Dependencies AddedThe new OpenTelemetry dependencies
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc,go.opentelemetry.io/otel,go.opentelemetry.io/otel/metric, andgo.opentelemetry.io/otel/tracewill enhance the observability and tracing capabilities of the project.x/evidence/go.mod (6)
78-78: LGTM! Added dependency for structured logging.The addition of
github.com/go-logr/logr v1.4.2enhances the logging capabilities of the project.
79-79: LGTM! Added dependency for standard library logger implementation.The addition of
github.com/go-logr/stdr v1.2.2complements thego-logrlibrary by providing a standard library logger implementation.
156-156: LGTM! Added dependency for gRPC instrumentation with OpenTelemetry.The addition of
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.49.0enables gRPC instrumentation for distributed tracing.
157-157: LGTM! Added core OpenTelemetry dependency.The addition of
go.opentelemetry.io/otel v1.28.0is crucial for integrating OpenTelemetry's tracing and metrics capabilities.
158-158: LGTM! Added dependency for OpenTelemetry metrics.The addition of
go.opentelemetry.io/otel/metric v1.28.0enhances the project's capability to collect and export metrics using OpenTelemetry.
159-159: LGTM! Added dependency for OpenTelemetry tracing.The addition of
go.opentelemetry.io/otel/trace v1.28.0is essential for implementing distributed tracing using OpenTelemetry.x/protocolpool/go.mod (6)
78-78: LGTM! Added dependency for structured logging.The addition of
github.com/go-logr/logr v1.4.2enhances the logging capabilities of the project.
79-79: LGTM! Added dependency for standard library logger implementation.The addition of
github.com/go-logr/stdr v1.2.2complements thego-logrlibrary by providing a standard library logger implementation.
157-157: LGTM! Added dependency for gRPC instrumentation with OpenTelemetry.The addition of
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.49.0enables gRPC instrumentation for distributed tracing.
158-158: LGTM! Added core OpenTelemetry dependency.The addition of
go.opentelemetry.io/otel v1.28.0is crucial for integrating OpenTelemetry's tracing and metrics capabilities.
159-159: LGTM! Added dependency for OpenTelemetry metrics.The addition of
go.opentelemetry.io/otel/metric v1.28.0enhances the project's capability to collect and export metrics using OpenTelemetry.
160-160: LGTM! Added dependency for OpenTelemetry tracing.The addition of
go.opentelemetry.io/otel/trace v1.28.0is essential for implementing distributed tracing using OpenTelemetry.x/epochs/go.mod (6)
171-171: LGTM! Added dependency for structured logging.The addition of
github.com/go-logr/logr v1.4.2enhances the logging capabilities of the project.
172-172: LGTM! Added dependency for standard library logger implementation.The addition of
github.com/go-logr/stdr v1.2.2complements thego-logrlibrary by providing a standard library logger implementation.
175-175: LGTM! Added dependency for gRPC instrumentation with OpenTelemetry.The addition of
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.49.0enables gRPC instrumentation for distributed tracing.
176-176: LGTM! Added core OpenTelemetry dependency.The addition of
go.opentelemetry.io/otel v1.28.0is crucial for integrating OpenTelemetry's tracing and metrics capabilities.
177-177: LGTM! Added dependency for OpenTelemetry metrics.The addition of
go.opentelemetry.io/otel/metric v1.28.0enhances the project's capability to collect and export metrics using OpenTelemetry.
178-178: LGTM! Added dependency for OpenTelemetry tracing.The addition of
go.opentelemetry.io/otel/trace v1.28.0is essential for implementing distributed tracing using OpenTelemetry.x/slashing/go.mod (6)
79-79: Dependency addition approved:github.com/go-logr/logr v1.4.2.This dependency enhances structured logging capabilities.
80-80: Dependency addition approved:github.com/go-logr/stdr v1.2.2.This dependency provides a standard library logger implementation for
logr.
158-158: Dependency addition approved:go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.49.0.This dependency provides OpenTelemetry instrumentation for gRPC.
159-159: Dependency addition approved:go.opentelemetry.io/otel v1.28.0.This dependency is the core OpenTelemetry library for Go.
160-160: Dependency addition approved:go.opentelemetry.io/otel/metric v1.28.0.This dependency provides OpenTelemetry metrics support for Go.
161-161: Dependency addition approved:go.opentelemetry.io/otel/trace v1.28.0.This dependency provides OpenTelemetry tracing support for Go.
x/nft/go.mod (6)
76-76: Dependency addition approved:github.com/go-logr/logr v1.4.2.This dependency enhances structured logging capabilities.
77-77: Dependency addition approved:github.com/go-logr/stdr v1.2.2.This dependency provides a standard library logger implementation for
logr.
155-155: Dependency addition approved:go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.49.0.This dependency provides OpenTelemetry instrumentation for gRPC.
156-156: Dependency addition approved:go.opentelemetry.io/otel v1.28.0.This dependency is the core OpenTelemetry library for Go.
157-157: Dependency addition approved:go.opentelemetry.io/otel/metric v1.28.0.This dependency provides OpenTelemetry metrics support for Go.
158-158: Dependency addition approved:go.opentelemetry.io/otel/trace v1.28.0.This dependency provides OpenTelemetry tracing support for Go.
x/circuit/go.mod (6)
75-75: Dependency addition approved:github.com/go-logr/logr v1.4.2.This dependency enhances structured logging capabilities.
76-76: Dependency addition approved:github.com/go-logr/stdr v1.2.2.This dependency provides a standard library logger implementation for
logr.
155-155: Dependency addition approved:go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.49.0.This dependency provides OpenTelemetry instrumentation for gRPC.
156-156: Dependency addition approved:go.opentelemetry.io/otel v1.28.0.This dependency is the core OpenTelemetry library for Go.
157-157: Dependency addition approved:go.opentelemetry.io/otel/metric v1.28.0.This dependency provides OpenTelemetry metrics support for Go.
158-158: Dependency addition approved:go.opentelemetry.io/otel/trace v1.28.0.This dependency provides OpenTelemetry tracing support for Go.
x/distribution/go.mod (6)
36-36: Addgithub.com/go-logr/logrdependency.The addition of
github.com/go-logr/logr(version 1.4.2) as an indirect dependency is appropriate for enhanced logging capabilities.
37-37: Addgithub.com/go-logr/stdrdependency.The addition of
github.com/go-logr/stdr(version 1.2.2) as an indirect dependency complements the logging capabilities provided bylogr.
40-40: Addgo.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpcdependency.The addition of
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc(version 0.49.0) as an indirect dependency is aligned with the PR objective of integrating OpenTelemetry tracing.
41-41: Addgo.opentelemetry.io/oteldependency.The addition of
go.opentelemetry.io/otel(version 1.28.0) as an indirect dependency is essential for OpenTelemetry integration.
42-42: Addgo.opentelemetry.io/otel/metricdependency.The addition of
go.opentelemetry.io/otel/metric(version 1.28.0) as an indirect dependency supports metrics collection in OpenTelemetry.
43-43: Addgo.opentelemetry.io/otel/tracedependency.The addition of
go.opentelemetry.io/otel/trace(version 1.28.0) as an indirect dependency is crucial for tracing capabilities in OpenTelemetry.x/mint/go.mod (6)
168-168: Addgithub.com/go-logr/logrdependency.The addition of
github.com/go-logr/logr(version 1.4.2) as an indirect dependency is appropriate for enhanced logging capabilities.
169-169: Addgithub.com/go-logr/stdrdependency.The addition of
github.com/go-logr/stdr(version 1.2.2) as an indirect dependency complements the logging capabilities provided bylogr.
177-177: Addgo.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpcdependency.The addition of
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc(version 0.49.0) as an indirect dependency is aligned with the PR objective of integrating OpenTelemetry tracing.
178-178: Addgo.opentelemetry.io/oteldependency.The addition of
go.opentelemetry.io/otel(version 1.28.0) as an indirect dependency is essential for OpenTelemetry integration.
179-179: Addgo.opentelemetry.io/otel/metricdependency.The addition of
go.opentelemetry.io/otel/metric(version 1.28.0) as an indirect dependency supports metrics collection in OpenTelemetry.
180-180: Addgo.opentelemetry.io/otel/tracedependency.The addition of
go.opentelemetry.io/otel/trace(version 1.28.0) as an indirect dependency is crucial for tracing capabilities in OpenTelemetry.x/authz/go.mod (6)
174-174: Addgithub.com/go-logr/logrdependency.The addition of
github.com/go-logr/logr(version 1.4.2) as an indirect dependency is appropriate for enhanced logging capabilities.
175-175: Addgithub.com/go-logr/stdrdependency.The addition of
github.com/go-logr/stdr(version 1.2.2) as an indirect dependency complements the logging capabilities provided bylogr.
177-177: Addgo.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpcdependency.The addition of
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc(version 0.49.0) as an indirect dependency is aligned with the PR objective of integrating OpenTelemetry tracing.
178-178: Addgo.opentelemetry.io/oteldependency.The addition of
go.opentelemetry.io/otel(version 1.28.0) as an indirect dependency is essential for OpenTelemetry integration.
179-179: Addgo.opentelemetry.io/otel/metricdependency.The addition of
go.opentelemetry.io/otel/metric(version 1.28.0) as an indirect dependency supports metrics collection in OpenTelemetry.
180-180: Addgo.opentelemetry.io/otel/tracedependency.The addition of
go.opentelemetry.io/otel/trace(version 1.28.0) as an indirect dependency is crucial for tracing capabilities in OpenTelemetry.x/accounts/defaults/lockup/go.mod (5)
26-26: Dependency Addition Approved:github.com/go-logr/logr v1.4.2.The addition of
github.com/go-logr/logr v1.4.2is appropriate for enhancing structured logging capabilities.
27-27: Dependency Addition Approved:github.com/go-logr/stdr v1.2.2.The addition of
github.com/go-logr/stdr v1.2.2is suitable for providing a standard library logger implementation forlogr.
33-33: Dependency Addition Approved:go.opentelemetry.io/otel v1.28.0.The addition of
go.opentelemetry.io/otel v1.28.0is crucial for integrating OpenTelemetry tracing into the application.
34-34: Dependency Addition Approved:go.opentelemetry.io/otel/metric v1.28.0.The addition of
go.opentelemetry.io/otel/metric v1.28.0is appropriate for enabling metrics instrumentation in the application.
35-35: Dependency Addition Approved:go.opentelemetry.io/otel/trace v1.28.0.The addition of
go.opentelemetry.io/otel/trace v1.28.0is suitable for enabling tracing instrumentation in the application.x/gov/go.mod (6)
177-177: Dependency Addition Approved:github.com/go-logr/logr v1.4.2.The addition of
github.com/go-logr/logr v1.4.2is appropriate for enhancing structured logging capabilities.
178-178: Dependency Addition Approved:github.com/go-logr/stdr v1.2.2.The addition of
github.com/go-logr/stdr v1.2.2is suitable for providing a standard library logger implementation forlogr.
179-179: Dependency Addition Approved:go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.49.0.The addition of
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.49.0is appropriate for enabling OpenTelemetry instrumentation for gRPC.
180-180: Dependency Addition Approved:go.opentelemetry.io/otel v1.28.0.The addition of
go.opentelemetry.io/otel v1.28.0is crucial for integrating OpenTelemetry tracing into the application.
181-181: Dependency Addition Approved:go.opentelemetry.io/otel/metric v1.28.0.The addition of
go.opentelemetry.io/otel/metric v1.28.0is appropriate for enabling metrics instrumentation in the application.
182-182: Dependency Addition Approved:go.opentelemetry.io/otel/trace v1.28.0.The addition of
go.opentelemetry.io/otel/trace v1.28.0is suitable for enabling tracing instrumentation in the application.server/v2/cometbft/go.mod (5)
101-101: Dependency Addition Approved:github.com/go-logr/logr v1.4.2.The addition of
github.com/go-logr/logr v1.4.2is appropriate for enhancing structured logging capabilities.
102-102: Dependency Addition Approved:github.com/go-logr/stdr v1.2.2.The addition of
github.com/go-logr/stdr v1.2.2is suitable for providing a standard library logger implementation forlogr.
178-178: Dependency Addition Approved:go.opentelemetry.io/otel v1.28.0.The addition of
go.opentelemetry.io/otel v1.28.0is crucial for integrating OpenTelemetry tracing into the application.
179-179: Dependency Addition Approved:go.opentelemetry.io/otel/metric v1.28.0.The addition of
go.opentelemetry.io/otel/metric v1.28.0is appropriate for enabling metrics instrumentation in the application.
180-180: Dependency Addition Approved:go.opentelemetry.io/otel/trace v1.28.0.The addition of
go.opentelemetry.io/otel/trace v1.28.0is suitable for enabling tracing instrumentation in the application.x/params/go.mod (6)
78-78: Approved: Addition of logging library.The addition of
github.com/go-logr/logr v1.4.2enhances the logging framework.
79-79: Approved: Addition of standard library adapter for logging.The addition of
github.com/go-logr/stdr v1.2.2provides a standard library adapter for logr.
156-156: Approved: Addition of OpenTelemetry instrumentation for gRPC.The addition of
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.49.0enhances observability for gRPC.
157-157: Approved: Addition of core OpenTelemetry library.The addition of
go.opentelemetry.io/otel v1.28.0provides the core functionalities for OpenTelemetry.
158-158: Approved: Addition of OpenTelemetry metrics library.The addition of
go.opentelemetry.io/otel/metric v1.28.0enhances the metrics tracking capabilities.
159-159: Approved: Addition of OpenTelemetry tracing library.The addition of
go.opentelemetry.io/otel/trace v1.28.0enhances the tracing capabilities.x/feegrant/go.mod (6)
177-177: Approved: Addition of logging library.The addition of
github.com/go-logr/logr v1.4.2enhances the logging framework.
178-178: Approved: Addition of standard library adapter for logging.The addition of
github.com/go-logr/stdr v1.2.2provides a standard library adapter for logr.
180-180: Approved: Addition of OpenTelemetry instrumentation for gRPC.The addition of
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.49.0enhances observability for gRPC.
181-181: Approved: Addition of core OpenTelemetry library.The addition of
go.opentelemetry.io/otel v1.28.0provides the core functionalities for OpenTelemetry.
182-182: Approved: Addition of OpenTelemetry metrics library.The addition of
go.opentelemetry.io/otel/metric v1.28.0enhances the metrics tracking capabilities.
183-183: Approved: Addition of OpenTelemetry tracing library.The addition of
go.opentelemetry.io/otel/trace v1.28.0enhances the tracing capabilities.x/accounts/defaults/multisig/go.mod (6)
73-73: Approved: Addition of logging library.The addition of
github.com/go-logr/logr v1.4.2enhances the logging framework.
74-74: Approved: Addition of standard library adapter for logging.The addition of
github.com/go-logr/stdr v1.2.2provides a standard library adapter for logr.
154-154: Approved: Addition of OpenTelemetry instrumentation for gRPC.The addition of
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc v0.49.0enhances observability for gRPC.
155-155: Approved: Addition of core OpenTelemetry library.The addition of
go.opentelemetry.io/otel v1.28.0provides the core functionalities for OpenTelemetry.
156-156: Approved: Addition of OpenTelemetry metrics library.The addition of
go.opentelemetry.io/otel/metric v1.28.0enhances the metrics tracking capabilities.
157-157: Approved: Addition of OpenTelemetry tracing library.The addition of
go.opentelemetry.io/otel/trace v1.28.0enhances the tracing capabilities.x/accounts/go.mod (6)
27-27: Approved: Addition ofgithub.com/go-logr/logrdependency.The
github.com/go-logr/logrlibrary provides a structured logger interface, which aligns with the PR's objective to enhance logging capabilities.
28-28: Approved: Addition ofgithub.com/go-logr/stdrdependency.The
github.com/go-logr/stdrlibrary provides a standard logger implementation forlogr, enhancing the logging capabilities.
30-30: Approved: Addition ofgo.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpcdependency.The
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpclibrary provides OpenTelemetry instrumentation for gRPC, enhancing observability and tracing.
31-31: Approved: Addition ofgo.opentelemetry.io/oteldependency.The
go.opentelemetry.io/otellibrary is the core OpenTelemetry library, essential for tracing and metrics.
32-32: Approved: Addition ofgo.opentelemetry.io/otel/metricdependency.The
go.opentelemetry.io/otel/metriclibrary supports metric collection and reporting, complementing the tracing capabilities.
33-33: Approved: Addition ofgo.opentelemetry.io/otel/tracedependency.The
go.opentelemetry.io/otel/tracelibrary supports tracing functionalities, enhancing observability.client/v2/go.mod (6)
176-176: Approved: Addition ofgithub.com/go-logr/logrdependency.The
github.com/go-logr/logrlibrary provides a structured logger interface, which aligns with the PR's objective to enhance logging capabilities.
177-177: Approved: Addition ofgithub.com/go-logr/stdrdependency.The
github.com/go-logr/stdrlibrary provides a standard logger implementation forlogr, enhancing the logging capabilities.
179-179: Approved: Addition ofgo.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpcdependency.The
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpclibrary provides OpenTelemetry instrumentation for gRPC, enhancing observability and tracing.
180-180: Approved: Addition ofgo.opentelemetry.io/oteldependency.The
go.opentelemetry.io/otellibrary is the core OpenTelemetry library, essential for tracing and metrics.
181-181: Approved: Addition ofgo.opentelemetry.io/otel/metricdependency.The
go.opentelemetry.io/otel/metriclibrary supports metric collection and reporting, complementing the tracing capabilities.
182-182: Approved: Addition ofgo.opentelemetry.io/otel/tracedependency.The
go.opentelemetry.io/otel/tracelibrary supports tracing functionalities, enhancing observability.go.mod (6)
59-59: Approved: Addition ofgo.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpcdependency.The
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpclibrary provides OpenTelemetry instrumentation for gRPC, enhancing observability and tracing.
60-60: Approved: Addition ofgo.opentelemetry.io/oteldependency.The
go.opentelemetry.io/otellibrary is the core OpenTelemetry library, essential for tracing and metrics.
61-61: Approved: Addition ofgo.opentelemetry.io/otel/tracedependency.The
go.opentelemetry.io/otel/tracelibrary supports tracing functionalities, enhancing observability.
107-107: Approved: Addition ofgithub.com/go-logr/logrdependency.The
github.com/go-logr/logrlibrary provides a structured logger interface, which aligns with the PR's objective to enhance logging capabilities.
108-108: Approved: Addition ofgithub.com/go-logr/stdrdependency.The
github.com/go-logr/stdrlibrary provides a standard logger implementation forlogr, enhancing the logging capabilities.
171-171: Approved: Addition ofgo.opentelemetry.io/otel/metricdependency.The
go.opentelemetry.io/otel/metriclibrary supports metric collection and reporting, complementing the tracing capabilities.x/group/go.mod (5)
90-90: Approved: Addition ofgithub.com/go-logr/logr.The addition of this dependency aligns with the PR objective to enhance logging capabilities.
91-91: Approved: Addition ofgithub.com/go-logr/stdr.The addition of this dependency aligns with the PR objective to enhance logging capabilities.
168-168: Approved: Addition ofgo.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc.The addition of this dependency aligns with the PR objective to integrate OpenTelemetry tracing into the gRPC server interceptor.
169-169: Approved: Addition ofgo.opentelemetry.io/otel.The addition of this dependency aligns with the PR objective to enhance observability and monitoring capabilities.
170-171: Approved: Addition ofgo.opentelemetry.io/otel/metricandgo.opentelemetry.io/otel/trace.The addition of these dependencies aligns with the PR objective to enhance observability and monitoring capabilities.
tests/go.mod (3)
207-207: Approved: Update ofgo.opentelemetry.io/otelfromv1.27.0tov1.28.0.The update aligns with the PR objective to integrate the latest OpenTelemetry features and fixes.
208-208: Approved: Update ofgo.opentelemetry.io/otel/metricfromv1.27.0tov1.28.0.The update aligns with the PR objective to integrate the latest OpenTelemetry features and fixes.
209-209: Approved: Update ofgo.opentelemetry.io/otel/tracefromv1.27.0tov1.28.0.The update aligns with the PR objective to integrate the latest OpenTelemetry features and fixes.
simapp/go.mod (3)
206-206: Approved: Update ofgo.opentelemetry.io/otelfromv1.27.0tov1.28.0.The update aligns with the PR objective to integrate the latest OpenTelemetry features and fixes.
207-207: Approved: Update ofgo.opentelemetry.io/otel/metricfromv1.27.0tov1.28.0.The update aligns with the PR objective to integrate the latest OpenTelemetry features and fixes.
208-208: Approved: Update ofgo.opentelemetry.io/otel/tracefromv1.27.0tov1.28.0.The update aligns with the PR objective to integrate the latest OpenTelemetry features and fixes.
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files selected for processing (1)
- CHANGELOG.md (1 hunks)
Additional context used
Path-based instructions (1)
CHANGELOG.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Additional comments not posted (1)
CHANGELOG.md (1)
64-64: Changelog entry looks good.The entry documenting the addition of OTEL wiring in the gRPC server interceptor is accurate and correctly formatted.
p0mvn
force-pushed
the
roman/datadog-grpc-intercept
branch
from
a86a57f to
9ddcd02
Compare
There was a problem hiding this comment.
See #21029 (comment)
Could try revert CI and do a go mod tidy all (our script)
You are probably using a go.work. It currently fails on main or if you don't use go.work, there is a replace missing. |
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files ignored due to path filters (27)
client/v2/go.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.sumserver/v2/cometbft/go.sumis excluded by!**/*.sumsimapp/go.sumis excluded by!**/*.sumsimapp/v2/go.sumis excluded by!**/*.sumtests/go.sumis excluded by!**/*.sumx/accounts/defaults/lockup/go.sumis excluded by!**/*.sumx/accounts/defaults/multisig/go.sumis excluded by!**/*.sumx/accounts/go.sumis excluded by!**/*.sumx/auth/go.sumis excluded by!**/*.sumx/authz/go.sumis excluded by!**/*.sumx/bank/go.sumis excluded by!**/*.sumx/circuit/go.sumis excluded by!**/*.sumx/consensus/go.sumis excluded by!**/*.sumx/distribution/go.sumis excluded by!**/*.sumx/epochs/go.sumis excluded by!**/*.sumx/evidence/go.sumis excluded by!**/*.sumx/feegrant/go.sumis excluded by!**/*.sumx/gov/go.sumis excluded by!**/*.sumx/group/go.sumis excluded by!**/*.sumx/mint/go.sumis excluded by!**/*.sumx/nft/go.sumis excluded by!**/*.sumx/params/go.sumis excluded by!**/*.sumx/protocolpool/go.sumis excluded by!**/*.sumx/slashing/go.sumis excluded by!**/*.sumx/staking/go.sumis excluded by!**/*.sumx/upgrade/go.sumis excluded by!**/*.sum
Files selected for processing (27)
- client/v2/go.mod (1 hunks)
- go.mod (3 hunks)
- server/v2/cometbft/go.mod (2 hunks)
- simapp/go.mod (1 hunks)
- simapp/v2/go.mod (1 hunks)
- tests/go.mod (1 hunks)
- x/accounts/defaults/lockup/go.mod (1 hunks)
- x/accounts/defaults/multisig/go.mod (2 hunks)
- x/accounts/go.mod (1 hunks)
- x/auth/go.mod (2 hunks)
- x/authz/go.mod (1 hunks)
- x/bank/go.mod (1 hunks)
- x/circuit/go.mod (2 hunks)
- x/consensus/go.mod (2 hunks)
- x/distribution/go.mod (1 hunks)
- x/epochs/go.mod (1 hunks)
- x/evidence/go.mod (2 hunks)
- x/feegrant/go.mod (2 hunks)
- x/gov/go.mod (2 hunks)
- x/group/go.mod (2 hunks)
- x/mint/go.mod (1 hunks)
- x/nft/go.mod (2 hunks)
- x/params/go.mod (2 hunks)
- x/protocolpool/go.mod (2 hunks)
- x/slashing/go.mod (2 hunks)
- x/staking/go.mod (1 hunks)
- x/upgrade/go.mod (1 hunks)
Files skipped from review due to trivial changes (11)
- client/v2/go.mod
- go.mod
- simapp/go.mod
- tests/go.mod
- x/accounts/defaults/lockup/go.mod
- x/auth/go.mod
- x/circuit/go.mod
- x/distribution/go.mod
- x/evidence/go.mod
- x/feegrant/go.mod
- x/upgrade/go.mod
Files skipped from review as they are similar to previous changes (15)
- server/v2/cometbft/go.mod
- simapp/v2/go.mod
- x/accounts/defaults/multisig/go.mod
- x/accounts/go.mod
- x/authz/go.mod
- x/bank/go.mod
- x/consensus/go.mod
- x/epochs/go.mod
- x/group/go.mod
- x/mint/go.mod
- x/nft/go.mod
- x/params/go.mod
- x/protocolpool/go.mod
- x/slashing/go.mod
- x/staking/go.mod
Additional comments not posted (5)
x/gov/go.mod (5)
84-84: Addition ofgithub.com/go-logr/logris appropriate.The inclusion of
github.com/go-logr/logrenhances structured logging capabilities, aligning with the PR objectives.
85-85: Addition ofgithub.com/go-logr/stdris appropriate.The inclusion of
github.com/go-logr/stdrprovides a standard library implementation for structured logging, aligning with the PR objectives.
161-161: Addition ofgo.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpcis appropriate.The inclusion of
go.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpcenables OpenTelemetry tracing for gRPC, aligning with the PR objectives.
162-162: Addition ofgo.opentelemetry.io/otelis appropriate.The inclusion of
go.opentelemetry.io/otelprovides the core OpenTelemetry API, aligning with the PR objectives.
163-164: Addition ofgo.opentelemetry.io/otel/metricandgo.opentelemetry.io/otel/traceis appropriate.The inclusion of
go.opentelemetry.io/otel/metricandgo.opentelemetry.io/otel/traceenhances metrics and tracing capabilities, aligning with the PR objectives.
|
@julienrbrt R4R 🙏 |
I still stand by this comment: #21029 (comment) |
There was a problem hiding this comment.
Thanks for the PR! Very nice to see telemetry landing in the SDK.
I was wondering about your custom span in the interceptor that seems to be redundant with what comes out of the box with the otel stats_handler .
It would be great if you can elaborate on this a bit more.
Would a custom stats_handler make sense to avoid the interceptor changes? This would be a better decoupling
| @@ -35,6 +36,7 @@ func NewGRPCServer(clientCtx client.Context, app types.Application, cfg config.G | |||
| grpc.ForceServerCodec(codec.NewProtoCodec(clientCtx.InterfaceRegistry).GRPCCodec()), | |||
| grpc.MaxSendMsgSize(maxSendMsgSize), | |||
| grpc.MaxRecvMsgSize(maxRecvMsgSize), | |||
| grpc.StatsHandler(otelgrpc.NewServerHandler()), | |||
| grpcCtx, span := tracer.Start(parentCtx, grpcInfo.FullMethod, trace.WithSpanKind(trace.SpanKindServer)) | ||
| defer span.End() | ||
|
|
||
| span.SetAttributes(attribute.String("http.method", grpcInfo.FullMethod)) |
There was a problem hiding this comment.
Why do you add the http.method attribute? A span for FullMethod is created in the stats handler already
| span.SetStatus(otelcodes.Error, err.Error()) | ||
| } else { | ||
| span.SetStatus(otelcodes.Ok, "OK") | ||
| } |
There was a problem hiding this comment.
The span status is also set in the stats handler. Can you elaborate why you need this here?
github-actions
bot
added
C:server/v2
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yml
Review profile: CHILL
Files ignored due to path filters (2)
client/v2/go.sumis excluded by!**/*.sumgo.sumis excluded by!**/*.sum
Files selected for processing (3)
- CHANGELOG.md (1 hunks)
- client/v2/go.mod (1 hunks)
- go.mod (3 hunks)
Additional context used
Path-based instructions (1)
CHANGELOG.md (1)
Pattern
**/*.md: "Assess the documentation for misspellings, grammatical errors, missing documentation and correctness"
Additional comments not posted (13)
client/v2/go.mod (6)
175-175: LGTM: Addition ofgithub.com/go-logr/logr.The structured logging library
github.com/go-logr/logris a valuable addition for improved logging practices.
176-176: LGTM: Addition ofgithub.com/go-logr/stdr.The adapter for
logrto the standard library enhances logging flexibility.
178-178: LGTM: Addition ofgo.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc.This package supports the integration of OpenTelemetry for gRPC, enhancing observability.
179-179: LGTM: Addition ofgo.opentelemetry.io/otel.The core OpenTelemetry package is essential for tracing and metrics.
180-180: LGTM: Addition ofgo.opentelemetry.io/otel/metric.This package enhances observability by adding metrics capabilities.
181-181: LGTM: Addition ofgo.opentelemetry.io/otel/trace.This package is crucial for implementing tracing capabilities in OpenTelemetry.
go.mod (6)
59-59: LGTM: Addition ofgo.opentelemetry.io/contrib/instrumentation/google.golang.org/grpc/otelgrpc.This package supports the integration of OpenTelemetry for gRPC, enhancing observability.
60-60: LGTM: Addition ofgo.opentelemetry.io/otel.The core OpenTelemetry package is essential for tracing and metrics.
61-61: LGTM: Addition ofgo.opentelemetry.io/otel/trace.This package is crucial for implementing tracing capabilities in OpenTelemetry.
107-107: LGTM: Addition ofgithub.com/go-logr/logr.The structured logging library
github.com/go-logr/logris a valuable addition for improved logging practices.
108-108: LGTM: Addition ofgithub.com/go-logr/stdr.The adapter for
logrto the standard library enhances logging flexibility.
171-171: LGTM: Addition ofgo.opentelemetry.io/otel/metric.This package enhances observability by adding metrics capabilities.
CHANGELOG.md (1)
46-46: Changelog entry looks good.The changelog entry for the OTEL wiring in the gRPC server interceptor is clear and correctly formatted. No grammatical or typographical issues are present.
|
This pull request has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions. |
|
closing due to stale, @romac if you have time to address the comments please reopen the pr |
Description
Closes: #XXXX
This PR wires up OTEL tracing in gprcserver interceptor, allowing tracing requests that originate from external services.
I had to bump up go as 1.20 did not support some of the otel grpc dependencies.
This has led to bumping up go versions of a lot of the CI stuff.
Tested on Osmosis node.
Author Checklist
All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.
I have...
!in the type prefix if API or client breaking changeCHANGELOG.mdReviewers Checklist
All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.
Please see Pull Request Reviewer section in the contributing guide for more information on how to review a pull request.
I have...
Summary by CodeRabbit
New Features
Bug Fixes