Add debug_traceTransactionProfile endpoint#3267
Merged
Merged
Conversation
Adds a granular tracing layer on top of the existing debug_trace path so operators can see where time is spent during a historical trace. New RPC endpoint: - debug_traceTransactionProfile returns the normal trace result plus a profile payload that breaks the request into high-level phases (block lookup, historical-state replay, tx prep, execution, tracer result), and dumps per-module store access stats, iterator stats, and any low-level DB read events collected during the trace. Store tracer plumbing: - Introduce a StoreTracer on sdk.Context that records per-module accesses, iterator lifecycle events, and low-level ReadTraceEvents. - Thread the tracer through gaskv, cachekv, and tracekv so every store operation is timed; cachekv/tracekv forward the collector to their parents so the tracer reaches the state store underneath. - Wire storev2/state.Store to optionally swap its underlying StateStore for a traceable wrapper via SetReadTraceCollector, and add forwarding WithReadTraceCollector shims on the composite/cosmos/evm SS-layer stores so a collector injected from the top reaches the MVCC layer when it implements TraceableStateStore. Lifecycle: - TraceTransactionProfile (and TraceStateAccess) defer a cleanup that closes any trace-scoped resources (Pebble snapshots, reusable iterators) attached during the request. Reporting CLI: - Add seidb trace-profile-report which takes an RPC endpoint and a block range, calls debug_traceTransactionProfile for every tx, persists the raw responses, and emits a summary + interactive HTML report (phase breakdown, hot modules, hot low-level ops, per-tx table). The low-level Pebble read events populate when the MVCC layer implements TraceableStateStore (shipped in the companion perf PR); if that PR is not merged, the profile still shows phase timings, module access stats, and iterator stats, but the low-level MVCC breakdown will be empty. Made-with: Cursor
|
The latest Buf updates on your PR. Results from workflow Buf / buf (pull_request).
|
Kbhat1
changed the title
outputDir is a CLI flag on this offline seidb reporting tool, not a request-level input; suppress the file-inclusion warning with a nolint:gosec comment matching conventions elsewhere in sei-db/tools.
Kbhat1
changed the title
Audit found the closer plumbing on StoreTracer is dormant: nothing in the repo calls AddReadTraceCloser, so readClosers is always empty and both CloseReadTraceResources and the closeStoreTraceResources wrapper in evmrpc are no-ops. Remove all of it. When low-level (pebbledb) read tracing gets wired in via TraceableStateStore in a follow-up, the closer side can return with the add side in one change. Also add doc comments to the newly-introduced exported surface that the profile endpoint depends on: StoreTracer and its types (ModuleTrace/IteratorTrace/Access/OpType), record methods (Get/Set/Has/Delete/StartIterator/RecordIteratorValue/RecordIteratorNext), Dump/DerivePrestateToJson/RecordReadTrace, and the seidb-db types (ReadTraceEvent/ReadTraceCollector/TraceableStateStore/NewReadTraceEvent). Also annotate the per-tx sample caps with the reason they exist. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## main #3267 +/- ##
==========================================
- Coverage 59.19% 59.18% -0.02%
==========================================
Files 2091 2072 -19
Lines 171432 169472 -1960
==========================================
- Hits 101481 100295 -1186
+ Misses 61152 60420 -732
+ Partials 8799 8757 -42
Flags with carried forward coverage won't be shown. Click here to find out more.
🚀 New features to boost your workflow:
|
The CLI already emits raw_profiles.jsonl and summary.json; the third artifact (report.html) was a 335-line embedded template with picked- from-a-hat styling, no tests, and no durable callers. Anyone who wants a visual from summary.json can render it with any tool. Dropping the template shrinks the CLI file from 855 to 507 lines and removes the html/template dependency. - Remove writeTraceHTMLReport + its template block - Remove traceReportData wrapper type (unused after) - Remove the report.html write + fprintln in runTraceProfileReport - Tighten --output-dir help to name the two remaining artifacts No API or endpoint change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
Nothing on this branch emits ReadTraceEvents: no state store backend implements TraceableStateStore, so every WithReadTraceCollector forwarder hits a type-assertion that fails at the leaf, RecordReadTrace has zero callers, and LowLevelStats/LowLevelSamples on the response are always empty. The test already gates its assertions with `if foundAnyLowLevelStats` and passes vacuously. Same pattern as the earlier AddReadTraceCloser cut: ship the plumbing with the emitter, not ahead of it. A follow-up PR (on top of perf-mvcc once its MVCC layer grows a TraceableStateStore implementation) can restore the interfaces, the forwarders, the RecordReadTrace handler, the LowLevelStats wire fields, and the CLI aggregation in one change that's testable end-to-end. Removed: - ReadTraceEvent / ReadTraceCollector / TraceableStateStore / NewReadTraceEvent from sei-db/db_engine/types/types.go - WithReadTraceCollector on composite/cosmos/evm SS stores - SetReadTraceCollector passthroughs on cachekv/tracekv/storev2 - injectReadTraceCollector + its call sites in context.go KVStore / TransientStore - activeStore / readTraceCollector fields on storev2 state.Store - LowLevelReads field on ModuleTrace, LowLevelStats / LowLevelSamples fields on ModuleTraceDump, ReadTraceEventDump type, maxLowLevelReadSamples const, RecordReadTrace method and the low-level branch in StoreTracer.dumpLocked - LowLevelStats / LowLevelTotals aggregation in seidb trace-profile-report CLI - low-level assertions in evmrpc TestTraceTransactionProfile The KVStore-level tracer (Get/Has/Set/Delete + iterator tracking) that powers Stats and iterators on the profile response is untouched. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
The one-line comments I added on Get/Set/Has/Delete/OpType.String just restated the function names without adding information. The non-trivial docstrings (StartIterator's iteratorID contract, RecordIteratorValue's Truncated semantics, Dump's prestate-exclusion rule, DerivePrestateToJson's consumer, the cap constants) stay. No behavior change. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
yzang2019
reviewed
Apr 17, 2026
gs.parent.Has(key) does the same underlying lookup whether the key
exists or not. The tracer's whole point is to surface where time was
spent, so filtering out the miss case hides exactly the pattern we
most want to see in a profile ("tx spent X ms on 500 Has() calls
that all missed"). Drop the && res guard so Has is traced on every
call, matching Get/Set/Delete.
Spotted by @yzang2019 in review.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
yzang2019
approved these changes
Apr 17, 2026
Every Get/Set/Has/Delete/Iterator/Next call was unconditionally calling time.Now() to stage a duration for the tracer, even when no tracer is attached (the common case in production). Guard each time.Now() on gs.tracer != nil so the non-traced hot path is unaffected by the tracing machinery. Spotted by @yzang2019 in review. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
…guards
The 'var start time.Time; if tracer != nil { start = time.Now() }' dance
was repeated at every Get/Set/Has/Delete/iterator/Next call site. Fold
it into a tiny helper so each call site becomes 'start := traceStart(
tracer)'. Behavior is identical: time.Now() only fires when a tracer is
attached, same as before; every tracer call site still nil-checks
before invoking. Addresses @yzang2019's review comment consistently
across the 6 sites.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
jewei1997
approved these changes
Apr 21, 2026
The old assertion checked that iterated keys showed up in bankDump.Has, which worked because gasIterator.Key() previously logged every key as a tracer.Has call. This PR gives iterators their own ops (IteratorOpen / IteratorValue / IteratorNext) and drops the Has shortcut, so iterator keys now surface on bankDump.Iterators[0].Keys instead. Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Describe your changes and provide context
debug_traceTransactionProfile, a trace endpoint that returns the normal tx trace plus a granular timing/profile breakdownTesting performed to validate your change