feat(opbot): experimental logger #3132
Conversation
golangisfun123
changed the title
WalkthroughThe changes introduce a new logging mechanism to the Changes
Possibly related PRs
Suggested labels
Poem
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
Documentation and Community
|
Deploying sanguine-fe with
|
| Latest commit: |
3d7c677
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://b1630fbc.sanguine-fe.pages.dev |
| Branch Preview URL: | https://log-errors.sanguine-fe.pages.dev |
![coderabbitai[bot]](https://github.com/avatars/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range and nitpick comments (1)
contrib/opbot/botmd/commands.go (1)
343-356: Justify the chosen retry values.Increasing the retry attempts and maximum attempt time can make the process more resilient to transient failures, which is a good improvement.
However, please provide a justification for the specific values chosen (5 attempts and 30 seconds). This will help in understanding the reasoning behind these values and ensure that they are suitable for the given use case.
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (3)
- contrib/opbot/botmd/botmd.go (2 hunks)
- contrib/opbot/botmd/commands.go (3 hunks)
- contrib/opbot/go.mod (1 hunks)
Additional comments not posted (8)
contrib/opbot/botmd/botmd.go (4)
14-14: LGTM!The import statement for the experimental logger looks good. The alias and package path are appropriate.
20-20: Looks good! Theotelzapimport will enable OpenTelemetry instrumentation for Zap logging.The
otelzappackage is an extension that allows emitting OpenTelemetry traces and metrics from the Zap logging library. This aligns with the PR objective of introducing an experimental logger for debugging purposes.By leveraging
otelzap, the bot will gain the ability to correlate logs with traces and metrics, enabling better observability and debugging capabilities.
35-35: The logger field addition and initialization look good!The
Botstruct now includes anExperimentalLoggerfield, which is initialized in theNewBotfunction using the following steps:
- A new
zap.Loggerinstance is created usingexperimentalLogger.MakeZapLogger().- The
zap.Loggeris wrapped withotelzap.New()to enable OpenTelemetry instrumentation.- The wrapped logger is then converted to a sugared logger using
.Sugar().- Finally, the sugared logger is wrapped again using
experimentalLogger.MakeWrappedSugaredLogger()to create the finalExperimentalLoggerinstance.This initialization process ensures that the bot's logger is properly set up with the experimental logging capabilities and OpenTelemetry instrumentation.
Also applies to: 48-48
41-43: This code segment is part of the logger initialization process that was already covered in the previous review comment. No further analysis is needed.contrib/opbot/botmd/commands.go (3)
85-85: LGTM!The error logging is appropriate and helps in diagnosing issues when searching for a transaction fails. The log message provides relevant context about the failure.
294-294: LGTM!The error logging is appropriate and helps in diagnosing issues when fetching a quote request fails. The log message provides relevant context about the failure.
359-359: LGTM!The error logging is appropriate and helps in diagnosing issues when fetching a quote request fails after retries. The log message provides relevant context about the failure.
contrib/opbot/go.mod (1)
31-31: LGTM!The addition of the
github.com/uptrace/opentelemetry-go-extra/otelzapdependency in therequiresection is a good practice, as it explicitly declares the direct usage of this package in the project. The chosen versionv0.3.1also seems to be a stable release.
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #3132 +/- ##
===================================================
+ Coverage 36.66888% 37.95807% +1.28918%
===================================================
Files 443 418 -25
Lines 25643 24232 -1411
Branches 119 82 -37
===================================================
- Hits 9403 9198 -205
+ Misses 15498 14295 -1203
+ Partials 742 739 -3
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
Description
A clear and concise description of the features you're adding in this pull request.
Additional context
Add any other context about the problem you're solving.
Metadata
Summary by CodeRabbit
New Features
Improvements
Dependency Updates