[rfq] add withdraw tracing #2929
Conversation
[goreleaser]
|
Caution Review failedThe pull request is closed. WalkthroughThe recent changes enhance the codebase by introducing new features and improving existing functionalities. Key updates include the addition of a Changes
Sequence Diagram(s)sequenceDiagram
participant Client
participant Server
participant Handler
participant Metrics
Client->>Server: Send API Request
Server->>Handler: Handle Request
Handler->>Metrics: Start Metrics Trace
Metrics-->>Handler: Trace Context
Handler->>Server: Process Request
Server->>Client: Return Response
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 (
|
![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 introduces RFQ tracing capabilities, enhancing observability and debugging for the Withdraw function.
- Tracing in Withdraw Function:
services/rfq/relayer/relapi/handler.gonow includes tracing spans for theWithdrawfunction. - Handler Initialization:
services/rfq/relayer/relapi/server.gomodifiesNewHandlerto accept an additionalhandlerparameter for tracing. - Dependency Update:
contrib/screener-api/go.modmovesgo.opentelemetry.io/otel/metricto direct dependencies. - New Field Addition:
contrib/promexporter/internal/gql/explorer/models.gen.goaddsLineafield toDateResultByChainstruct for tracking.
4 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #2929 +/- ##
===================================================
+ Coverage 25.33491% 25.35241% +0.01749%
===================================================
Files 790 790
Lines 56732 56752 +20
Branches 80 80
===================================================
+ Hits 14373 14388 +15
- Misses 40873 40876 +3
- Partials 1486 1488 +2
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: 0
Outside diff range, codebase verification and nitpick comments (1)
services/rfq/relayer/relapi/server.go (1)
115-115: Mismatch inNewHandlerfunction signature.The
NewHandlerfunction inservices/rfq/relayer/relapi/handler.godoes not include thehandlerargument, while the call inservices/rfq/relayer/relapi/server.godoes. This indicates a potential mismatch in the function signature. Please update theNewHandlerfunction to include thehandlerargument and ensure all calls to this function pass the correct arguments.
services/rfq/relayer/relapi/handler.go: UpdateNewHandlerfunction signature.- Verify all calls to
NewHandlerto ensure they pass the correct arguments.Analysis chain
Verify the integration of the new handler argument.
The
NewHandlerfunction now requires an additionalhandlerargument. Ensure that this change is reflected correctly throughout the codebase and that the handler is properly initialized and used.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of the new handler argument in the codebase. # Test: Search for the `NewHandler` function calls. Expect: All occurrences reflect the new signature. rg --type go -A 5 $'NewHandler'Length of output: 2032
Review details
Configuration used: .coderabbit.yaml
Review profile: CHILL
Files selected for processing (4)
- contrib/promexporter/internal/gql/explorer/models.gen.go (1 hunks)
- contrib/screener-api/go.mod (2 hunks)
- services/rfq/relayer/relapi/handler.go (6 hunks)
- services/rfq/relayer/relapi/server.go (1 hunks)
Additional comments not posted (5)
contrib/screener-api/go.mod (1)
30-30: Verify the dependency change.The
go.opentelemetry.io/otel/metricdependency is now a direct requirement. Ensure that this change is correctly reflected in the codebase and that the dependency is used appropriately.Verification successful
Verified the dependency change.
The
go.opentelemetry.io/otel/metricdependency is now a direct requirement and is used appropriately across multiple files in the codebase.
services/scribe/service/scribe.goservices/scribe/service/indexer/indexer.goservices/scribe/service/chain.goservices/explorer/api/server.goservices/rfq/relayer/service/otel.goservices/rfq/relayer/quoter/quoter.goservices/rfq/relayer/inventory/manager.goservices/omnirpc/chainmanager/manager.goservices/rfq/api/rest/server.goservices/rfq/guard/service/otel.goethergo/submitter/metrics.goethergo/listener/otel.gocore/metrics/null.gocore/metrics/meter_test.gocore/metrics/meter.gocore/metrics/metrics.gocore/metrics/instrumentation/otelginmetrics/otelrecorder.gocore/metrics/base.gocontrib/screener-api/screener/screener.gocontrib/promexporter/exporters/otel.gocontrib/opbot/botmd/middleware.goScripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the usage of the `go.opentelemetry.io/otel/metric` dependency in the codebase. # Test: Search for the usage of the `metric` package. Expect: The package is used directly in the codebase. rg --type go $'go.opentelemetry.io/otel/metric'Length of output: 1705
services/rfq/relayer/relapi/handler.go (3)
26-26: Verify the integration of the new field in theHandlerstruct.The
Handlerstruct now includes ametrics.Handlerfield. Ensure that this field is correctly initialized and used within the struct.Verification successful
The
metricsfield is correctly integrated and used within theHandlerstruct.
- The
metricsfield is initialized in theNewHandlerfunction.- The
metricsfield is used within theHandlerstruct methods.Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of the `metrics.Handler` field in the `Handler` struct. # Test: Search for the usage of the `metrics` field in the `Handler` struct. Expect: The field is used appropriately within the struct. rg --type go -A 5 $'metrics metrics.Handler' services/rfq/relayer/relapi/handler.goLength of output: 84
Script:
#!/bin/bash # Description: Verify the integration of the `metrics.Handler` field in the `Handler` struct. # Test: Search for any usage of the `metrics` field in the `Handler` struct. Expect: The field is used appropriately within the struct. rg --type go 'metrics' services/rfq/relayer/relapi/handler.goLength of output: 427
34-36: Verify the integration of the new parameter in theNewHandlerfunction.The
NewHandlerfunction now accepts ametricsHandlerparameter. Ensure that this parameter is correctly passed and used within the function.Verification successful
Verify the usage of the
metricsfield within theHandlerstruct.The
metricsHelperparameter is assigned to themetricsfield of theHandlerstruct. Ensure that this field is used appropriately within theHandlerstruct.
Verified: The
metricsHelperparameter is correctly integrated and used within theHandlerstruct.The
metricsHelperparameter is assigned to themetricsfield and is used appropriately for tracing operations within theHandlerstruct.
ctx, span := h.metrics.Tracer().Start(c, "withdraw")metrics.EndSpanWithErr(span, err)Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the integration of the `metricsHandler` parameter in the `NewHandler` function. # Test: Search for the usage of the `metricsHandler` parameter in the `NewHandler` function. Expect: The parameter is used appropriately within the function. rg --type go -A 5 $'metricsHelper metrics.Handler' services/rfq/relayer/relapi/handler.goLength of output: 414
Script:
#!/bin/bash # Description: Verify the usage of the `metrics` field within the `Handler` struct. # Test: Search for the usage of the `metrics` field within the `Handler` struct. Expect: The field is used appropriately within the struct. rg --type go 'metrics' services/rfq/relayer/relapi/handler.goLength of output: 427
184-188: Verify the integration of tracing functionality in theWithdrawmethod.The
Withdrawmethod now includes tracing functionality using themetricsinstance. Ensure that the tracing is correctly integrated and does not introduce any issues.contrib/promexporter/internal/gql/explorer/models.gen.go (1)
106-106: LGTM! The addition of theLineafield is consistent with the existing structure.The new field
Lineais correctly added as a pointer to afloat64and includes the appropriate JSON tag.
[goreleaser]
Deploying sanguine-fe with
|
| Latest commit: |
46bd17d
|
| Status: | ⚡️ Build in progress... |
There was a problem hiding this comment.
PR Summary
(updates since last review)
The pull request introduces RFQ tracing capabilities, enhancing observability and debugging for the Withdraw function.
- Tracing in Withdraw Function:
services/rfq/relayer/relapi/handler.gonow includes tracing spans for theWithdrawfunction. - Handler Initialization:
services/rfq/relayer/relapi/server.gomodifiesNewHandlerto accept an additionalhandlerparameter for tracing. - Dependency Update:
contrib/screener-api/go.modmovesgo.opentelemetry.io/otel/metricto direct dependencies. - New Field Addition:
contrib/promexporter/internal/gql/explorer/models.gen.goaddsLineafield toDateResultByChainstruct for tracking. - Goroutine Variable Capture Fix:
contrib/promexporter/exporters/util.gocaptures the loop variabletaskwithin the goroutine to avoid potential issues with variable scoping.
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/promexporter/exporters/util.go (1 hunks)
Additional context used
GitHub Check: codecov/patch
contrib/promexporter/exporters/util.go
[warning] 94-94: contrib/promexporter/exporters/util.go#L94
Added line #L94 was not covered by tests
Additional comments not posted (1)
contrib/promexporter/exporters/util.go (1)
94-94: LGTM! Ensure test coverage for the new line.The change to capture the
taskvariable in a closure is correct and necessary to prevent concurrency issues. However, the new line is not covered by tests.Do you want me to generate the unit testing code or open a GitHub issue to track this task?
Tools
GitHub Check: codecov/patch
[warning] 94-94: contrib/promexporter/exporters/util.go#L94
Added line #L94 was not covered by tests
There was a problem hiding this comment.
PR Summary
(updates since last review)
The pull request introduces a retry mechanism for withdrawal transaction hash retrieval and fixes an error reference in the RFQ relayer service.
- Retry Mechanism: Added a retry mechanism with a 1-minute max duration for withdrawal transaction hash retrieval in
services/rfq/relayer/cmd/commands.go. - Error Reference Fix: Corrected the reference to
errClientin error messages inservices/rfq/relayer/cmd/commands.go.
These changes aim to improve the reliability of the withdrawal process. Ensure thorough testing of the retry mechanism to handle edge cases effectively.
1 file(s) reviewed, no comment(s)
Edit PR Review Bot Settings
Description
adds rfq tracing
Summary by CodeRabbit
New Features
Improvements
Bug Fixes