rookout #2991
rookout #2991
Conversation
trajan0x
requested review from
dwasse,
aureliusbtc and
ChiTimesChi
as code owners
|
Warning Rate limit exceeded@trajan0x has exceeded the limit for the number of commits or files that can be reviewed per hour. Please wait 3 minutes and 3 seconds before requesting another review. How to resolve this issue?After the wait time has elapsed, a review can be triggered using the We recommend that you space out your commits to avoid hitting the rate limit. How do rate limits work?CodeRabbit enforces hourly rate limits for each developer per organization. Our paid plans have higher rate limits than the trial, open-source and free plans. In all cases, we re-allow further reviews after a brief timeout. Please see our FAQ for further information. WalkthroughThe recent updates enhance build configurations across various components by introducing debugging flags and new indirect dependencies to support Rookout integration. Formatting adjustments in Dockerfiles and documentation clarify configuration requirements, contributing to a more maintainable and flexible codebase. Additionally, changes to versioning information allow for dynamic updates, empowering developers with improved traceability. Changes
Sequence Diagram(s)sequenceDiagram
participant Developer
participant CI/CD
participant Docker
participant Rookout
Developer->>CI/CD: Push code changes
CI/CD->>Docker: Build images with new configurations
Docker->>Rookout: Initialize monitoring with Rookout token
Rookout-->>Docker: Ready for monitoring
Docker-->>CI/CD: Images built successfully
CI/CD-->>Developer: Notification of successful build
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
go
Deploying sanguine-fe with
|
| Latest commit: |
de6f622
|
| Status: | ✅ Deploy successful! |
| Preview URL: | https://99ffe8b7.sanguine-fe.pages.dev |
| Branch Preview URL: | https://feat-stage-rookout.sanguine-fe.pages.dev |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2991 +/- ##
===================================================
- Coverage 25.24881% 24.93629% -0.31252%
===================================================
Files 780 800 +20
Lines 56771 58080 +1309
Branches 82 82
===================================================
+ Hits 14334 14483 +149
- Misses 40958 42107 +1149
- Partials 1479 1490 +11
Flags with carried forward coverage won't be shown. Click here to find out more. ☔ View full report in Codecov by Sentry. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 5
Outside diff range, codebase verification and nitpick comments (1)
core/metrics/rookout.go (1)
16-26: Lack of test coverage.The added lines in
core/metrics/rookout.goare not covered by tests. Ensure that test cases are added to cover this functionality.
core/metrics/rookout.go: Lines 16-26Analysis chain
Lack of test coverage.
The added lines are not covered by tests. Ensure that test cases are added to cover this functionality.
Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the test coverage of the new Rookout integration. # Test: Search for test cases related to Rookout integration. rg --type go -A 5 'Rookout'Length of output: 2163
Script:
#!/bin/bash # Description: Verify the test coverage of the new Rookout integration. # Search for test files that might cover the Rookout integration. rg --type go --files-with-matches '_test\.go' # Search for test functions that might cover the Rookout integration. rg --type go '^func Test' -A 5Length of output: 99461
Tools
GitHub Check: codecov/patch
[warning] 16-26: core/metrics/rookout.go#L16-L26
Added lines #L16 - L26 were not covered by tests
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (12)
agents/go.sumis excluded by!**/*.sumcontrib/opbot/go.sumis excluded by!**/*.sumcontrib/promexporter/go.sumis excluded by!**/*.sumcontrib/screener-api/go.sumis excluded by!**/*.sumcore/go.sumis excluded by!**/*.sumethergo/go.sumis excluded by!**/*.sumgo.work.sumis excluded by!**/*.sumservices/cctp-relayer/go.sumis excluded by!**/*.sumservices/explorer/go.sumis excluded by!**/*.sumservices/omnirpc/go.sumis excluded by!**/*.sumservices/rfq/go.sumis excluded by!**/*.sumservices/scribe/go.sumis excluded by!**/*.sum
Files selected for processing (37)
- .dockerignore (1 hunks)
- agents/.goreleaser.yml (1 hunks)
- agents/go.mod (3 hunks)
- contrib/git-changes-action/.goreleaser.yml (1 hunks)
- contrib/opbot/.goreleaser.yml (2 hunks)
- contrib/opbot/go.mod (7 hunks)
- contrib/promexporter/.goreleaser.yml (1 hunks)
- contrib/promexporter/go.mod (7 hunks)
- contrib/screener-api/.goreleaser.yml (1 hunks)
- contrib/screener-api/go.mod (5 hunks)
- core/go.mod (5 hunks)
- core/metrics/README.md (1 hunks)
- core/metrics/base.go (1 hunks)
- core/metrics/internal/const.go (1 hunks)
- core/metrics/rookout.go (1 hunks)
- docker/agents.Dockerfile (1 hunks)
- docker/explorer.Dockerfile (1 hunks)
- docker/git-changes-action.Dockerfile (1 hunks)
- docker/omnirpc.Dockerfile (1 hunks)
- docker/opbot.Dockerfile (1 hunks)
- docker/promexporter.Dockerfile (1 hunks)
- docker/rfq-api.Dockerfile (1 hunks)
- docker/rfq-relayer.Dockerfile (1 hunks)
- docker/screener-api.Dockerfile (1 hunks)
- docker/scribe.Dockerfile (1 hunks)
- ethergo/.goreleaser.yml (1 hunks)
- ethergo/go.mod (3 hunks)
- services/cctp-relayer/.goreleaser.yml (1 hunks)
- services/cctp-relayer/go.mod (4 hunks)
- services/explorer/.goreleaser.yml (1 hunks)
- services/explorer/go.mod (5 hunks)
- services/omnirpc/.goreleaser.yml (1 hunks)
- services/omnirpc/go.mod (5 hunks)
- services/rfq/.goreleaser.yml (3 hunks)
- services/rfq/go.mod (5 hunks)
- services/scribe/.goreleaser.yml (1 hunks)
- services/scribe/go.mod (5 hunks)
Files skipped from review due to trivial changes (14)
- .dockerignore
- contrib/screener-api/go.mod
- core/metrics/README.md
- docker/agents.Dockerfile
- docker/explorer.Dockerfile
- docker/git-changes-action.Dockerfile
- docker/omnirpc.Dockerfile
- docker/opbot.Dockerfile
- docker/promexporter.Dockerfile
- docker/rfq-api.Dockerfile
- docker/rfq-relayer.Dockerfile
- docker/scribe.Dockerfile
- services/cctp-relayer/go.mod
- services/explorer/go.mod
Additional context used
GitHub Check: Lint (core)
core/metrics/internal/const.go
[failure] 16-16:
Comment should end in a period (godot)core/metrics/rookout.go
[failure] 13-13:
Comment should end in a period (godot)
GitHub Check: codecov/patch
core/metrics/rookout.go
[warning] 16-26: core/metrics/rookout.go#L16-L26
Added lines #L16 - L26 were not covered by tests
Additional comments not posted (70)
docker/screener-api.Dockerfile (1)
15-17: Addition of ENTRYPOINT directive.The addition of the
ENTRYPOINTdirective is a good practice as it specifies the command to run the application when the container starts.core/metrics/internal/const.go (1)
15-17: Addition of Rookout constants.The new constants
RookoutTokenandRookoutDebugare well-defined and appropriately named for Rookout integration.Tools
GitHub Check: Lint (core)
[failure] 16-16:
Comment should end in a period (godot)contrib/git-changes-action/.goreleaser.yml (1)
12-13: Enhance debugging capabilities withgcflags.The addition of
gcflagswith-all=-dwarflocationlists=trueimproves the handling of DWARF location lists, enhancing debugging capabilities. Ensure this change aligns with the intended debugging requirements.services/scribe/.goreleaser.yml (1)
11-12: Enhance debugging capabilities withgcflags.The addition of
gcflagswith-all=-dwarflocationlists=trueimproves the handling of DWARF location lists, enhancing debugging capabilities. Ensure this change aligns with the intended debugging requirements.services/explorer/.goreleaser.yml (1)
12-13: Enhance debugging capabilities withgcflags.The addition of
gcflagswith-all=-dwarflocationlists=trueimproves the handling of DWARF location lists, enhancing debugging capabilities. Ensure this change aligns with the intended debugging requirements.contrib/promexporter/.goreleaser.yml (1)
11-12: Enhance debugging capabilities with gcflags.The addition of
gcflagswith-all=-dwarflocationlists=trueenhances the debugging capabilities by controlling the generation of DWARF location lists. This is a beneficial change for developers when analyzing stack traces and debugging.services/cctp-relayer/.goreleaser.yml (1)
11-12: Enhance debugging capabilities with gcflags.The addition of
gcflagswith-all=-dwarflocationlists=trueenhances the debugging capabilities by controlling the generation of DWARF location lists. This is a beneficial change for developers when analyzing stack traces and debugging.agents/.goreleaser.yml (2)
11-12: Enhance debugging capabilities with gcflags.The addition of
gcflagswith-all=-dwarflocationlists=trueenhances the debugging capabilities by controlling the generation of DWARF location lists. This is a beneficial change for developers when analyzing stack traces and debugging.
15-15: Embed versioning details into the binary with ldflags.The addition of
ldflagsto embed versioning details (DefaultVersion,DefaultCommit,DefaultDate) into the binary is a good practice. It ensures that the resulting binary carries specific metadata regarding its version and commit information, which is crucial for debugging and tracking deployments.services/omnirpc/.goreleaser.yml (2)
12-13: LGTM! Addition ofgcflagsfor enhanced debugging.The
gcflagsentry-all=-dwarflocationlists=truewill improve debugging information.
16-16: LGTM! Expansion ofldflagsfor embedding versioning information.The expanded
ldflagsincludeDefaultVersion,DefaultCommit, andDefaultDate, which enhance the traceability of the binary.contrib/screener-api/.goreleaser.yml (2)
11-12: LGTM! Addition ofgcflagsfor enhanced debugging.The
gcflagsentry-all=-dwarflocationlists=truewill improve debugging information.
15-15: LGTM! Expansion ofldflagsfor embedding versioning information.The expanded
ldflagsincludeDefaultVersion,DefaultCommit, andDefaultDate, which enhance the traceability of the binary.ethergo/.goreleaser.yml (2)
11-12: LGTM! Addition ofgcflagsfor enhanced debugging.The
gcflagsentry-all=-dwarflocationlists=truewill improve debugging information.
15-15: LGTM! Expansion ofldflagsfor embedding versioning information.The expanded
ldflagsincludeDefaultVersion,DefaultCommit, andDefaultDate, which enhance the traceability of the binary.contrib/opbot/.goreleaser.yml (3)
13-13: Enhance versioning metadata in build configuration.The added
ldflagsembed versioning information directly into the compiled output, which is useful for tracking versions and commits in deployment contexts. This is a good practice for maintainability.
14-15: Improve debugging capabilities with GC flags.The addition of
gcflags: - all=-dwarflocationlists=trueenhances the debugging information, which can be beneficial for troubleshooting.
36-36: Enhance versioning metadata in build configuration.The added
ldflagsembed versioning information directly into the compiled output, which is useful for tracking versions and commits in deployment contexts. This is a good practice for maintainability.services/rfq/.goreleaser.yml (6)
11-12: Improve debugging capabilities with GC flags.The addition of
gcflags: - all=-dwarflocationlists=trueenhances the debugging information, which can be beneficial for troubleshooting.
15-15: Enhance versioning metadata in build configuration.The added
ldflagsembed versioning information directly into the compiled output, which is useful for tracking versions and commits in deployment contexts. This is a good practice for maintainability.
35-36: Improve debugging capabilities with GC flags.The addition of
gcflags: - all=-dwarflocationlists=trueenhances the debugging information, which can be beneficial for troubleshooting.
39-39: Enhance versioning metadata in build configuration.The added
ldflagsembed versioning information directly into the compiled output, which is useful for tracking versions and commits in deployment contexts. This is a good practice for maintainability.
54-55: Improve debugging capabilities with GC flags.The addition of
gcflags: - all=-dwarflocationlists=trueenhances the debugging information, which can be beneficial for troubleshooting.
58-58: Enhance versioning metadata in build configuration.The added
ldflagsembed versioning information directly into the compiled output, which is useful for tracking versions and commits in deployment contexts. This is a good practice for maintainability.core/metrics/base.go (1)
216-217: Integrate new function call for Rookout.The call to
startRookout()is added to thenewBaseHandlerfunction. Ensure that the function is implemented correctly to avoid potential runtime errors.core/go.mod (7)
12-12: Dependency Added:github.com/Rookout/GoSDK v0.1.49The Rookout Go SDK is added, which should enhance debugging and observability capabilities.
96-96: Dependency Added:github.com/fallais/logrus-lumberjack-hook v0.0.0-20210917073259-3227e1ab93b0This hook integrates Logrus with Lumberjack for log rotation, which should help manage log files efficiently.
113-113: Dependency Added:github.com/golang/protobuf v1.5.4The golang/protobuf library is essential for handling protocol buffers in Go.
118-118: Dependency Added:github.com/hashicorp/golang-lru v0.5.4The golang-lru library provides an efficient LRU cache implementation.
170-170: Dependency Added:github.com/yhirose/go-peg v0.0.0-20210804202551-de25d6753cf1The go-peg library is useful for parsing expressions efficiently.
181-181: Dependency Added:golang.org/x/time v0.3.0The golang/x/time library provides additional time-related utilities.
187-187: Dependency Added:gopkg.in/natefinch/lumberjack.v2 v2.0.0The lumberjack library is essential for managing log rotation efficiently.
contrib/promexporter/go.mod (7)
57-57: Dependency Added:github.com/Rookout/GoSDK v0.1.49The Rookout Go SDK is added, which should enhance debugging and observability capabilities.
97-97: Dependency Added:github.com/fallais/logrus-lumberjack-hook v0.0.0-20210917073259-3227e1ab93b0This hook integrates Logrus with Lumberjack for log rotation, which should help manage log files efficiently.
110-110: Dependency Added:github.com/go-errors/errors v1.4.2The go-errors library provides enhanced error handling capabilities.
127-127: Dependency Added:github.com/golang/protobuf v1.5.4The golang/protobuf library is essential for handling protocol buffers in Go.
194-194: Dependency Added:github.com/sirupsen/logrus v1.9.3The logrus library is widely used for logging in Go applications.
215-215: Dependency Added:github.com/yhirose/go-peg v0.0.0-20210804202551-de25d6753cf1The go-peg library is useful for parsing expressions efficiently.
245-245: Dependency Added:gopkg.in/natefinch/lumberjack.v2 v2.0.0The lumberjack library is essential for managing log rotation efficiently.
services/omnirpc/go.mod (5)
64-64: Dependency Added:github.com/Rookout/GoSDK v0.1.49The Rookout Go SDK is added, which should enhance debugging and observability capabilities.
108-108: Dependency Added:github.com/fallais/logrus-lumberjack-hook v0.0.0-20210917073259-3227e1ab93b0This hook integrates Logrus with Lumberjack for log rotation, which should help manage log files efficiently.
121-121: Dependency Added:github.com/go-errors/errors v1.4.2The go-errors library provides enhanced error handling capabilities.
138-138: Dependency Added:github.com/golang/protobuf v1.5.4The golang/protobuf library is essential for handling protocol buffers in Go.
243-243: Dependency Added:github.com/yhirose/go-peg v0.0.0-20210804202551-de25d6753cf1The go-peg library is useful for parsing expressions efficiently.
ethergo/go.mod (3)
93-93: Verify the necessity and usage ofgithub.aaakk.us.kg/Rookout/GoSDK.Ensure that the addition of the Rookout SDK aligns with the project's goals and does not introduce any security or performance issues.
143-143: Verify the necessity and usage ofgithub.aaakk.us.kg/fallais/logrus-lumberjack-hook.Ensure that the addition of the Logrus Lumberjack hook aligns with the project's logging requirements and does not introduce any potential issues.
262-262: Verify the necessity and usage ofgithub.aaakk.us.kg/yhirose/go-peg.Ensure that the addition of the go-peg library aligns with the project's parsing requirements and does not introduce any potential issues.
contrib/opbot/go.mod (7)
58-58: Verify the necessity and usage ofgithub.aaakk.us.kg/Rookout/GoSDK.Ensure that the addition of the Rookout SDK aligns with the project's goals and does not introduce any security or performance issues.
120-120: Verify the necessity and usage ofgithub.aaakk.us.kg/fallais/logrus-lumberjack-hook.Ensure that the addition of the Logrus Lumberjack hook aligns with the project's logging requirements and does not introduce any potential issues.
132-132: Verify the necessity and usage ofgithub.aaakk.us.kg/go-errors/errors.Ensure that the addition of the go-errors library aligns with the project's error handling requirements and does not introduce any potential issues.
164-164: Verify the necessity and usage ofgithub.aaakk.us.kg/hashicorp/golang-lru.Ensure that the addition of the golang-lru library aligns with the project's caching requirements and does not introduce any potential issues.
226-226: Verify the necessity and usage ofgithub.aaakk.us.kg/sirupsen/logrus.Ensure that the addition of the Logrus library aligns with the project's logging requirements and does not introduce any potential issues.
250-250: Verify the necessity and usage ofgithub.aaakk.us.kg/yhirose/go-peg.Ensure that the addition of the go-peg library aligns with the project's parsing requirements and does not introduce any potential issues.
285-285: Verify the necessity and usage ofgopkg.in/natefinch/lumberjack.v2.Ensure that the addition of the Lumberjack library aligns with the project's logging requirements and does not introduce any potential issues.
services/scribe/go.mod (5)
83-83: Verify the necessity and usage ofgithub.aaakk.us.kg/Rookout/GoSDK.Ensure that the addition of the Rookout SDK aligns with the project's goals and does not introduce any security or performance issues.
136-136: Verify the necessity and usage ofgithub.aaakk.us.kg/fallais/logrus-lumberjack-hook.Ensure that the addition of the Logrus Lumberjack hook aligns with the project's logging requirements and does not introduce any potential issues.
151-151: Verify the necessity and usage ofgithub.aaakk.us.kg/go-errors/errors.Ensure that the addition of the go-errors library aligns with the project's error handling requirements and does not introduce any potential issues.
169-169: Verify the necessity and usage ofgithub.aaakk.us.kg/golang/protobuf.Ensure that the addition of the golang/protobuf library aligns with the project's requirements and does not introduce any potential issues.
280-280: Verify the necessity and usage ofgithub.aaakk.us.kg/yhirose/go-peg.Ensure that the addition of the go-peg library aligns with the project's parsing requirements and does not introduce any potential issues.
services/rfq/go.mod (7)
9-9: Approved: Addition ofgithub.aaakk.us.kg/alecthomas/assert v1.0.0.This package is useful for assertions in tests and aligns with the PR's focus on test builds.
66-66: Approved: Addition ofgithub.aaakk.us.kg/Rookout/GoSDK v0.1.49.This package is related to the Rookout integration, which is the focus of this PR.
69-69: Approved: Addition ofgithub.aaakk.us.kg/alecthomas/colour v0.1.0.This package is useful for colorizing terminal output, enhancing the readability of logs and test results.
70-70: Approved: Addition ofgithub.aaakk.us.kg/alecthomas/repr v0.0.0-20210801044451-80ca428c5142.This package is useful for pretty-printing Go data structures, aiding in debugging and logging.
138-138: Approved: Addition ofgithub.aaakk.us.kg/fallais/logrus-lumberjack-hook v0.0.0-20210917073259-3227e1ab93b0.This package integrates Logrus with Lumberjack for log rotation, which is useful for managing log files.
152-152: Approved: Addition ofgithub.aaakk.us.kg/go-errors/errors v1.4.2.This package provides enhanced error handling capabilities, improving the robustness of the code.
293-293: Approved: Addition ofgithub.aaakk.us.kg/yhirose/go-peg v0.0.0-20210804202551-de25d6753cf1.This package is useful for parsing PEG grammars, aiding in complex parsing tasks.
agents/go.mod (4)
54-54: Approved: Addition ofgithub.aaakk.us.kg/Rookout/GoSDK v0.1.49.This package is related to the Rookout integration, which is the focus of this PR.
76-76: Approved: Addition ofgithub.aaakk.us.kg/fallais/logrus-lumberjack-hook v0.0.0-20210917073259-3227e1ab93b0.This package integrates Logrus with Lumberjack for log rotation, which is useful for managing log files.
82-82: Approved: Addition ofgithub.aaakk.us.kg/go-errors/errors v1.4.2.This package provides enhanced error handling capabilities, improving the robustness of the code.
107-107: Approved: Addition ofgithub.aaakk.us.kg/yhirose/go-peg v0.0.0-20210804202551-de25d6753cf1.This package is useful for parsing PEG grammars, aiding in complex parsing tasks.
core/metrics/internal/const.go
Outdated
| // RookoutToken is used for https://app.rookout.com/ | ||
| RookoutToken = "ROOKOUT_TOKEN" | ||
| // RookoutDebug can be used to enable rookout debug | ||
| RookoutDebug = "ROOKOUT_DEBUG" |
There was a problem hiding this comment.
Add a period at the end of the comment.
To adhere to the linting rules, the comment on line 16 should end with a period.
- // RookoutDebug can be used to enable rookout debug
+ // RookoutDebug can be used to enable rookout debug.Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // RookoutToken is used for https://app.rookout.com/ | |
| RookoutToken = "ROOKOUT_TOKEN" | |
| // RookoutDebug can be used to enable rookout debug | |
| RookoutDebug = "ROOKOUT_DEBUG" | |
| // RookoutToken is used for https://app.rookout.com/ | |
| RookoutToken = "ROOKOUT_TOKEN" | |
| // RookoutDebug can be used to enable rookout debug. | |
| RookoutDebug = "ROOKOUT_DEBUG" |
Tools
GitHub Check: Lint (core)
[failure] 16-16:
Comment should end in a period (godot)
core/metrics/rookout.go
Outdated
| func init() { | ||
| if core.HasEnv(internal.RookoutToken) { | ||
| // TODO: Consider doing git stuff here. | ||
| err := rookout.Start(config.RookOptions{ | ||
| Token: os.Getenv(internal.RookoutToken), | ||
| Debug: core.HasEnv(internal.RookoutDebug), | ||
| GitCommit: synconfig.DefaultCommit, | ||
| // note: will chagne on fork | ||
| GitOrigin: "https://github.com/synapsecns/sanguine", | ||
| }) | ||
| if err != nil { | ||
| logger.Warn(err) | ||
| } | ||
| } |
There was a problem hiding this comment.
Consider addressing TODO comments.
There are TODO comments indicating areas for potential improvement. Address these comments to enhance the code quality.
Do you want me to help address the TODO comments or open a GitHub issue to track these tasks?
Tools
GitHub Check: codecov/patch
[warning] 16-26: core/metrics/rookout.go#L16-L26
Added lines #L16 - L26 were not covered by tests
| func startRookout() { | ||
|
|
||
| } |
There was a problem hiding this comment.
Implement the startRookout function.
The startRookout function is defined but currently lacks implementation. This needs to be implemented to ensure the intended functionality is achieved.
Would you like assistance in implementing this function or opening a GitHub issue to track this task?
core/metrics/rookout.go
Outdated
| // rookout is sideloaded. | ||
| // TODO: consider moving this to metrics |
There was a problem hiding this comment.
Add a period at the end of the comment.
To adhere to the linting rules, the comment on line 13 should end with a period.
-// TODO: consider moving this to metrics
+// TODO: consider moving this to metrics.Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| // rookout is sideloaded. | |
| // TODO: consider moving this to metrics | |
| // rookout is sideloaded. | |
| // TODO: consider moving this to metrics. |
Tools
GitHub Check: Lint (core)
[failure] 13-13:
Comment should end in a period (godot)
core/metrics/rookout.go
Outdated
| err := rookout.Start(config.RookOptions{ | ||
| Token: os.Getenv(internal.RookoutToken), | ||
| Debug: core.HasEnv(internal.RookoutDebug), | ||
| GitCommit: synconfig.DefaultCommit, | ||
| // note: will chagne on fork | ||
| GitOrigin: "https://github.com/synapsecns/sanguine", | ||
| }) | ||
| if err != nil { | ||
| logger.Warn(err) | ||
| } |
There was a problem hiding this comment.
Ensure proper error handling.
The current error handling logs a warning. Consider handling the error more robustly, depending on the criticality of the Rookout integration.
- if err != nil {
- logger.Warn(err)
- }
+ if err != nil {
+ logger.Error("Failed to start Rookout:", err)
+ // Consider adding additional error handling logic here.
+ }Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| err := rookout.Start(config.RookOptions{ | |
| Token: os.Getenv(internal.RookoutToken), | |
| Debug: core.HasEnv(internal.RookoutDebug), | |
| GitCommit: synconfig.DefaultCommit, | |
| // note: will chagne on fork | |
| GitOrigin: "https://github.com/synapsecns/sanguine", | |
| }) | |
| if err != nil { | |
| logger.Warn(err) | |
| } | |
| err := rookout.Start(config.RookOptions{ | |
| Token: os.Getenv(internal.RookoutToken), | |
| Debug: core.HasEnv(internal.RookoutDebug), | |
| GitCommit: synconfig.DefaultCommit, | |
| // note: will chagne on fork | |
| GitOrigin: "https://github.com/synapsecns/sanguine", | |
| }) | |
| if err != nil { | |
| logger.Error("Failed to start Rookout:", err) | |
| // Consider adding additional error handling logic here. | |
| } |
Tools
GitHub Check: codecov/patch
[warning] 16-26: core/metrics/rookout.go#L16-L26
Added lines #L16 - L26 were not covered by tests
![greptile-apps[bot]](https://avatars.githubusercontent.com/in/867647?s=60&v=4){kind=small}
There was a problem hiding this comment.
PR Summary
The pull request integrates Rookout for debugging and logging, updating various GoReleaser configurations, Dockerfiles, and Go module dependencies.
- agents/.goreleaser.yml: Added
gcflagsand enhancedldflagsfor better debugging and traceability. - core/metrics/rookout.go: New file for Rookout integration, initializing based on environment variables.
- docker/*.Dockerfile: Added
COPY .git /.gitto multiple Dockerfiles, potentially increasing image size and exposing sensitive information. - agents/go.mod: Added dependencies for Rookout and logging enhancements.
- core/go.sum: Updated to reflect new dependencies, ensuring compatibility and security.
30 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
PR Summary
(updates since last review)
The pull request focuses on integrating Rookout for debugging and logging, updating GoReleaser configurations, and refining Dockerfiles to improve security and efficiency.
- core/metrics/rookout.go: Added
GitCommitandGitOriginfields toRookOptionsfor better debugging. - agents/.goreleaser.yml: Updated
ldflagsto useDefaultVersion,DefaultCommit, andDefaultDatefromcore/config. - docker/*.Dockerfile: Removed
.gitdirectory copy step to reduce image size and improve security. - contrib/opbot/.goreleaser.yml: Updated
ldflagsfor consistency in versioning and commit information. - services/omnirpc/.goreleaser.yml: Updated
ldflagsto use centralized configuration for build metadata.
19 file(s) reviewed, 2 comment(s)
Edit PR Review Bot Settings
core/metrics/rookout.go
Outdated
| Token: os.Getenv(internal.RookoutToken), | ||
| Debug: core.HasEnv(internal.RookoutDebug), | ||
| GitCommit: synconfig.DefaultCommit, | ||
| // note: will chagne on fork |
There was a problem hiding this comment.
syntax: Typo in comment: 'chagne' should be 'change'.
| @@ -12,4 +12,6 @@ USER nonroot:nonroot | |||
| WORKDIR /app | |||
| COPY --chown=nonroot:nonroot promexporter /app/promexporter | |||
|
|
|||
|
|
|||
There was a problem hiding this comment.
logic: Copying the entire .git directory into the Docker image can expose sensitive information and increase the image size unnecessarily. Ensure this is absolutely necessary.
There was a problem hiding this comment.
PR Summary
(updates since last review)
The pull request integrates Rookout for enhanced debugging and updates GoReleaser configurations across multiple components.
- contrib/opbot/.goreleaser.yml: Updated
gcflagsto-dwarflocationlists=truefor better debugging. - .github/workflows/goreleaser-actions.yml: Added steps for building and pushing Docker images, including caching and environment variable setup.
- services/omnirpc/.goreleaser.yml: Updated
ldflagsfor consistent versioning and commit information. - agents/.goreleaser.yml: Similar updates to
ldflagsfor centralized build metadata. - docker/goreleaser: Dockerfile improvements for better readability and security.
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- contrib/opbot/.goreleaser.yml (2 hunks)
Additional comments not posted (2)
contrib/opbot/.goreleaser.yml (2)
13-15: Approved: Enhanced versioning and debugging capabilities.The
ldflagschanges to embed versioning information are beneficial for traceability. The addition ofgcflagsmay improve debugging capabilities. Ensure that the impact ofgcflagson build performance and debugging is as expected.
36-36: Approved: Consistent versioning and debugging enhancements.The
ldflagsandgcflagschanges are consistent with theopbotconfiguration, providing similar benefits in terms of traceability and debugging.
There was a problem hiding this comment.
PR Summary
(updates since last review)
The pull request integrates Rookout for enhanced debugging and updates GoReleaser configurations across multiple components.
- core/config/build-info.go: Modified
DefaultVersionandDefaultCommitfrom constants to variables for dynamic runtime or build-time setting. - contrib/opbot/.goreleaser.yml: Added
gcflagsfor better debugging and updatedldflagsfor consistent versioning. - .github/workflows/goreleaser-actions.yml: Enhanced Docker build and push steps, including caching and environment variable setup.
- services/omnirpc/.goreleaser.yml: Updated
ldflagsfor consistent versioning and commit information. - agents/.goreleaser.yml: Similar updates to
ldflagsfor centralized build metadata.
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
core/config/build-info.go (1)
16-16: Reconsider changingDefaultVersionfrom a constant to a variable.The usage of
DefaultVersionacross the codebase primarily involves initializing or assigning a version value without any indication of runtime modification. The change fromconsttovardoes not seem necessary and could introduce risks of unintended modifications. Consider reverting it back to a constant unless there's a specific need for this flexibility.
DefaultVersionis used in multiple files for version initialization or assignment.- No evidence was found that requires
DefaultVersionto be mutable at runtime.Analysis chain
Consider the implications of changing
DefaultVersionto a variable.Changing
DefaultVersionfrom a constant to a variable allows its value to be modified at runtime, which increases flexibility but also introduces potential risks of accidental modification. Ensure this change is necessary and consider documenting any safeguards or intended use cases.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if the change from const to var for DefaultVersion is necessary. # Test: Search for the usage of DefaultVersion. Expect: Usage patterns that require runtime modification. rg --type go 'DefaultVersion'Length of output: 2019
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- core/config/build-info.go (1 hunks)
Additional comments not posted (1)
core/config/build-info.go (1)
20-20: Consider the implications of changingDefaultCommitto a variable.Changing
DefaultCommitfrom a constant to a variable allows its value to be modified at runtime, which increases flexibility but also introduces potential risks of accidental modification. Ensure this change is necessary and consider documenting any safeguards or intended use cases.Verification successful
Consider the implications of changing
DefaultCommitto a variable.The transition from a constant to a variable for
DefaultCommitallows its value to be modified at runtime. This change increases flexibility but also introduces potential risks of accidental modification. Ensure this change is necessary and consider documenting any safeguards or intended use cases. Given its widespread usage, clarity on the rationale for this change is essential.
- Files with potential impact:
tools/modulecopier/main.goservices/scribe/metadata/metadata.gocore/metrics/metrics_test.gocore/config/build-info.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify if the change from const to var for DefaultCommit is necessary. # Test: Search for the usage of DefaultCommit. Expect: Usage patterns that require runtime modification. rg --type go 'DefaultCommit'Length of output: 2295
There was a problem hiding this comment.
PR Summary
(updates since last review)
The pull request integrates Rookout for enhanced debugging and updates GoReleaser configurations across multiple components.
- contrib/opbot/.goreleaser.yml: Added
gcflagsfor better debugging and updatedldflagsfor consistent versioning. - .github/workflows/goreleaser-actions.yml: Enhanced Docker build and push steps, including caching and environment variable setup.
- services/omnirpc/.goreleaser.yml: Updated
ldflagsfor consistent versioning and commit information. - agents/.goreleaser.yml: Similar updates to
ldflagsfor centralized build metadata.
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- contrib/opbot/.goreleaser.yml (2 hunks)
Additional comments not posted (3)
contrib/opbot/.goreleaser.yml (3)
13-13: Enhance traceability with versioning metadata inldflags.The addition of linker flags to embed versioning metadata (
DefaultVersion,DefaultCommit,DefaultDate) into theopbotbinary enhances traceability and deployment context awareness. Ensure that these values are correctly populated during the build process.
14-15: Improve debugging withgcflags.The introduction of
gcflagswith-dwarflocationlists=truecan enhance debugging capabilities by providing more detailed debugging information. Ensure that this change does not negatively impact the build performance or binary size.
36-36: Enhance traceability with versioning metadata inldflags.The addition of linker flags to embed versioning metadata (
DefaultVersion,DefaultCommit,DefaultDate) into thesignozexamplebinary enhances traceability and deployment context awareness. Ensure that these values are correctly populated during the build process.
There was a problem hiding this comment.
PR Summary
(updates since last review)
The pull request integrates Rookout for enhanced debugging and updates GoReleaser configurations across multiple components.
- contrib/opbot/.goreleaser.yml: Removed unnecessary
ldflagsand correctedgcflagssyntax foropbotandsignozexamplebinaries. - .github/workflows/goreleaser-actions.yml: Enhanced Docker build and push steps, including caching and environment variable setup.
- services/omnirpc/.goreleaser.yml: Updated
ldflagsfor consistent versioning and commit information. - agents/.goreleaser.yml: Similar updates to
ldflagsfor centralized build metadata.
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (1)
- contrib/opbot/.goreleaser.yml (2 hunks)
Files skipped from review as they are similar to previous changes (1)
- contrib/opbot/.goreleaser.yml
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (6)
- agents/.goreleaser.yml (1 hunks)
- contrib/opbot/.goreleaser.yml (2 hunks)
- contrib/screener-api/.goreleaser.yml (1 hunks)
- ethergo/.goreleaser.yml (1 hunks)
- services/omnirpc/.goreleaser.yml (1 hunks)
- services/rfq/.goreleaser.yml (3 hunks)
Files skipped from review as they are similar to previous changes (3)
- agents/.goreleaser.yml
- contrib/screener-api/.goreleaser.yml
- services/rfq/.goreleaser.yml
Additional comments not posted (3)
services/omnirpc/.goreleaser.yml (1)
12-16: Enhancements to build configuration approved.The addition of
gcflagsand modifications toldflagsimprove debugging capabilities and embed versioning information, enhancing traceability and maintainability.ethergo/.goreleaser.yml (1)
11-15: Enhancements to build configuration approved.The addition of
gcflagsand modifications toldflagsimprove debugging capabilities and embed versioning information, enhancing traceability and maintainability.contrib/opbot/.goreleaser.yml (1)
13-15: Enhancements to build configuration approved.The addition of
gcflagsand modifications toldflagsfor bothopbotandsignozexamplebinaries improve debugging capabilities and embed versioning information, enhancing traceability and maintainability.Also applies to: 36-36
There was a problem hiding this comment.
PR Summary
(updates since last review)
The pull request integrates Rookout for enhanced debugging and updates GoReleaser configurations across multiple components.
- contrib/opbot/.goreleaser.yml: Added
gcflagsfor improved debugging and updated Docker image metadata labels. - services/rfq/.goreleaser.yml: Removed
-s -wflags fromldflagsto include debugging information inapi,relayer, andrfqdecoderbuilds. - ethergo/.goreleaser.yml: Refined build configuration for
signer-examplebinary, ensuring debugging information is included. - services/omnirpc/.goreleaser.yml: Ensured binaries are built with debugging information by removing
-s -wflags. - agents/.goreleaser.yml: Removed
-s -wflags fromldflagsto include versioning information for theagentsbinary.
6 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
There was a problem hiding this comment.
PR Summary
(updates since last review)
The pull request integrates Rookout for enhanced debugging and updates GoReleaser configurations across multiple components.
- contrib/screener-api/go.mod: Added
github.com/Rookout/GoSDKand updated several dependencies. - core/go.mod: Included
github.com/BurntSushi/tomland updated existing dependencies. - core/metrics/README.md: Updated to include Rookout setup instructions.
- core/metrics/rookout.go: New file for Rookout initialization, lacks robust error handling.
- core/metrics/internal/const.go: Minor punctuation correction in comments.
7 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings
core/metrics/README.md
Outdated
| @@ -23,6 +23,10 @@ Pass in the `JAEGER_ENDPOINT` enviornment variable | |||
|
|
|||
| Pass in the `PYROSCOPE_ENDPOINT` environment variable | |||
|
|
|||
| ## Rookout | |||
|
|
|||
| Pass in `ROOKOUT_TOKEN`. Note: this will not work if ldflags -s and -w are used, as these diable the symbol table. Additionally the gcflag `all=-dwarflocationlists=true` must be enabled. | |||
There was a problem hiding this comment.
syntax: Typo in 'diable'. Correct to 'disable'.
core/metrics/rookout.go
Outdated
| ) | ||
|
|
||
| // rookout is sideloaded. | ||
| // TODO: consider moving this to metrics. |
There was a problem hiding this comment.
style: Consider adding more robust error handling for rookout.Start to ensure proper initialization.
core/metrics/rookout.go
Outdated
| Token: os.Getenv(internal.RookoutToken), | ||
| Debug: core.HasEnv(internal.RookoutDebug), | ||
| GitCommit: synconfig.DefaultCommit, | ||
| // note: will chagne on fork |
There was a problem hiding this comment.
syntax: Typo in comment: 'chagne' should be 'change'.
| GitOrigin: "https://github.com/synapsecns/sanguine", | ||
| }) | ||
| if err != nil { | ||
| logger.Warn(err) |
There was a problem hiding this comment.
style: Consider logging more details about the error to aid in debugging.
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (1)
core/metrics/README.md (1)
28-28: Add a comma after 'Additionally'.There is a grammatical issue in the sentence. Add a comma after "Additionally" to improve readability.
- Additionally the gcflag `all=-dwarflocationlists=true` must be enabled. + Additionally, the gcflag `all=-dwarflocationlists=true` must be enabled.Tools
LanguageTool
[uncategorized] ~28-~28: A comma may be missing after the conjunctive/linking adverb ‘Additionally’.
Context: ...used, as these diable the symbol table. Additionally the gcflag `all=-dwarflocationlists=tru...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files ignored due to path filters (2)
contrib/screener-api/go.sumis excluded by!**/*.sumcore/go.sumis excluded by!**/*.sum
Files selected for processing (5)
- contrib/screener-api/go.mod (5 hunks)
- core/go.mod (6 hunks)
- core/metrics/README.md (1 hunks)
- core/metrics/internal/const.go (1 hunks)
- core/metrics/rookout.go (1 hunks)
Files skipped from review as they are similar to previous changes (3)
- core/go.mod
- core/metrics/internal/const.go
- core/metrics/rookout.go
Additional context used
LanguageTool
core/metrics/README.md
[uncategorized] ~28-~28: A comma may be missing after the conjunctive/linking adverb ‘Additionally’.
Context: ...used, as these diable the symbol table. Additionally the gcflag `all=-dwarflocationlists=tru...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
Additional comments not posted (1)
contrib/screener-api/go.mod (1)
42-42: Verify the necessity and compatibility of new dependencies.The newly added dependencies enhance various functionalities like logging, configuration handling, and time management. Ensure they are necessary for the module's functionality and do not conflict with existing dependencies.
Also applies to: 48-48, 70-70, 97-97, 105-105, 168-168, 191-191, 197-197
There was a problem hiding this comment.
PR Summary
(updates since last review)
The pull request integrates Rookout for enhanced debugging and updates GoReleaser configurations across multiple components.
- core/metrics/README.md: Updated to include new environment variable instructions for Rookout, such as
ROOKOUT_TOKENandgcflagsettings. - core/metrics/internal/const.go: Added new environment variables for Rookout integration, enhancing debugging capabilities.
- core/metrics/rookout.go: Introduced Rookout initialization with environment variables for debugging and Git information. Minimal error handling could lead to silent failures.
3 file(s) reviewed, 4 comment(s)
Edit PR Review Bot Settings
core/metrics/README.md
Outdated
| @@ -23,6 +23,11 @@ Pass in the `JAEGER_ENDPOINT` enviornment variable | |||
|
|
|||
| Pass in the `PYROSCOPE_ENDPOINT` environment variable | |||
|
|
|||
| ## Rookout | |||
|
|
|||
| Pass in `ROOKOUT_TOKEN`. Note: this will not work if ldflags -s and -w are used, as these diable the symbol table. Additionally the gcflag `all=-dwarflocationlists=true` must be enabled. You can override the git repo by setting an ldflag on `github.com/synapsecns/sanguine/core/metrics.DefaultGitRepo` to your repo or setting the enviornment variable `GIT_REPO`. | |||
There was a problem hiding this comment.
syntax: Typo in 'diable'. Correct to 'disable'.
core/metrics/README.md
Outdated
| @@ -23,6 +23,11 @@ Pass in the `JAEGER_ENDPOINT` enviornment variable | |||
|
|
|||
| Pass in the `PYROSCOPE_ENDPOINT` environment variable | |||
|
|
|||
| ## Rookout | |||
|
|
|||
| Pass in `ROOKOUT_TOKEN`. Note: this will not work if ldflags -s and -w are used, as these diable the symbol table. Additionally the gcflag `all=-dwarflocationlists=true` must be enabled. You can override the git repo by setting an ldflag on `github.com/synapsecns/sanguine/core/metrics.DefaultGitRepo` to your repo or setting the enviornment variable `GIT_REPO`. | |||
There was a problem hiding this comment.
syntax: Typo in 'enviornment'. Correct to 'environment'.
| ## Rookout | ||
|
|
||
| Pass in `ROOKOUT_TOKEN`. Note: this will not work if ldflags -s and -w are used, as these diable the symbol table. Additionally the gcflag `all=-dwarflocationlists=true` must be enabled. You can override the git repo by setting an ldflag on `github.com/synapsecns/sanguine/core/metrics.DefaultGitRepo` to your repo or setting the enviornment variable `GIT_REPO`. | ||
| Additionally, all [rookout enviornment](https://docs.rookout.com/docs/setup-guide/#configuration) variables are supported. |
There was a problem hiding this comment.
syntax: Typo in 'enviornment'. Correct to 'environment'.
| GitOrigin: core.GetEnv(internal.RookoutRemoteOrigin, core.GetEnv(internal.GitRepo, DefaultGitRepo)), | ||
| }) | ||
| if err != nil { | ||
| logger.Warn(err) |
There was a problem hiding this comment.
style: Consider adding more robust error handling for rookout.Start to ensure proper initialization.
There was a problem hiding this comment.
Actionable comments posted: 2
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (3)
- core/metrics/README.md (1 hunks)
- core/metrics/internal/const.go (1 hunks)
- core/metrics/rookout.go (1 hunks)
Files skipped from review as they are similar to previous changes (2)
- core/metrics/internal/const.go
- core/metrics/rookout.go
Additional context used
LanguageTool
core/metrics/README.md
[uncategorized] ~28-~28: A comma may be missing after the conjunctive/linking adverb ‘Additionally’.
Context: ...used, as these diable the symbol table. Additionally the gcflag `all=-dwarflocationlists=tru...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[misspelling] ~28-~28: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...ou can override the git repo by setting an ldflag on `github.com/synapsecns/sangui...(EN_A_VS_AN)
| ## Rookout | ||
|
|
||
| Pass in `ROOKOUT_TOKEN`. Note: this will not work if ldflags -s and -w are used, as these diable the symbol table. Additionally the gcflag `all=-dwarflocationlists=true` must be enabled. You can override the git repo by setting an ldflag on `github.com/synapsecns/sanguine/core/metrics.DefaultGitRepo` to your repo or setting the enviornment variable `GIT_REPO`. | ||
| Additionally, all [rookout enviornment](https://docs.rookout.com/docs/setup-guide/#configuration) variables are supported. |
There was a problem hiding this comment.
Fix the spelling of "environment."
The word "enviornment" is misspelled. It should be "environment."
- Additionally, all [rookout enviornment](https://docs.rookout.com/docs/setup-guide/#configuration) variables are supported.
+ Additionally, all [rookout environment](https://docs.rookout.com/docs/setup-guide/#configuration) variables are supported.Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Additionally, all [rookout enviornment](https://docs.rookout.com/docs/setup-guide/#configuration) variables are supported. | |
| Additionally, all [rookout environment](https://docs.rookout.com/docs/setup-guide/#configuration) variables are supported. |
core/metrics/README.md
Outdated
| @@ -23,6 +23,11 @@ | |||
|
|
|||
| Pass in the `PYROSCOPE_ENDPOINT` environment variable | |||
|
|
|||
| ## Rookout | |||
|
|
|||
| Pass in `ROOKOUT_TOKEN`. Note: this will not work if ldflags -s and -w are used, as these diable the symbol table. Additionally the gcflag `all=-dwarflocationlists=true` must be enabled. You can override the git repo by setting an ldflag on `github.com/synapsecns/sanguine/core/metrics.DefaultGitRepo` to your repo or setting the enviornment variable `GIT_REPO`. | |||
There was a problem hiding this comment.
Correct the typo in "diable" and improve clarity.
There's a typo in "diable," which should be "disable." Additionally, a comma is needed after "Additionally" for clarity.
- Pass in `ROOKOUT_TOKEN`. Note: this will not work if ldflags -s and -w are used, as these diable the symbol table. Additionally the gcflag `all=-dwarflocationlists=true` must be enabled. You can override the git repo by setting an ldflag on `github.com/synapsecns/sanguine/core/metrics.DefaultGitRepo` to your repo or setting the enviornment variable `GIT_REPO`.
+ Pass in `ROOKOUT_TOKEN`. Note: this will not work if ldflags -s and -w are used, as these disable the symbol table. Additionally, the gcflag `all=-dwarflocationlists=true` must be enabled. You can override the git repo by setting an ldflag on `github.com/synapsecns/sanguine/core/metrics.DefaultGitRepo` to your repo or setting the environment variable `GIT_REPO`.Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Pass in `ROOKOUT_TOKEN`. Note: this will not work if ldflags -s and -w are used, as these diable the symbol table. Additionally the gcflag `all=-dwarflocationlists=true` must be enabled. You can override the git repo by setting an ldflag on `github.com/synapsecns/sanguine/core/metrics.DefaultGitRepo` to your repo or setting the enviornment variable `GIT_REPO`. | |
| Pass in `ROOKOUT_TOKEN`. Note: this will not work if ldflags -s and -w are used, as these disable the symbol table. Additionally, the gcflag `all=-dwarflocationlists=true` must be enabled. You can override the git repo by setting an ldflag on `github.com/synapsecns/sanguine/core/metrics.DefaultGitRepo` to your repo or setting the environment variable `GIT_REPO`. |
Tools
LanguageTool
[uncategorized] ~28-~28: A comma may be missing after the conjunctive/linking adverb ‘Additionally’.
Context: ...used, as these diable the symbol table. Additionally the gcflag `all=-dwarflocationlists=tru...(SENT_START_CONJUNCTIVE_LINKING_ADVERB_COMMA)
[misspelling] ~28-~28: Use “a” instead of ‘an’ if the following word doesn’t start with a vowel sound, e.g. ‘a sentence’, ‘a university’.
Context: ...ou can override the git repo by setting an ldflag on `github.com/synapsecns/sangui...(EN_A_VS_AN)
There was a problem hiding this comment.
PR Summary
(updates since last review)
The pull request integrates Rookout for enhanced debugging and updates GoReleaser configurations across multiple components.
- core/metrics/README.md: Corrected typos and added detailed instructions for Rookout setup.
- core/metrics/rookout.go: Introduced Rookout initialization with environment variables for debugging.
- core/metrics/internal/const.go: Added new environment variables for Rookout integration.
- .github/workflows/goreleaser-actions.yml: Updated GoReleaser configurations to include versioning and improved debugging capabilities.
2 file(s) reviewed, 5 comment(s)
Edit PR Review Bot Settings
| ## Rookout | ||
|
|
||
| Pass in `ROOKOUT_TOKEN`. Note: this will not work if ldflags -s and -w are used, as these disable the symbol table. Additionally the gcflag `all=-dwarflocationlists=true` must be enabled. You can override the git repo by setting an ldflag on `github.com/synapsecns/sanguine/core/metrics.DefaultGitRepo` to your repo or setting the enviornment variable `GIT_REPO`. | ||
| Additionally, all [rookout enviornment](https://docs.rookout.com/docs/setup-guide/#configuration) variables are supported. |
There was a problem hiding this comment.
syntax: Typo in 'enviornment'. Correct to 'environment'.
core/metrics/README.md
Outdated
| @@ -48,7 +53,7 @@ The current logger is currently depended on by a large amount of modules: | |||
|
|
|||
| ### Limitations | |||
|
|
|||
| Currently, no enviornment variables are supported for the logger. This is a known limitation and will be fixed in a future release. Things like controlling the log level, sugarring, format, etc are [not currently supported](https://pkg.go.dev/go.uber.org/zap#NewProductionConfig). These will be added as the module beocmes more stable. | |||
| Currently, no environment variables are supported for the logger. This is a known limitation and will be fixed in a future release. Things like controlling the log level, sugarring, format, etc are [not currently supported](https://pkg.go.dev/go.uber.org/zap#NewProductionConfig). These will be added as the module beocmes more stable. | |||
There was a problem hiding this comment.
syntax: Typo in 'sugarring'. Correct to 'sugaring'.
core/metrics/README.md
Outdated
| @@ -48,7 +53,7 @@ The current logger is currently depended on by a large amount of modules: | |||
|
|
|||
| ### Limitations | |||
|
|
|||
| Currently, no enviornment variables are supported for the logger. This is a known limitation and will be fixed in a future release. Things like controlling the log level, sugarring, format, etc are [not currently supported](https://pkg.go.dev/go.uber.org/zap#NewProductionConfig). These will be added as the module beocmes more stable. | |||
| Currently, no environment variables are supported for the logger. This is a known limitation and will be fixed in a future release. Things like controlling the log level, sugarring, format, etc are [not currently supported](https://pkg.go.dev/go.uber.org/zap#NewProductionConfig). These will be added as the module beocmes more stable. | |||
|
|
|||
| Note: because both [ipfs go-log]("https://github.com/ipfs/go-log") and [otelzap logger](https://pkg.go.dev/github.com/uptrace/opentelemetry-go-extra/otelzap) depend on zap globals, in orderr to enable globals you can set `ENABLE_EXPERIMENTAL_ZAP_GLOBALS` to `true` in your environment. This will enable the zap globals, and you can use the `handler.Logger` to log to the global logger. This is not recommended, and will be removed in a future release. | |||
There was a problem hiding this comment.
syntax: Typo in 'in orderr'. Correct to 'in order'.
| GitOrigin: core.GetEnv(internal.RookoutRemoteOrigin, core.GetEnv(internal.GitRepo, DefaultGitRepo)), | ||
| }) | ||
| if err != nil { | ||
| logger.Warn(err) |
There was a problem hiding this comment.
style: Consider adding more robust error handling for rookout.Start to ensure proper initialization.
| GitOrigin: core.GetEnv(internal.RookoutRemoteOrigin, core.GetEnv(internal.GitRepo, DefaultGitRepo)), | ||
| }) | ||
| if err != nil { | ||
| logger.Warn(err) |
There was a problem hiding this comment.
style: Consider logging more details about the error to aid in debugging.
trajan0x
force-pushed
the
feat/stage-rookout
branch
from
13cd21a to
add2caf
Compare
There was a problem hiding this comment.
PR Summary
(updates since last review)
No major changes found since the last review.
2 file(s) reviewed, 7 comment(s)
Edit PR Review Bot Settings
| @@ -23,6 +23,11 @@ Pass in the `JAEGER_ENDPOINT` enviornment variable | |||
|
|
|||
| Pass in the `PYROSCOPE_ENDPOINT` environment variable | |||
|
|
|||
There was a problem hiding this comment.
syntax: Typo in 'enviornment'. Correct to 'environment'.
| ## Rookout | ||
|
|
||
| Pass in `ROOKOUT_TOKEN`. Note: this will not work if ldflags -s and -w are used, as these disable the symbol table. Additionally the gcflag `all=-dwarflocationlists=true` must be enabled. You can override the git repo by setting an ldflag on `github.com/synapsecns/sanguine/core/metrics.DefaultGitRepo` to your repo or setting the enviornment variable `GIT_REPO`. | ||
| Additionally, all [rookout enviornment](https://docs.rookout.com/docs/setup-guide/#configuration) variables are supported. |
There was a problem hiding this comment.
syntax: Typo in 'enviornment'. Correct to 'environment'.
| ## Rookout | ||
|
|
||
| Pass in `ROOKOUT_TOKEN`. Note: this will not work if ldflags -s and -w are used, as these disable the symbol table. Additionally the gcflag `all=-dwarflocationlists=true` must be enabled. You can override the git repo by setting an ldflag on `github.com/synapsecns/sanguine/core/metrics.DefaultGitRepo` to your repo or setting the enviornment variable `GIT_REPO`. | ||
| Additionally, all [rookout enviornment](https://docs.rookout.com/docs/setup-guide/#configuration) variables are supported. |
There was a problem hiding this comment.
syntax: Typo in 'enviornment'. Correct to 'environment'.
core/metrics/README.md
Outdated
| @@ -48,7 +53,7 @@ The current logger is currently depended on by a large amount of modules: | |||
|
|
|||
| ### Limitations | |||
|
|
|||
| Currently, no enviornment variables are supported for the logger. This is a known limitation and will be fixed in a future release. Things like controlling the log level, sugarring, format, etc are [not currently supported](https://pkg.go.dev/go.uber.org/zap#NewProductionConfig). These will be added as the module beocmes more stable. | |||
| Currently, no environment variables are supported for the logger. This is a known limitation and will be fixed in a future release. Things like controlling the log level, sugarring, format, etc are [not currently supported](https://pkg.go.dev/go.uber.org/zap#NewProductionConfig). These will be added as the module beocmes more stable. | |||
There was a problem hiding this comment.
syntax: Typo in 'sugarring'. Correct to 'sugaring'.
core/metrics/README.md
Outdated
| @@ -48,7 +53,7 @@ The current logger is currently depended on by a large amount of modules: | |||
|
|
|||
| ### Limitations | |||
|
|
|||
| Currently, no enviornment variables are supported for the logger. This is a known limitation and will be fixed in a future release. Things like controlling the log level, sugarring, format, etc are [not currently supported](https://pkg.go.dev/go.uber.org/zap#NewProductionConfig). These will be added as the module beocmes more stable. | |||
| Currently, no environment variables are supported for the logger. This is a known limitation and will be fixed in a future release. Things like controlling the log level, sugarring, format, etc are [not currently supported](https://pkg.go.dev/go.uber.org/zap#NewProductionConfig). These will be added as the module beocmes more stable. | |||
|
|
|||
| Note: because both [ipfs go-log]("https://github.com/ipfs/go-log") and [otelzap logger](https://pkg.go.dev/github.com/uptrace/opentelemetry-go-extra/otelzap) depend on zap globals, in orderr to enable globals you can set `ENABLE_EXPERIMENTAL_ZAP_GLOBALS` to `true` in your environment. This will enable the zap globals, and you can use the `handler.Logger` to log to the global logger. This is not recommended, and will be removed in a future release. | |||
There was a problem hiding this comment.
syntax: Typo in 'in orderr'. Correct to 'in order'.
| GitOrigin: core.GetEnv(internal.RookoutRemoteOrigin, core.GetEnv(internal.GitRepo, DefaultGitRepo)), | ||
| }) | ||
| if err != nil { | ||
| logger.Warn(err) |
There was a problem hiding this comment.
style: Consider adding more robust error handling for rookout.Start to ensure proper initialization.
| GitOrigin: core.GetEnv(internal.RookoutRemoteOrigin, core.GetEnv(internal.GitRepo, DefaultGitRepo)), | ||
| }) | ||
| if err != nil { | ||
| logger.Warn(err) |
There was a problem hiding this comment.
style: Consider logging more details about the error to aid in debugging.
There was a problem hiding this comment.
PR Summary
(updates since last review)
The pull request introduces test builds for Rookout, enhancing debugging capabilities and improving documentation.
- Updated
core/metrics/README.mdto correct typographical errors and add instructions for configuring Rookout. - Enhanced build configurations in multiple
.goreleaser.ymlfiles to include versioning information for better traceability. - Added
gcflagsfor various builds to improve debugging. - Improved formatting in Dockerfiles for better readability.
- Updated several
go.modfiles with new dependencies to enhance functionality across multiple services.
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
Description
test builds for rookout
Summary by CodeRabbit
New Features
gcflagsfor various builds.Bug Fixes
Documentation
Chores
go.modfiles with new dependencies to enhance functionality across multiple services.