From a3c30e5c03761e165225665d03977d77c5b667c3 Mon Sep 17 00:00:00 2001 From: ramtinms Date: Wed, 12 Jun 2024 11:14:55 -0700 Subject: [PATCH 1/2] add open tracer to EVM handler --- fvm/environment/env.go | 9 +-------- fvm/environment/tracer.go | 16 ++++++++++++++++ fvm/evm/backends/wrappedEnv.go | 10 ++++++++++ fvm/evm/handler/handler.go | 20 ++++++++++++++++++++ fvm/evm/testutils/backend.go | 22 ++++++++++++++++++++++ fvm/evm/types/backend.go | 1 + module/trace/constants.go | 8 ++++++++ 7 files changed, 78 insertions(+), 8 deletions(-) create mode 100644 fvm/environment/tracer.go diff --git a/fvm/environment/env.go b/fvm/environment/env.go index 031eb460dc4..02b864fbce3 100644 --- a/fvm/environment/env.go +++ b/fvm/environment/env.go @@ -4,12 +4,9 @@ import ( "github.com/onflow/cadence" "github.com/onflow/cadence/runtime" "github.com/rs/zerolog" - otelTrace "go.opentelemetry.io/otel/trace" reusableRuntime "github.com/onflow/flow-go/fvm/runtime" - "github.com/onflow/flow-go/fvm/tracing" "github.com/onflow/flow-go/model/flow" - "github.com/onflow/flow-go/module/trace" ) // Environment implements the accounts business logic and exposes cadence @@ -17,11 +14,7 @@ import ( type Environment interface { runtime.Interface - // Tracer - StartChildSpan( - name trace.SpanName, - options ...otelTrace.SpanStartOption, - ) tracing.TracerSpan + Tracer Meter diff --git a/fvm/environment/tracer.go b/fvm/environment/tracer.go new file mode 100644 index 00000000000..f276286475e --- /dev/null +++ b/fvm/environment/tracer.go @@ -0,0 +1,16 @@ +package environment + +import ( + otelTrace "go.opentelemetry.io/otel/trace" + + "github.com/onflow/flow-go/fvm/tracing" + "github.com/onflow/flow-go/module/trace" +) + +// Tracer captures traces +type Tracer interface { + StartChildSpan( + name trace.SpanName, + options ...otelTrace.SpanStartOption, + ) tracing.TracerSpan +} diff --git a/fvm/evm/backends/wrappedEnv.go b/fvm/evm/backends/wrappedEnv.go index d22aabc191c..a996dff05da 100644 --- a/fvm/evm/backends/wrappedEnv.go +++ b/fvm/evm/backends/wrappedEnv.go @@ -5,12 +5,15 @@ import ( "github.com/onflow/cadence" "github.com/onflow/cadence/runtime" "github.com/onflow/cadence/runtime/common" + otelTrace "go.opentelemetry.io/otel/trace" "github.com/onflow/flow-go/fvm/environment" "github.com/onflow/flow-go/fvm/errors" "github.com/onflow/flow-go/fvm/evm/types" "github.com/onflow/flow-go/fvm/meter" + "github.com/onflow/flow-go/fvm/tracing" "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/module/trace" ) // WrappedEnvironment wraps an FVM environment @@ -144,6 +147,13 @@ func (we *WrappedEnvironment) GenerateUUID() (uint64, error) { return uuid, handleEnvironmentError(err) } +func (we *WrappedEnvironment) StartChildSpan( + name trace.SpanName, + options ...otelTrace.SpanStartOption, +) tracing.TracerSpan { + return we.env.StartChildSpan(name, options...) +} + func handleEnvironmentError(err error) error { if err == nil { return nil diff --git a/fvm/evm/handler/handler.go b/fvm/evm/handler/handler.go index 2a0a048ccfe..495de6c880c 100644 --- a/fvm/evm/handler/handler.go +++ b/fvm/evm/handler/handler.go @@ -6,6 +6,7 @@ import ( "github.com/onflow/cadence/runtime/common" gethCommon "github.com/onflow/go-ethereum/common" gethTypes "github.com/onflow/go-ethereum/core/types" + "go.opentelemetry.io/otel/attribute" "github.com/onflow/flow-go/fvm/environment" fvmErrors "github.com/onflow/flow-go/fvm/errors" @@ -13,6 +14,7 @@ import ( "github.com/onflow/flow-go/fvm/evm/handler/coa" "github.com/onflow/flow-go/fvm/evm/types" "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/module/trace" ) // ContractHandler is responsible for triggering calls to emulator, metering, @@ -65,6 +67,8 @@ func NewContractHandler( // DeployCOA deploys a cadence-owned-account and returns the address func (h *ContractHandler) DeployCOA(uuid uint64) types.Address { + defer h.backend.StartChildSpan(trace.FVMEVMDeployCOA).End() + res, err := h.deployCOA(uuid) panicOnErrorOrInvalidOrFailedState(res, err) return *res.DeployedContractAddress @@ -119,6 +123,8 @@ func (h *ContractHandler) RunOrPanic(rlpEncodedTx []byte, coinbase types.Address // Run tries to run an rlpencoded evm transaction and // collects the gas fees and pay it to the coinbase address provided. func (h *ContractHandler) Run(rlpEncodedTx []byte, coinbase types.Address) *types.ResultSummary { + defer h.backend.StartChildSpan(trace.FVMEVMRun).End() + res, err := h.run(rlpEncodedTx, coinbase) panicOnError(err) return res.ResultSummary() @@ -129,6 +135,10 @@ func (h *ContractHandler) Run(rlpEncodedTx []byte, coinbase types.Address) *type // All transactions provided in the batch are included in a single block, // except for invalid transactions func (h *ContractHandler) BatchRun(rlpEncodedTxs [][]byte, coinbase types.Address) []*types.ResultSummary { + span := h.backend.StartChildSpan(trace.FVMEVMRun) + span.SetAttributes(attribute.Int("tx_counts", len(rlpEncodedTxs))) + defer span.End() + res, err := h.batchRun(rlpEncodedTxs, coinbase) panicOnError(err) @@ -341,6 +351,8 @@ func (h *ContractHandler) DryRun( rlpEncodedTx []byte, from types.Address, ) *types.ResultSummary { + defer h.backend.StartChildSpan(trace.FVMEVMRun).End() + res, err := h.dryRun(rlpEncodedTx, from) panicOnError(err) return res.ResultSummary() @@ -667,6 +679,8 @@ func (a *Account) codeHash() ([]byte, error) { // Deposit deposits the token from the given vault into the flow evm main vault // and update the account balance with the new amount func (a *Account) Deposit(v *types.FLOWTokenVault) { + defer a.fch.backend.StartChildSpan(trace.FVMEVMDeposit).End() + res, err := a.deposit(v) panicOnErrorOrInvalidOrFailedState(res, err) } @@ -692,6 +706,8 @@ func (a *Account) deposit(v *types.FLOWTokenVault) (*types.Result, error) { // Withdraw deducts the balance from the account and // withdraw and return flow token from the Flex main vault. func (a *Account) Withdraw(b types.Balance) *types.FLOWTokenVault { + defer a.fch.backend.StartChildSpan(trace.FVMEVMWithdraw).End() + res, err := a.withdraw(b) panicOnErrorOrInvalidOrFailedState(res, err) @@ -745,6 +761,8 @@ func (a *Account) transfer(to types.Address, balance types.Balance) (*types.Resu // contained in the result summary as data and // the contract data is not controlled by the caller accounts func (a *Account) Deploy(code types.Code, gaslimit types.GasLimit, balance types.Balance) *types.ResultSummary { + defer a.fch.backend.StartChildSpan(trace.FVMEVMDeploy).End() + res, err := a.deploy(code, gaslimit, balance) panicOnError(err) return res.ResultSummary() @@ -771,6 +789,8 @@ func (a *Account) deploy(code types.Code, gaslimit types.GasLimit, balance types // given it doesn't goes beyond what Flow transaction allows. // the balance would be deducted from the OFA account and would be transferred to the target address func (a *Account) Call(to types.Address, data types.Data, gaslimit types.GasLimit, balance types.Balance) *types.ResultSummary { + defer a.fch.backend.StartChildSpan(trace.FVMEVMCall).End() + res, err := a.call(to, data, gaslimit, balance) panicOnError(err) return res.ResultSummary() diff --git a/fvm/evm/testutils/backend.go b/fvm/evm/testutils/backend.go index 472d38201f4..1b36bbe3e02 100644 --- a/fvm/evm/testutils/backend.go +++ b/fvm/evm/testutils/backend.go @@ -7,6 +7,7 @@ import ( "testing" "github.com/onflow/cadence/runtime/stdlib" + otelTrace "go.opentelemetry.io/otel/trace" "github.com/onflow/atree" "github.com/onflow/cadence" @@ -19,7 +20,9 @@ import ( "github.com/onflow/flow-go/fvm/environment" "github.com/onflow/flow-go/fvm/evm/types" "github.com/onflow/flow-go/fvm/meter" + "github.com/onflow/flow-go/fvm/tracing" "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/module/trace" ) var TestFlowEVMRootAddress = flow.BytesToAddress([]byte("FlowEVM")) @@ -40,6 +43,7 @@ func RunWithTestBackend(t testing.TB, f func(*TestBackend)) { TestBlockInfo: getSimpleBlockStore(), TestRandomGenerator: getSimpleRandomGenerator(), TestContractFunctionInvoker: &TestContractFunctionInvoker{}, + TestTracer: &TestTracer{}, } f(tb) } @@ -185,6 +189,7 @@ type TestBackend struct { *TestRandomGenerator *TestContractFunctionInvoker *testUUIDGenerator + *TestTracer } var _ types.Backend = &TestBackend{} @@ -497,3 +502,20 @@ func (t *testUUIDGenerator) GenerateUUID() (uint64, error) { } return t.generateUUID() } + +type TestTracer struct { + StartChildSpanFunc func(trace.SpanName, ...otelTrace.SpanStartOption) tracing.TracerSpan +} + +var _ environment.Tracer = &TestTracer{} + +func (t *TestTracer) StartChildSpan( + name trace.SpanName, + options ...otelTrace.SpanStartOption, +) tracing.TracerSpan { + // if not set we use noop tracer + if t.StartChildSpanFunc == nil { + return tracing.NewMockTracerSpan() + } + return t.StartChildSpanFunc(name, options...) +} diff --git a/fvm/evm/types/backend.go b/fvm/evm/types/backend.go index 2a281f94a6b..23fd2c5e377 100644 --- a/fvm/evm/types/backend.go +++ b/fvm/evm/types/backend.go @@ -15,4 +15,5 @@ type Backend interface { environment.RandomGenerator environment.ContractFunctionInvoker environment.UUIDGenerator + environment.Tracer } diff --git a/module/trace/constants.go b/module/trace/constants.go index 0b349bc360c..3f3e04a9d50 100644 --- a/module/trace/constants.go +++ b/module/trace/constants.go @@ -194,5 +194,13 @@ const ( FVMEnvRemoveAccountContractCode SpanName = "fvm.env.removeAccountContractCode" FVMEnvGetSigningAccounts SpanName = "fvm.env.getSigningAccounts" + FVMEVMDeployCOA SpanName = "fvm.evm.deployCOA" + FVMEVMRun SpanName = "fvm.evm.run" + FVMEVMBatchRun SpanName = "fvm.evm.batchRun" + FVMEVMDeposit SpanName = "fvm.evm.deposit" + FVMEVMWithdraw SpanName = "fvm.evm.withdraw" + FVMEVMDeploy SpanName = "fvm.evm.deploy" + FVMEVMCall SpanName = "fvm.evm.call" + FVMCadenceTrace SpanName = "fvm.cadence.trace" ) From 7defe630ab44970e599d877145ec213e7383f3b7 Mon Sep 17 00:00:00 2001 From: ramtinms Date: Thu, 13 Jun 2024 10:59:54 -0700 Subject: [PATCH 2/2] add tests --- fvm/evm/handler/handler.go | 4 +-- fvm/evm/handler/handler_test.go | 52 +++++++++++++++++++++++++++++++++ fvm/evm/testutils/backend.go | 16 ++++++++-- module/trace/constants.go | 1 + 4 files changed, 68 insertions(+), 5 deletions(-) diff --git a/fvm/evm/handler/handler.go b/fvm/evm/handler/handler.go index 495de6c880c..eb1dbd7822d 100644 --- a/fvm/evm/handler/handler.go +++ b/fvm/evm/handler/handler.go @@ -135,7 +135,7 @@ func (h *ContractHandler) Run(rlpEncodedTx []byte, coinbase types.Address) *type // All transactions provided in the batch are included in a single block, // except for invalid transactions func (h *ContractHandler) BatchRun(rlpEncodedTxs [][]byte, coinbase types.Address) []*types.ResultSummary { - span := h.backend.StartChildSpan(trace.FVMEVMRun) + span := h.backend.StartChildSpan(trace.FVMEVMBatchRun) span.SetAttributes(attribute.Int("tx_counts", len(rlpEncodedTxs))) defer span.End() @@ -351,7 +351,7 @@ func (h *ContractHandler) DryRun( rlpEncodedTx []byte, from types.Address, ) *types.ResultSummary { - defer h.backend.StartChildSpan(trace.FVMEVMRun).End() + defer h.backend.StartChildSpan(trace.FVMEVMDryRun).End() res, err := h.dryRun(rlpEncodedTx, from) panicOnError(err) diff --git a/fvm/evm/handler/handler_test.go b/fvm/evm/handler/handler_test.go index ca22cbb3b5c..58b9e9a7be5 100644 --- a/fvm/evm/handler/handler_test.go +++ b/fvm/evm/handler/handler_test.go @@ -33,6 +33,7 @@ import ( "github.com/onflow/flow-go/fvm/evm/types" "github.com/onflow/flow-go/fvm/systemcontracts" "github.com/onflow/flow-go/model/flow" + "github.com/onflow/flow-go/module/trace" ) // TODO add test for fatal errors @@ -1257,6 +1258,57 @@ func TestHandler_TransactionRun(t *testing.T) { }) }) }) + + t.Run("test - open tracing", func(t *testing.T) { + t.Parallel() + + testutils.RunWithTestBackend(t, func(backend *testutils.TestBackend) { + testutils.RunWithTestFlowEVMRootAddress(t, backend, func(rootAddr flow.Address) { + testutils.RunWithEOATestAccount(t, backend, rootAddr, func(eoa *testutils.EOATestAccount) { + + tx := gethTypes.NewTransaction( + uint64(1), + gethCommon.Address{1, 2}, + big.NewInt(13), + uint64(0), + big.NewInt(1000), + []byte{}, + ) + + rlpTx, err := tx.MarshalBinary() + require.NoError(t, err) + + handler := SetupHandler(t, backend, rootAddr) + + backend.ExpectedSpan(t, trace.FVMEVMDryRun) + handler.DryRun(rlpTx, types.EmptyAddress) + + backend.ExpectedSpan(t, trace.FVMEVMRun) + handler.Run(rlpTx, types.EmptyAddress) + + backend.ExpectedSpan(t, trace.FVMEVMBatchRun) + handler.BatchRun([][]byte{rlpTx}, types.EmptyAddress) + + backend.ExpectedSpan(t, trace.FVMEVMDeployCOA) + coa := handler.DeployCOA(1) + + acc := handler.AccountByAddress(coa, true) + + backend.ExpectedSpan(t, trace.FVMEVMCall) + acc.Call(types.EmptyAddress, nil, 1000, types.EmptyBalance) + + backend.ExpectedSpan(t, trace.FVMEVMDeposit) + acc.Deposit(types.NewFlowTokenVault(types.EmptyBalance)) + + backend.ExpectedSpan(t, trace.FVMEVMWithdraw) + acc.Withdraw(types.EmptyBalance) + + backend.ExpectedSpan(t, trace.FVMEVMDeploy) + acc.Deploy(nil, 1, types.EmptyBalance) + }) + }) + }) + }) } // returns true if error passes the checks diff --git a/fvm/evm/testutils/backend.go b/fvm/evm/testutils/backend.go index 1b36bbe3e02..6d5616a488e 100644 --- a/fvm/evm/testutils/backend.go +++ b/fvm/evm/testutils/backend.go @@ -509,13 +509,23 @@ type TestTracer struct { var _ environment.Tracer = &TestTracer{} -func (t *TestTracer) StartChildSpan( +func (tt *TestTracer) StartChildSpan( name trace.SpanName, options ...otelTrace.SpanStartOption, ) tracing.TracerSpan { // if not set we use noop tracer - if t.StartChildSpanFunc == nil { + if tt.StartChildSpanFunc == nil { + return tracing.NewMockTracerSpan() + } + return tt.StartChildSpanFunc(name, options...) +} + +func (tt *TestTracer) ExpectedSpan(t *testing.T, expected trace.SpanName) { + tt.StartChildSpanFunc = func( + sn trace.SpanName, + sso ...otelTrace.SpanStartOption, + ) tracing.TracerSpan { + require.Equal(t, expected, sn) return tracing.NewMockTracerSpan() } - return t.StartChildSpanFunc(name, options...) } diff --git a/module/trace/constants.go b/module/trace/constants.go index 3f3e04a9d50..069e5835251 100644 --- a/module/trace/constants.go +++ b/module/trace/constants.go @@ -196,6 +196,7 @@ const ( FVMEVMDeployCOA SpanName = "fvm.evm.deployCOA" FVMEVMRun SpanName = "fvm.evm.run" + FVMEVMDryRun SpanName = "fvm.evm.dryRun" FVMEVMBatchRun SpanName = "fvm.evm.batchRun" FVMEVMDeposit SpanName = "fvm.evm.deposit" FVMEVMWithdraw SpanName = "fvm.evm.withdraw"