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.

Sign up for GitHub

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

update dest tx hash before status/improve obversability #2888

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
12 commits
Select commit Hold shift + click to select a range
20bda71
[goreleaser] observability hotfix
trajan0x Jul 17, 2024
160a5b2
Feat: record err as attribute
dwasse Jul 17, 2024
dff84eb
[goreleaser]
dwasse Jul 17, 2024
690b167
[goreleaser]
trajan0x Jul 17, 2024
2358e16
order hotfix [goreleaser]
trajan0x Jul 17, 2024
72dbf2b
[goreleaser] error accumulator
trajan0x Jul 17, 2024
9f409ed
[goreleaser]
trajan0x Jul 17, 2024
4951dc7
[goreleaser]
trajan0x Jul 17, 2024
312e935
[goreleaser] again
trajan0x Jul 17, 2024
53067c3
test out namespace
trajan0x Jul 17, 2024
5504b32
[goreleaser]
trajan0x Jul 17, 2024
35baeea
fix
trajan0x Jul 17, 2024
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
6 changes: 5 additions & 1 deletion core/metrics/spanutils.go
View file
Open in desktop
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
9 changes: 9 additions & 0 deletions services/rfq/guard/service/handlers.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,16 @@
}

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

Check failure on line 45 in services/rfq/guard/service/handlers.go

View workflow job for this annotation

GitHub Actions / Lint (services/rfq) 

SA4009: argument ctx is overwritten before first use (staticcheck)
ctx, newSpan := g.metrics.Tracer().Start(parentCtx, "fetchBridgeTx", trace.WithAttributes(

Check failure on line 46 in services/rfq/guard/service/handlers.go

View workflow job for this annotation

GitHub Actions / Lint (services/rfq) 

SA4009(related information): assignment to ctx (staticcheck)
Copy link
Contributor

Choose a reason for hiding this comment

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

Avoid reassigning ctx within the call function.

Reassigning ctx within the call function can lead to potential issues. Instead, use a different variable name for the new context.

-		ctx, newSpan := g.metrics.Tracer().Start(parentCtx, "fetchBridgeTx", trace.WithAttributes(
+		newCtx, newSpan := g.metrics.Tracer().Start(parentCtx, "fetchBridgeTx", trace.WithAttributes(
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.

Suggested change
ctx, newSpan := g.metrics.Tracer().Start(parentCtx, "fetchBridgeTx", trace.WithAttributes(
newCtx, newSpan := g.metrics.Tracer().Start(parentCtx, "fetchBridgeTx", trace.WithAttributes(
Tools
GitHub Check: Lint (services/rfq)

[failure] 46-46:
SA4009(related information): assignment to ctx (staticcheck)

attribute.String("transaction_id", hexutil.Encode(req.TransactionId[:])),
attribute.String("bridge_address", req.Raw.Address.String()),
))

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

Check warning on line 53 in services/rfq/guard/service/handlers.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/guard/service/handlers.go#L46-L53 

Added lines #L46 - L53 were not covered by tests

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
21 changes: 19 additions & 2 deletions services/rfq/relayer/service/handlers.go
View file
Open in desktop
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,16 @@
}

var bridgeTx fastbridge.IFastBridgeBridgeTransaction
call := func(ctx context.Context) error {
call := func(parentCtx context.Context) error {
trajan0x marked this conversation as resolved.
Show resolved Hide resolved
ctx, newSpan := r.metrics.Tracer().Start(parentCtx, "fetchBridgeTx", trace.WithAttributes(
attribute.String("transaction_id", hexutil.Encode(req.TransactionId[:])),
attribute.String("bridge_address", req.Raw.Address.String()),
))

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

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

View check run for this annotation

Codecov / codecov/patch

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

Added lines #L74 - L82 were not covered by tests

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 @@ -421,6 +430,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 439 in services/rfq/relayer/service/handlers.go

View check run for this annotation

Codecov / codecov/patch

services/rfq/relayer/service/handlers.go#L433-L439 

Added lines #L433 - L439 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