diff --git a/eth/catalyst/api.go b/eth/catalyst/api.go index f001a62a7a..553c96e9ac 100644 --- a/eth/catalyst/api.go +++ b/eth/catalyst/api.go @@ -630,8 +630,11 @@ func (api *ConsensusAPI) getBlobs(hashes []common.Hash, v2 bool) ([]*engine.Blob // Helper for NewPayload* methods. var invalidStatus = engine.PayloadStatusV1{Status: engine.INVALID} -// startNewPayloadSpan starts a tracing span for a new payload. -func startNewPayloadSpan(ctx context.Context, name string, params engine.ExecutableData) (context.Context, trace.Span) { +// startNewPayloadSpan starts a tracing span for an RPC call and returns a function to +// end the span. The function will record errors and set span status based on +// the error value. +func startNewPayloadSpan(ctx context.Context, name string, params engine.ExecutableData) (context.Context, func(*error)) { + parentSpan := trace.SpanFromContext(ctx) tracer := otel.Tracer("") ctx, span := tracer.Start(ctx, name) span.SetAttributes( @@ -639,13 +642,23 @@ func startNewPayloadSpan(ctx context.Context, name string, params engine.Executa attribute.String("block.hash", params.BlockHash.Hex()), attribute.Int("tx.count", len(params.Transactions)), ) - return ctx, span + spanEnd := func(err *error) { + if *err != nil { + span.RecordError(*err) + span.SetStatus(codes.Error, (*err).Error()) + parentSpan.SetStatus(codes.Error, (*err).Error()) + } else { + span.SetStatus(codes.Ok, "") + } + span.End() + } + return ctx, spanEnd } // NewPayloadV1 creates an Eth1 block, inserts it in the chain, and returns the status of the chain. -func (api *ConsensusAPI) NewPayloadV1(ctx context.Context, params engine.ExecutableData) (engine.PayloadStatusV1, error) { - ctx, span := startNewPayloadSpan(ctx, "engine.newPayloadV1", params) - defer span.End() +func (api *ConsensusAPI) NewPayloadV1(ctx context.Context, params engine.ExecutableData) (result engine.PayloadStatusV1, err error) { + ctx, spanEnd := startNewPayloadSpan(ctx, "engine.newPayloadV1", params) + defer spanEnd(&err) if params.Withdrawals != nil { return invalidStatus, paramsErr("withdrawals not supported in V1") } @@ -653,9 +666,9 @@ func (api *ConsensusAPI) NewPayloadV1(ctx context.Context, params engine.Executa } // NewPayloadV2 creates an Eth1 block, inserts it in the chain, and returns the status of the chain. -func (api *ConsensusAPI) NewPayloadV2(ctx context.Context, params engine.ExecutableData) (engine.PayloadStatusV1, error) { - ctx, span := startNewPayloadSpan(ctx, "engine.newPayloadV2", params) - defer span.End() +func (api *ConsensusAPI) NewPayloadV2(ctx context.Context, params engine.ExecutableData) (result engine.PayloadStatusV1, err error) { + ctx, spanEnd := startNewPayloadSpan(ctx, "engine.newPayloadV2", params) + defer spanEnd(&err) var ( cancun = api.config().IsCancun(api.config().LondonBlock, params.Timestamp) shanghai = api.config().IsShanghai(api.config().LondonBlock, params.Timestamp) @@ -676,9 +689,9 @@ func (api *ConsensusAPI) NewPayloadV2(ctx context.Context, params engine.Executa } // NewPayloadV3 creates an Eth1 block, inserts it in the chain, and returns the status of the chain. -func (api *ConsensusAPI) NewPayloadV3(ctx context.Context, params engine.ExecutableData, versionedHashes []common.Hash, beaconRoot *common.Hash) (engine.PayloadStatusV1, error) { - ctx, span := startNewPayloadSpan(ctx, "engine.newPayloadV3", params) - defer span.End() +func (api *ConsensusAPI) NewPayloadV3(ctx context.Context, params engine.ExecutableData, versionedHashes []common.Hash, beaconRoot *common.Hash) (result engine.PayloadStatusV1, err error) { + ctx, spanEnd := startNewPayloadSpan(ctx, "engine.newPayloadV3", params) + defer spanEnd(&err) switch { case params.Withdrawals == nil: return invalidStatus, paramsErr("nil withdrawals post-shanghai") @@ -697,9 +710,9 @@ func (api *ConsensusAPI) NewPayloadV3(ctx context.Context, params engine.Executa } // NewPayloadV4 creates an Eth1 block, inserts it in the chain, and returns the status of the chain. -func (api *ConsensusAPI) NewPayloadV4(ctx context.Context, params engine.ExecutableData, versionedHashes []common.Hash, beaconRoot *common.Hash, executionRequests []hexutil.Bytes) (engine.PayloadStatusV1, error) { - ctx, span := startNewPayloadSpan(ctx, "engine.newPayloadV4", params) - defer span.End() +func (api *ConsensusAPI) NewPayloadV4(ctx context.Context, params engine.ExecutableData, versionedHashes []common.Hash, beaconRoot *common.Hash, executionRequests []hexutil.Bytes) (result engine.PayloadStatusV1, err error) { + ctx, spanEnd := startNewPayloadSpan(ctx, "engine.newPayloadV4", params) + defer spanEnd(&err) switch { case params.Withdrawals == nil: return invalidStatus, paramsErr("nil withdrawals post-shanghai") @@ -717,7 +730,7 @@ func (api *ConsensusAPI) NewPayloadV4(ctx context.Context, params engine.Executa return invalidStatus, unsupportedForkErr("newPayloadV4 must only be called for prague/osaka payloads") } requests := convertRequests(executionRequests) - if err := validateRequests(requests); err != nil { + if err = validateRequests(requests); err != nil { return engine.PayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(err) } return api.newPayload(ctx, params, versionedHashes, beaconRoot, requests, false) diff --git a/eth/catalyst/api_test.go b/eth/catalyst/api_test.go index f9d1b8e876..f8f8af9cb3 100644 --- a/eth/catalyst/api_test.go +++ b/eth/catalyst/api_test.go @@ -503,11 +503,11 @@ func setupBlocks(t *testing.T, ethservice *eth.Ethereum, n int, parent *types.He envelope := getNewEnvelope(t, api, parent, w, h) - // NOTE: This span is for the test harness only. Engine oot spans are created + // NOTE: This span is for the test harness only. Engine root spans are created // in NewPayloadV* entrypoints. This test calls newPayload() directly. - ctx, span := startNewPayloadSpan(context.Background(), "engine.api_test.setupBlocks", *envelope.ExecutionPayload) - defer span.End() + ctx, spanEnd := startNewPayloadSpan(context.Background(), "engine.api_test.setupBlocks", *envelope.ExecutionPayload) execResp, err := api.newPayload(ctx, *envelope.ExecutionPayload, []common.Hash{}, h, envelope.Requests, false) + spanEnd(&err) if err != nil { t.Fatalf("can't execute payload: %v", err) } diff --git a/eth/catalyst/simulated_beacon.go b/eth/catalyst/simulated_beacon.go index 2ccdfd4ba5..b02ab5a1db 100644 --- a/eth/catalyst/simulated_beacon.go +++ b/eth/catalyst/simulated_beacon.go @@ -258,9 +258,11 @@ func (c *SimulatedBeacon) sealBlock(withdrawals []*types.Withdrawal, timestamp u // NOTE: This span is for the simulated beacon harness only. Normal tracing // of engine_newPayload* is performed at the Engine API entrypoints. - ctx, span := startNewPayloadSpan(context.Background(), "engine.simulatedBeacon.sealBlock", *payload) - defer span.End() + ctx, spanEnd := startNewPayloadSpan(context.Background(), "engine.simulatedBeacon.sealBlock", *payload) + + // Mark the payload as canon _, err = c.engineAPI.newPayload(ctx, *payload, blobHashes, beaconRoot, requests, false) + spanEnd(&err) if err != nil { return err } diff --git a/eth/catalyst/witness.go b/eth/catalyst/witness.go index bc71df0863..ef8e0af117 100644 --- a/eth/catalyst/witness.go +++ b/eth/catalyst/witness.go @@ -87,18 +87,20 @@ func (api *ConsensusAPI) ForkchoiceUpdatedWithWitnessV3(update engine.Forkchoice // NewPayloadWithWitnessV1 is analogous to NewPayloadV1, only it also generates // and returns a stateless witness after running the payload. -func (api *ConsensusAPI) NewPayloadWithWitnessV1(params engine.ExecutableData) (engine.PayloadStatusV1, error) { +func (api *ConsensusAPI) NewPayloadWithWitnessV1(params engine.ExecutableData) (result engine.PayloadStatusV1, err error) { + ctx, spanEnd := startNewPayloadSpan(context.Background(), "engine.newPayloadWithWitnessV1", params) + defer spanEnd(&err) if params.Withdrawals != nil { return engine.PayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(errors.New("withdrawals not supported in V1")) } - ctx, span := startNewPayloadSpan(context.Background(), "engine.newPayloadWithWitnessV1", params) - defer span.End() return api.newPayload(ctx, params, nil, nil, nil, true) } // NewPayloadWithWitnessV2 is analogous to NewPayloadV2, only it also generates // and returns a stateless witness after running the payload. -func (api *ConsensusAPI) NewPayloadWithWitnessV2(params engine.ExecutableData) (engine.PayloadStatusV1, error) { +func (api *ConsensusAPI) NewPayloadWithWitnessV2(params engine.ExecutableData) (result engine.PayloadStatusV1, err error) { + ctx, spanEnd := startNewPayloadSpan(context.Background(), "engine.newPayloadWithWitnessV2", params) + defer spanEnd(&err) var ( cancun = api.config().IsCancun(api.config().LondonBlock, params.Timestamp) shanghai = api.config().IsShanghai(api.config().LondonBlock, params.Timestamp) @@ -115,14 +117,14 @@ func (api *ConsensusAPI) NewPayloadWithWitnessV2(params engine.ExecutableData) ( case params.BlobGasUsed != nil: return invalidStatus, paramsErr("non-nil blobGasUsed pre-cancun") } - ctx, span := startNewPayloadSpan(context.Background(), "engine.newPayloadWithWitnessV2", params) - defer span.End() return api.newPayload(ctx, params, nil, nil, nil, true) } // NewPayloadWithWitnessV3 is analogous to NewPayloadV3, only it also generates // and returns a stateless witness after running the payload. -func (api *ConsensusAPI) NewPayloadWithWitnessV3(params engine.ExecutableData, versionedHashes []common.Hash, beaconRoot *common.Hash) (engine.PayloadStatusV1, error) { +func (api *ConsensusAPI) NewPayloadWithWitnessV3(params engine.ExecutableData, versionedHashes []common.Hash, beaconRoot *common.Hash) (result engine.PayloadStatusV1, err error) { + ctx, spanEnd := startNewPayloadSpan(context.Background(), "engine.newPayloadWithWitnessV3", params) + defer spanEnd(&err) switch { case params.Withdrawals == nil: return invalidStatus, paramsErr("nil withdrawals post-shanghai") @@ -137,14 +139,14 @@ func (api *ConsensusAPI) NewPayloadWithWitnessV3(params engine.ExecutableData, v case !api.checkFork(params.Timestamp, forks.Cancun): return invalidStatus, unsupportedForkErr("newPayloadV3 must only be called for cancun payloads") } - ctx, span := startNewPayloadSpan(context.Background(), "engine.newPayloadWithWitnessV3", params) - defer span.End() return api.newPayload(ctx, params, versionedHashes, beaconRoot, nil, true) } // NewPayloadWithWitnessV4 is analogous to NewPayloadV4, only it also generates // and returns a stateless witness after running the payload. -func (api *ConsensusAPI) NewPayloadWithWitnessV4(params engine.ExecutableData, versionedHashes []common.Hash, beaconRoot *common.Hash, executionRequests []hexutil.Bytes) (engine.PayloadStatusV1, error) { +func (api *ConsensusAPI) NewPayloadWithWitnessV4(params engine.ExecutableData, versionedHashes []common.Hash, beaconRoot *common.Hash, executionRequests []hexutil.Bytes) (result engine.PayloadStatusV1, err error) { + ctx, spanEnd := startNewPayloadSpan(context.Background(), "engine.newPayloadWithWitnessV4", params) + defer spanEnd(&err) switch { case params.Withdrawals == nil: return invalidStatus, paramsErr("nil withdrawals post-shanghai") @@ -162,11 +164,9 @@ func (api *ConsensusAPI) NewPayloadWithWitnessV4(params engine.ExecutableData, v return invalidStatus, unsupportedForkErr("newPayloadV4 must only be called for prague/osaka payloads") } requests := convertRequests(executionRequests) - if err := validateRequests(requests); err != nil { + if err = validateRequests(requests); err != nil { return engine.PayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(err) } - ctx, span := startNewPayloadSpan(context.Background(), "engine.newPayloadWithWitnessV4", params) - defer span.End() return api.newPayload(ctx, params, versionedHashes, beaconRoot, requests, true) }