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

Jump to bottom

trace [SLT-330] #3264

Merged
merged 6 commits into from
Oct 10, 2024
Merged

trace [SLT-330] #3264

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
b17e6eb
trace [slt-330]
trajan0x Oct 10, 2024
2f7f49b
Merge branch 'master' into fix/track-issue
trajan0x Oct 10, 2024
3ad23c2
fix marshal
trajan0x Oct 10, 2024
d325d3f
marshal to bin [goreleaser]
trajan0x Oct 10, 2024
24785c9
syntax
trajan0x Oct 10, 2024
0b2dc8e
another test
trajan0x Oct 10, 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
8 changes: 8 additions & 0 deletions ethergo/util/attributes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
gasPriceAttr = "tx.GasPrice"
gasFeeCapAttr = "tx.GasFeeCap"
gasTipCapAttr = "tx.GasTipCap"
txRawAttr = "tx.Raw"
)

// TxToAttributes converts a transaction to a slice of attribute.KeyValue.
Expand All @@ -33,6 +34,12 @@
} else {
from = call.From.Hex()
}

bin, err := transaction.MarshalBinary()
if err != nil {
bin = []byte(fmt.Sprintf("could not be marshaled: %v", err))
}

Check warning on line 41 in ethergo/util/attributes.go

View check run for this annotation

Codecov / codecov/patch

ethergo/util/attributes.go#L40-L41 

Added lines #L40 - L41 were not covered by tests

var attributes = []attribute.KeyValue{
attribute.String(hashAttr, transaction.Hash().Hex()),
attribute.String(fromAttr, from),
Expand All @@ -46,6 +53,7 @@
// nolint: gosec
attribute.Int64(gasLimitAttr, int64(transaction.Gas())),
attribute.String(chainIDAttr, BigPtrToString(transaction.ChainId())),
attribute.String(txRawAttr, common.Bytes2Hex(bin)),
}

if transaction.Type() == types.LegacyTxType && transaction.GasPrice() != nil {
Expand Down
4 changes: 4 additions & 0 deletions ethergo/util/attributes_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,6 +109,10 @@ func (u *UtilSuite) TestTxToAttributesDynamicTX() {

u.Require().Equal(mapAttr[util.GasFeeCapAttr].AsString(), mockTX.GasFeeCap().String())
u.Require().Equal(mapAttr[util.GasTipCapAttr].AsString(), mockTX.GasTipCap().String())
marshaled, err := mockTX.MarshalBinary()
u.Require().NoError(err)

u.Require().Equal(mapAttr[util.TxRawAttr].AsString(), common.Bytes2Hex(marshaled))
_, hasGasPrice := mapAttr[util.GasPriceAttr]
u.Require().False(hasGasPrice)
u.Require().NotNil(mapAttr[util.FromAttr])
Expand Down
2 changes: 2 additions & 0 deletions ethergo/util/export_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -72,4 +72,6 @@ const (
GasFeeCapAttr = gasFeeCapAttr
// GasTipCapAttr exports gasTipCapAttr for testing.
GasTipCapAttr = gasTipCapAttr
// TxRawAttr exports txRawAttr for testing.
TxRawAttr = txRawAttr
)
1 change: 1 addition & 0 deletions services/omnirpc/http/client.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ type Client interface {
}

// Request is a request builder.
// TODO: this needs to support tracing.
type Request interface {
// SetBody sets the request body
SetBody(body []byte) Request
Expand Down
9 changes: 9 additions & 0 deletions services/omnirpc/modules/receiptsbackup/receiptsbackup.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -137,11 +137,18 @@
}

func (r *receiptsProxyImpl) processRequest(ctx context.Context, rpcRequest rpc.Request, requestID []byte) (resp omniHTTP.Response, err error) {
ctx, span := r.handler.Tracer().Start(ctx, "proxyrequest")
defer func() {
metrics.EndSpanWithErr(span, err)
}()

Check warning on line 143 in services/omnirpc/modules/receiptsbackup/receiptsbackup.go

View check run for this annotation

Codecov / codecov/patch

services/omnirpc/modules/receiptsbackup/receiptsbackup.go#L140-L143 

Added lines #L140 - L143 were not covered by tests
Comment on lines +140 to +143
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Increase test coverage for the new tracing code in processRequest.

The tracing code added in lines 140-143 is not currently covered by tests, as indicated by the static analysis tools. Adding unit tests will help ensure that the tracing functionality works as expected and improve the reliability of the code.

Would you like assistance in writing unit tests for this code?

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 140-143: services/omnirpc/modules/receiptsbackup/receiptsbackup.go#L140-L143
Added lines #L140 - L143 were not covered by tests


mixins.TxSubmitMixin(ctx, r.handler, rpcRequest)

req := r.client.NewRequest()
body, err := json.Marshal(rpcRequest)

span.AddEvent("request marshaled", trace.WithAttributes(attribute.String("body", string(body))))

Check warning on line 151 in services/omnirpc/modules/receiptsbackup/receiptsbackup.go

View check run for this annotation

Codecov / codecov/patch

services/omnirpc/modules/receiptsbackup/receiptsbackup.go#L150-L151 

Added lines #L150 - L151 were not covered by tests
Comment on lines +150 to +151
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Improve test coverage for the added tracing event.

The new tracing event in lines 150-151 is not covered by tests. Adding tests can ensure that this functionality is working correctly and help detect any issues early.

Would you like help in creating tests for this code?

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 150-151: services/omnirpc/modules/receiptsbackup/receiptsbackup.go#L150-L151
Added lines #L150 - L151 were not covered by tests

//nolint: exhaustive
switch client.RPCMethod(rpcRequest.Method) {
case client.TransactionReceiptByHashMethod:
Expand Down Expand Up @@ -192,6 +199,8 @@
return nil, fmt.Errorf("could not get response from RPC %s: %w", r.proxyURL, err)
}

span.AddEvent("response returned", trace.WithAttributes(attribute.String("body", string(resp.Body()))))

Check warning on line 203 in services/omnirpc/modules/receiptsbackup/receiptsbackup.go

View check run for this annotation

Codecov / codecov/patch

services/omnirpc/modules/receiptsbackup/receiptsbackup.go#L202-L203 

Added lines #L202 - L203 were not covered by tests
Comment on lines +202 to +203
Copy link
Contributor

Choose a reason for hiding this comment

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

⚠️ Potential issue

Add tests for the new tracing event to improve test coverage.

The tracing event added in lines 202-203 is not covered by tests, according to the static analysis tools. Please consider adding unit tests to cover this new functionality and enhance code reliability.

Do you need assistance in writing tests for this code?

🧰 Tools
🪛 GitHub Check: codecov/patch

[warning] 202-203: services/omnirpc/modules/receiptsbackup/receiptsbackup.go#L202-L203
Added lines #L202 - L203 were not covered by tests

return resp, nil
}
}
Loading