Ensure that all spans return ok when there are no errors

This commit is contained in:
jonny rhea 2026-01-04 16:12:09 -06:00
parent 3ca77216c3
commit 1c173805c6
4 changed files with 49 additions and 34 deletions

View file

@ -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)

View file

@ -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)
}

View file

@ -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
}

View file

@ -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)
}