Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

update dest tx hash before status/improve obversability #2888

Merged
merged 12 commits into from
Jul 19, 2024
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion core/go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ require (
go.opentelemetry.io/otel/sdk v1.28.0
go.opentelemetry.io/otel/sdk/metric v1.28.0
go.opentelemetry.io/otel/trace v1.28.0
go.uber.org/multierr v1.11.0
go.uber.org/zap v1.27.0
golang.org/x/sync v0.7.0
gorm.io/driver/sqlite v1.5.6
Expand Down Expand Up @@ -163,7 +164,6 @@ require (
github.com/yusufpapurcu/wmi v1.2.3 // indirect
go.opentelemetry.io/otel/log v0.3.0 // indirect
go.opentelemetry.io/proto/otlp v1.3.1 // indirect
go.uber.org/multierr v1.11.0 // indirect
golang.org/x/arch v0.8.0 // indirect
golang.org/x/crypto v0.24.0 // indirect
golang.org/x/exp v0.0.0-20240613232115-7f521ea00fb8 // indirect
Expand Down
6 changes: 5 additions & 1 deletion core/metrics/spanutils.go
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
package metrics

import "go.opentelemetry.io/otel/trace"
import (
"go.opentelemetry.io/otel/attribute"
"go.opentelemetry.io/otel/trace"
)

// EndSpanWithErr ends a span and records an error if one is present.
func EndSpanWithErr(span trace.Span, err error) {
Expand All @@ -9,6 +12,7 @@
}

if err != nil {
span.SetAttributes(attribute.String("span_error", err.Error()))

Check warning on line 15 in core/metrics/spanutils.go

View check run for this annotation

Codecov / codecov/patch

core/metrics/spanutils.go#L15

Added line #L15 was not covered by tests
span.RecordError(err)
}

Expand Down
11 changes: 8 additions & 3 deletions core/retry/retry.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,7 @@ import (
"errors"
"fmt"
"github.com/jpillora/backoff"
errorUtil "github.com/pkg/errors"
"go.uber.org/multierr"
"time"
)

Expand Down Expand Up @@ -118,6 +118,8 @@ func WithBackoff(ctx context.Context, doFunc RetryableFunc, configurators ...Wit
timeout := time.Duration(0)
startTime := time.Now()

var errs []error

attempts := 0
for !config.exceedsMaxAttempts(attempts) && !config.exceedsMaxTime(startTime) {
select {
Expand All @@ -135,6 +137,7 @@ func WithBackoff(ctx context.Context, doFunc RetryableFunc, configurators ...Wit

err := doFunc(funcCtx)
if err != nil {
errs = append(errs, err)
timeout = b.Duration()
attempts++
cancel()
Expand All @@ -146,10 +149,12 @@ func WithBackoff(ctx context.Context, doFunc RetryableFunc, configurators ...Wit
}

if config.exceedsMaxAttempts(attempts) {
return errorUtil.Wrapf(ErrMaxAttempts, "after %d attempts", attempts)
// nolint: wrapcheck
return multierr.Append(ErrMaxAttempts, fmt.Errorf("after %d attempts (attempt errors: %w)", attempts, multierr.Combine(errs...)))
trajan0x marked this conversation as resolved.
Show resolved Hide resolved
}
if config.exceedsMaxTime(startTime) {
return errorUtil.Wrapf(ErrMaxTime, "after %s (max was %s)", time.Since(startTime).String(), config.maxAllAttemptsTime.String())
// nolint: wrapcheck
return multierr.Append(ErrMaxTime, fmt.Errorf("after %s (max was %s) %w", time.Since(startTime).String(), config.maxAllAttemptsTime.String(), multierr.Combine(errs...)))
trajan0x marked this conversation as resolved.
Show resolved Hide resolved
}

return ErrUnknown
Expand Down
21 changes: 15 additions & 6 deletions services/rfq/relayer/service/handlers.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@
"go.opentelemetry.io/otel/trace"
)

var maxRPCRetryTime = 15 * time.Second
var maxRPCRetryTime = 30 * time.Second

// handleBridgeRequestedLog handles the BridgeRequestedLog event.
// Step 1: Seen
Expand Down Expand Up @@ -71,7 +71,7 @@
}

var bridgeTx fastbridge.IFastBridgeBridgeTransaction
call := func(ctx context.Context) error {
call := func(parentCtx context.Context) error {

Check warning on line 74 in services/rfq/relayer/service/handlers.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/service/handlers.go#L74

Added line #L74 was not covered by tests

Check failure on line 74 in services/rfq/relayer/service/handlers.go

View workflow job for this annotation

GitHub Actions / Lint (services/rfq)

unused-parameter: parameter 'parentCtx' seems to be unused, consider removing or renaming it as _ (revive)
trajan0x marked this conversation as resolved.
Show resolved Hide resolved
bridgeTx, err = fastBridge.GetBridgeTransaction(&bind.CallOpts{Context: ctx}, req.Request)
if err != nil {
return fmt.Errorf("could not get bridge transaction: %w", err)
Expand Down Expand Up @@ -332,14 +332,15 @@
}

// TODO: this can still get re-orged
err = r.db.UpdateQuoteRequestStatus(ctx, req.TransactionId, reldb.RelayCompleted, nil)
if err != nil {
return fmt.Errorf("could not update request status: %w", err)
}
err = r.db.UpdateDestTxHash(ctx, req.TransactionId, req.Raw.TxHash)
if err != nil {
return fmt.Errorf("could not update dest tx hash: %w", err)
}

err = r.db.UpdateQuoteRequestStatus(ctx, req.TransactionId, reldb.RelayCompleted, nil)
if err != nil {
return fmt.Errorf("could not update request status: %w", err)
}

Check warning on line 343 in services/rfq/relayer/service/handlers.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/service/handlers.go#L340-L343

Added lines #L340 - L343 were not covered by tests
trajan0x marked this conversation as resolved.
Show resolved Hide resolved
return nil
}

Expand Down Expand Up @@ -421,6 +422,14 @@

var canClaim bool
claimCall := func(ctx context.Context) error {
ctx, newSpan := q.metrics.Tracer().Start(ctx, "checkClaimCheck", trace.WithAttributes(
attribute.String("transaction_id", hexutil.Encode(request.TransactionID[:])),
))

defer func() {
metrics.EndSpanWithErr(newSpan, err)
}()

Check warning on line 431 in services/rfq/relayer/service/handlers.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/service/handlers.go#L425-L431

Added lines #L425 - L431 were not covered by tests
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Enhanced tracing and error handling.

The function has been enhanced with tracing and error handling for metrics, improving observability and reliability.

The added lines are 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] 425-431: services/rfq/relayer/service/handlers.go#L425-L431
Added lines #L425 - L431 were not covered by tests


Enhanced tracing and error handling.

The claimCall function has been enhanced with tracing and error handling for metrics, improving observability and reliability.

The added lines are 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] 425-431: services/rfq/relayer/service/handlers.go#L425-L431
Added lines #L425 - L431 were not covered by tests


canClaim, err = q.Origin.Bridge.CanClaim(&bind.CallOpts{Context: ctx}, request.TransactionID, q.RelayerAddress)
if err != nil {
return fmt.Errorf("could not check if can claim: %w", err)
Expand Down
Loading