From b616d63c556045be2470f8771b6ddc3703950b61 Mon Sep 17 00:00:00 2001 From: jonny rhea Date: Wed, 14 Jan 2026 13:45:43 -0600 Subject: [PATCH] incorporate internel/telemetry changes --- eth/catalyst/api.go | 88 ++++++++++++++------------------ eth/catalyst/api_test.go | 9 +--- eth/catalyst/simulated_beacon.go | 7 +-- eth/catalyst/witness.go | 45 +++++++++++----- 4 files changed, 74 insertions(+), 75 deletions(-) diff --git a/eth/catalyst/api.go b/eth/catalyst/api.go index c7c73e1caf..f36e9a40c0 100644 --- a/eth/catalyst/api.go +++ b/eth/catalyst/api.go @@ -36,6 +36,7 @@ import ( "github.com/ethereum/go-ethereum/core/types" "github.com/ethereum/go-ethereum/eth" "github.com/ethereum/go-ethereum/eth/ethconfig" + "github.com/ethereum/go-ethereum/internal/telemetry" "github.com/ethereum/go-ethereum/internal/version" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/miner" @@ -44,12 +45,6 @@ import ( "github.com/ethereum/go-ethereum/params/forks" "github.com/ethereum/go-ethereum/rlp" "github.com/ethereum/go-ethereum/rpc" - - "go.opentelemetry.io/otel" - "go.opentelemetry.io/otel/attribute" - "go.opentelemetry.io/otel/codes" - sdktrace "go.opentelemetry.io/otel/sdk/trace" - "go.opentelemetry.io/otel/trace" ) // Register adds the engine API to the full node. @@ -631,42 +626,15 @@ 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 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( - attribute.Int64("block.number", int64(params.Number)), - attribute.String("block.hash", params.BlockHash.Hex()), - attribute.Int("tx.count", len(params.Transactions)), - ) - spanEnd := func(err *error) { - ro, _ := span.(sdktrace.ReadOnlySpan) - if *err != nil { - // Error occurred, record it and set status on span and parent - span.RecordError(*err) - span.SetStatus(codes.Error, (*err).Error()) - parentSpan.SetStatus(codes.Error, (*err).Error()) - } else if ro.Status().Code == codes.Error { - // Span's child had an error, propagate it to parent - // Note: Span's status was already set in the child - parentSpan.SetStatus(codes.Error, ro.Status().Description) - } else if ro.Status().Code == codes.Unset { - // No error and no status set, mark as success - 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) (result engine.PayloadStatusV1, err error) { - ctx, spanEnd := startNewPayloadSpan(ctx, "engine.newPayloadV1", params) - defer spanEnd(&err) + attrs := []telemetry.Attribute{ + telemetry.Int64Attribute("block.number", int64(params.Number)), + telemetry.StringAttribute("block.hash", params.BlockHash.Hex()), + telemetry.Int64Attribute("tx.count", int64(len(params.Transactions))), + } + ctx, _, spanEnd := telemetry.StartSpan(ctx, "engine.newPayloadV1", attrs...) + defer spanEnd(err) if params.Withdrawals != nil { return invalidStatus, paramsErr("withdrawals not supported in V1") } @@ -675,8 +643,13 @@ 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) (result engine.PayloadStatusV1, err error) { - ctx, spanEnd := startNewPayloadSpan(ctx, "engine.newPayloadV2", params) - defer spanEnd(&err) + attrs := []telemetry.Attribute{ + telemetry.Int64Attribute("block.number", int64(params.Number)), + telemetry.StringAttribute("block.hash", params.BlockHash.Hex()), + telemetry.Int64Attribute("tx.count", int64(len(params.Transactions))), + } + ctx, _, spanEnd := telemetry.StartSpan(ctx, "engine.newPayloadV2", attrs...) + defer spanEnd(err) var ( cancun = api.config().IsCancun(api.config().LondonBlock, params.Timestamp) shanghai = api.config().IsShanghai(api.config().LondonBlock, params.Timestamp) @@ -698,8 +671,13 @@ 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) (result engine.PayloadStatusV1, err error) { - ctx, spanEnd := startNewPayloadSpan(ctx, "engine.newPayloadV3", params) - defer spanEnd(&err) + attrs := []telemetry.Attribute{ + telemetry.Int64Attribute("block.number", int64(params.Number)), + telemetry.StringAttribute("block.hash", params.BlockHash.Hex()), + telemetry.Int64Attribute("tx.count", int64(len(params.Transactions))), + } + ctx, _, spanEnd := telemetry.StartSpan(ctx, "engine.newPayloadV3", attrs...) + defer spanEnd(err) switch { case params.Withdrawals == nil: return invalidStatus, paramsErr("nil withdrawals post-shanghai") @@ -719,8 +697,13 @@ 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) (result engine.PayloadStatusV1, err error) { - ctx, spanEnd := startNewPayloadSpan(ctx, "engine.newPayloadV4", params) - defer spanEnd(&err) + attrs := []telemetry.Attribute{ + telemetry.Int64Attribute("block.number", int64(params.Number)), + telemetry.StringAttribute("block.hash", params.BlockHash.Hex()), + telemetry.Int64Attribute("tx.count", int64(len(params.Transactions))), + } + ctx, _, spanEnd := telemetry.StartSpan(ctx, "engine.newPayloadV4", attrs...) + defer spanEnd(err) switch { case params.Withdrawals == nil: return invalidStatus, paramsErr("nil withdrawals post-shanghai") @@ -763,9 +746,14 @@ func (api *ConsensusAPI) newPayload(ctx context.Context, params engine.Executabl log.Trace("Engine API request received", "method", "NewPayload", "number", params.Number, "hash", params.BlockHash) - _, spanEnd := startNewPayloadSpan(ctx, "engine.newPayload.ExecutableDataToBlock", params) + attrs := []telemetry.Attribute{ + telemetry.Int64Attribute("block.number", int64(params.Number)), + telemetry.StringAttribute("block.hash", params.BlockHash.Hex()), + telemetry.Int64Attribute("tx.count", int64(len(params.Transactions))), + } + _, _, spanEnd := telemetry.StartSpan(ctx, "engine.newPayload.ExecutableDataToBlock", attrs...) block, err := engine.ExecutableDataToBlock(params, versionedHashes, beaconRoot, requests) - spanEnd(&err) + spanEnd(err) if err != nil { bgu := "nil" @@ -839,11 +827,11 @@ func (api *ConsensusAPI) newPayload(ctx context.Context, params engine.Executabl return engine.PayloadStatusV1{Status: engine.ACCEPTED}, nil } log.Trace("Inserting block without sethead", "hash", block.Hash(), "number", block.Number()) - _, spanEnd = startNewPayloadSpan(ctx, "engine.newPayload.InsertBlockWithoutSetHead", params) + _, _, spanEnd = telemetry.StartSpan(ctx, "engine.newPayload.InsertBlockWithoutSetHead", attrs...) start := time.Now() proofs, err := api.eth.BlockChain().InsertBlockWithoutSetHead(block, witness) processingTime := time.Since(start) - spanEnd(&err) + spanEnd(err) if err != nil { log.Warn("NewPayload: inserting block failed", "error", err) diff --git a/eth/catalyst/api_test.go b/eth/catalyst/api_test.go index f8f8af9cb3..7eb26065dc 100644 --- a/eth/catalyst/api_test.go +++ b/eth/catalyst/api_test.go @@ -502,12 +502,7 @@ 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 root spans are created - // in NewPayloadV* entrypoints. This test calls newPayload() directly. - 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) + execResp, err := api.newPayload(context.Background(), *envelope.ExecutionPayload, []common.Hash{}, h, envelope.Requests, false) if err != nil { t.Fatalf("can't execute payload: %v", err) } @@ -1710,7 +1705,7 @@ func TestWitnessCreationAndConsumption(t *testing.T) { envelope.ExecutionPayload.StateRoot = wantStateRoot envelope.ExecutionPayload.ReceiptsRoot = wantReceiptRoot - res2, err := api.NewPayloadWithWitnessV3(*envelope.ExecutionPayload, []common.Hash{}, &common.Hash{42}) + res2, err := api.NewPayloadWithWitnessV3(context.Background(), *envelope.ExecutionPayload, []common.Hash{}, &common.Hash{42}) if err != nil { t.Fatalf("error executing stateless payload witness: %v", err) } diff --git a/eth/catalyst/simulated_beacon.go b/eth/catalyst/simulated_beacon.go index b02ab5a1db..6ae2bb2fdc 100644 --- a/eth/catalyst/simulated_beacon.go +++ b/eth/catalyst/simulated_beacon.go @@ -256,13 +256,8 @@ func (c *SimulatedBeacon) sealBlock(withdrawals []*types.Withdrawal, timestamp u requests = envelope.Requests } - // NOTE: This span is for the simulated beacon harness only. Normal tracing - // of engine_newPayload* is performed at the Engine API entrypoints. - 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) + _, err = c.engineAPI.newPayload(context.Background(), *payload, blobHashes, beaconRoot, requests, false) if err != nil { return err } diff --git a/eth/catalyst/witness.go b/eth/catalyst/witness.go index ef8e0af117..d2f25e6b2d 100644 --- a/eth/catalyst/witness.go +++ b/eth/catalyst/witness.go @@ -28,6 +28,7 @@ import ( "github.com/ethereum/go-ethereum/core" "github.com/ethereum/go-ethereum/core/stateless" "github.com/ethereum/go-ethereum/core/vm" + "github.com/ethereum/go-ethereum/internal/telemetry" "github.com/ethereum/go-ethereum/log" "github.com/ethereum/go-ethereum/params/forks" "github.com/ethereum/go-ethereum/rlp" @@ -87,9 +88,14 @@ 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) (result engine.PayloadStatusV1, err error) { - ctx, spanEnd := startNewPayloadSpan(context.Background(), "engine.newPayloadWithWitnessV1", params) - defer spanEnd(&err) +func (api *ConsensusAPI) NewPayloadWithWitnessV1(ctx context.Context, params engine.ExecutableData) (result engine.PayloadStatusV1, err error) { + attrs := []telemetry.Attribute{ + telemetry.Int64Attribute("block.number", int64(params.Number)), + telemetry.StringAttribute("block.hash", params.BlockHash.Hex()), + telemetry.Int64Attribute("tx.count", int64(len(params.Transactions))), + } + ctx, _, spanEnd := telemetry.StartSpan(ctx, "engine.newPayloadWithWitnessV1", attrs...) + defer spanEnd(err) if params.Withdrawals != nil { return engine.PayloadStatusV1{Status: engine.INVALID}, engine.InvalidParams.With(errors.New("withdrawals not supported in V1")) } @@ -98,9 +104,14 @@ func (api *ConsensusAPI) NewPayloadWithWitnessV1(params engine.ExecutableData) ( // 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) (result engine.PayloadStatusV1, err error) { - ctx, spanEnd := startNewPayloadSpan(context.Background(), "engine.newPayloadWithWitnessV2", params) - defer spanEnd(&err) +func (api *ConsensusAPI) NewPayloadWithWitnessV2(ctx context.Context, params engine.ExecutableData) (result engine.PayloadStatusV1, err error) { + attrs := []telemetry.Attribute{ + telemetry.Int64Attribute("block.number", int64(params.Number)), + telemetry.StringAttribute("block.hash", params.BlockHash.Hex()), + telemetry.Int64Attribute("tx.count", int64(len(params.Transactions))), + } + ctx, _, spanEnd := telemetry.StartSpan(ctx, "engine.newPayloadWithWitnessV2", attrs...) + defer spanEnd(err) var ( cancun = api.config().IsCancun(api.config().LondonBlock, params.Timestamp) shanghai = api.config().IsShanghai(api.config().LondonBlock, params.Timestamp) @@ -122,9 +133,14 @@ func (api *ConsensusAPI) NewPayloadWithWitnessV2(params engine.ExecutableData) ( // 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) (result engine.PayloadStatusV1, err error) { - ctx, spanEnd := startNewPayloadSpan(context.Background(), "engine.newPayloadWithWitnessV3", params) - defer spanEnd(&err) +func (api *ConsensusAPI) NewPayloadWithWitnessV3(ctx context.Context, params engine.ExecutableData, versionedHashes []common.Hash, beaconRoot *common.Hash) (result engine.PayloadStatusV1, err error) { + attrs := []telemetry.Attribute{ + telemetry.Int64Attribute("block.number", int64(params.Number)), + telemetry.StringAttribute("block.hash", params.BlockHash.Hex()), + telemetry.Int64Attribute("tx.count", int64(len(params.Transactions))), + } + ctx, _, spanEnd := telemetry.StartSpan(ctx, "engine.newPayloadWithWitnessV3", attrs...) + defer spanEnd(err) switch { case params.Withdrawals == nil: return invalidStatus, paramsErr("nil withdrawals post-shanghai") @@ -144,9 +160,14 @@ func (api *ConsensusAPI) NewPayloadWithWitnessV3(params engine.ExecutableData, v // 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) (result engine.PayloadStatusV1, err error) { - ctx, spanEnd := startNewPayloadSpan(context.Background(), "engine.newPayloadWithWitnessV4", params) - defer spanEnd(&err) +func (api *ConsensusAPI) NewPayloadWithWitnessV4(ctx context.Context, params engine.ExecutableData, versionedHashes []common.Hash, beaconRoot *common.Hash, executionRequests []hexutil.Bytes) (result engine.PayloadStatusV1, err error) { + attrs := []telemetry.Attribute{ + telemetry.Int64Attribute("block.number", int64(params.Number)), + telemetry.StringAttribute("block.hash", params.BlockHash.Hex()), + telemetry.Int64Attribute("tx.count", int64(len(params.Transactions))), + } + ctx, _, spanEnd := telemetry.StartSpan(ctx, "engine.newPayloadWithWitnessV4", attrs...) + defer spanEnd(err) switch { case params.Withdrawals == nil: return invalidStatus, paramsErr("nil withdrawals post-shanghai")