op-challenger: Use a wrapper to specify the vm type for metrics (ethe…

…reum-optimism#12020) * op-challenger: Use a wrapper to specify the vm type for metrics Decouples the vm type from the trace type to give the run-trace subcommand more flexibility, allowing it to report separate metrics for cannon and mt-cannon. Also adjust the run-trace loggers to consistently use `mt-cannon` as the type. * Use constant. Co-authored-by: Inphi <[email protected]> --------- Co-authored-by: Inphi <[email protected]>
celestiaorg · Sep 20, 2024 · 3c3a311 · 3c3a311
1 parent eaf4d3e
commit 3c3a311
Show file tree

Hide file tree

Showing 10 changed files with 65 additions and 23 deletions.
diff --git a/op-challenger/game/fault/trace/outputs/output_asterisc.go b/op-challenger/game/fault/trace/outputs/output_asterisc.go
@@ -41,7 +41,7 @@ func NewOutputAsteriscTraceAccessor(
 		if err != nil {
 			return nil, fmt.Errorf("failed to fetch asterisc local inputs: %w", err)
 		}
-		provider := asterisc.NewTraceProvider(logger, m, cfg, vmCfg, prestateProvider, asteriscPrestate, localInputs, subdir, depth)
+		provider := asterisc.NewTraceProvider(logger, m.VmMetrics(cfg.VmType.String()), cfg, vmCfg, prestateProvider, asteriscPrestate, localInputs, subdir, depth)
 		return provider, nil
 	}
 

diff --git a/op-challenger/game/fault/trace/outputs/output_cannon.go b/op-challenger/game/fault/trace/outputs/output_cannon.go
@@ -41,7 +41,7 @@ func NewOutputCannonTraceAccessor(
 		if err != nil {
 			return nil, fmt.Errorf("failed to fetch cannon local inputs: %w", err)
 		}
-		provider := cannon.NewTraceProvider(logger, m, cfg, serverExecutor, prestateProvider, cannonPrestate, localInputs, subdir, depth)
+		provider := cannon.NewTraceProvider(logger, m.VmMetrics(cfg.VmType.String()), cfg, serverExecutor, prestateProvider, cannonPrestate, localInputs, subdir, depth)
 		return provider, nil
 	}
 

diff --git a/op-challenger/game/fault/trace/vm/executor.go b/op-challenger/game/fault/trace/vm/executor.go
@@ -22,8 +22,8 @@ const (
 )
 
 type Metricer interface {
-	RecordVmExecutionTime(vmType string, t time.Duration)
-	RecordVmMemoryUsed(vmType string, memoryUsed uint64)
+	RecordExecutionTime(t time.Duration)
+	RecordMemoryUsed(memoryUsed uint64)
 }
 
 type Config struct {
@@ -133,12 +133,12 @@ func (e *Executor) DoGenerateProof(ctx context.Context, dir string, begin uint64
 	err = e.cmdExecutor(ctx, e.logger.New("proof", end), e.cfg.VmBin, args...)
 	execTime := time.Since(execStart)
 	memoryUsed := "unknown"
-	e.metrics.RecordVmExecutionTime(e.cfg.VmType.String(), execTime)
+	e.metrics.RecordExecutionTime(execTime)
 	if e.cfg.DebugInfo && err == nil {
 		if info, err := jsonutil.LoadJSON[debugInfo](filepath.Join(dataDir, debugFilename)); err != nil {
 			e.logger.Warn("Failed to load debug metrics", "err", err)
 		} else {
-			e.metrics.RecordVmMemoryUsed(e.cfg.VmType.String(), uint64(info.MemoryUsed))
+			e.metrics.RecordMemoryUsed(uint64(info.MemoryUsed))
 			memoryUsed = fmt.Sprintf("%d", uint64(info.MemoryUsed))
 		}
 	}

diff --git a/op-challenger/game/fault/trace/vm/executor_test.go b/op-challenger/game/fault/trace/vm/executor_test.go
@@ -9,7 +9,6 @@ import (
 	"time"
 
 	"github.com/ethereum-optimism/optimism/op-challenger/game/fault/trace/utils"
-	"github.com/ethereum-optimism/optimism/op-challenger/metrics"
 	"github.com/ethereum-optimism/optimism/op-service/testlog"
 	"github.com/ethereum/go-ethereum/common"
 	"github.com/ethereum/go-ethereum/log"
@@ -145,10 +144,12 @@ func TestGenerateProof(t *testing.T) {
 }
 
 type stubVmMetrics struct {
-	metrics.NoopMetricsImpl
 	executionTimeRecordCount int
 }
 
-func (c *stubVmMetrics) RecordVmExecutionTime(_ string, _ time.Duration) {
+func (c *stubVmMetrics) RecordExecutionTime(_ time.Duration) {
 	c.executionTimeRecordCount++
 }
+
+func (c *stubVmMetrics) RecordMemoryUsed(_ uint64) {
+}
diff --git a/op-challenger/metrics/metrics.go b/op-challenger/metrics/metrics.go
@@ -38,8 +38,6 @@ type Metricer interface {
 	RecordGameStep()
 	RecordGameMove()
 	RecordGameL2Challenge()
-	RecordVmExecutionTime(vmType string, t time.Duration)
-	RecordVmMemoryUsed(vmType string, memoryUsed uint64)
 	RecordClaimResolutionTime(t float64)
 	RecordGameActTime(t float64)
 
@@ -60,6 +58,10 @@ type Metricer interface {
 	DecActiveExecutors()
 	IncIdleExecutors()
 	DecIdleExecutors()
+
+	// Record vm execution metrics
+	VmMetricer
+	VmMetrics(vmType string) *VmMetrics
 }
 
 // Metrics implementation must implement RegistryMetricer to allow the metrics server to work.
@@ -339,3 +341,7 @@ func (m *Metrics) RecordGameUpdateScheduled() {
 func (m *Metrics) RecordGameUpdateCompleted() {
 	m.inflightGames.Sub(1)
 }
+
+func (m *Metrics) VmMetrics(vmType string) *VmMetrics {
+	return NewVmMetrics(m, vmType)
+}
diff --git a/op-challenger/metrics/noop.go b/op-challenger/metrics/noop.go
@@ -56,3 +56,7 @@ func (*NoopMetricsImpl) DecIdleExecutors()   {}
 
 func (*NoopMetricsImpl) CacheAdd(_ string, _ int, _ bool) {}
 func (*NoopMetricsImpl) CacheGet(_ string, _ bool)        {}
+
+func (m *NoopMetricsImpl) VmMetrics(vmType string) *VmMetrics {
+	return NewVmMetrics(m, vmType)
+}
diff --git a/op-challenger/metrics/vm.go b/op-challenger/metrics/vm.go
@@ -0,0 +1,28 @@
+package metrics
+
+import "time"
+
+type VmMetricer interface {
+	RecordVmExecutionTime(vmType string, t time.Duration)
+	RecordVmMemoryUsed(vmType string, memoryUsed uint64)
+}
+
+type VmMetrics struct {
+	m      VmMetricer
+	vmType string
+}
+
+func NewVmMetrics(m VmMetricer, vmType string) *VmMetrics {
+	return &VmMetrics{
+		m:      m,
+		vmType: vmType,
+	}
+}
+
+func (m *VmMetrics) RecordExecutionTime(dur time.Duration) {
+	m.m.RecordVmExecutionTime(m.vmType, dur)
+}
+
+func (m *VmMetrics) RecordMemoryUsed(memoryUsed uint64) {
+	m.m.RecordVmMemoryUsed(m.vmType, memoryUsed)
+}
diff --git a/op-challenger/runner/runner.go b/op-challenger/runner/runner.go
@@ -17,8 +17,8 @@ import (
 	"github.com/ethereum-optimism/optimism/op-challenger/game/fault/contracts"
 	contractMetrics "github.com/ethereum-optimism/optimism/op-challenger/game/fault/contracts/metrics"
 	"github.com/ethereum-optimism/optimism/op-challenger/game/fault/trace/utils"
-	"github.com/ethereum-optimism/optimism/op-challenger/game/fault/trace/vm"
 	"github.com/ethereum-optimism/optimism/op-challenger/game/fault/types"
+	"github.com/ethereum-optimism/optimism/op-challenger/metrics"
 	"github.com/ethereum-optimism/optimism/op-service/cliapp"
 	"github.com/ethereum-optimism/optimism/op-service/dial"
 	"github.com/ethereum-optimism/optimism/op-service/httputil"
@@ -29,14 +29,17 @@ import (
 	"github.com/ethereum/go-ethereum/log"
 )
 
+const mtCannonType = "mt-cannon"
+
 var (
 	ErrUnexpectedStatusCode = errors.New("unexpected status code")
 )
 
 type Metricer interface {
-	vm.Metricer
 	contractMetrics.ContractMetricer
 
+	RecordVmExecutionTime(vmType string, t time.Duration)
+	RecordVmMemoryUsed(vmType string, memoryUsed uint64)
 	RecordFailure(vmType string)
 	RecordInvalid(vmType string)
 	RecordSuccess(vmType string)
@@ -155,21 +158,21 @@ func (r *Runner) runAndRecordOnce(ctx context.Context, traceType types.TraceType
 		wg.Add(1)
 		go func() {
 			defer wg.Done()
-			dir, err := r.prepDatadir("mt-cannon")
+			dir, err := r.prepDatadir(mtCannonType)
 			if err != nil {
-				recordError(err, "mt-cannon", r.m, r.log)
+				recordError(err, mtCannonType, r.m, r.log)
 				return
 			}
-			logger := inputsLogger.With("type", "mt-cannon")
-			err = r.runMTOnce(ctx, logger, traceType, prestateHash, localInputs, dir)
-			recordError(err, traceType.String(), r.m, r.log.With("mt-cannon", true))
+			logger := inputsLogger.With("type", mtCannonType)
+			err = r.runMTOnce(ctx, logger, localInputs, dir)
+			recordError(err, mtCannonType, r.m, r.log.With(mtCannonType, true))
 		}()
 	}
 	wg.Wait()
 }
 
 func (r *Runner) runOnce(ctx context.Context, logger log.Logger, traceType types.TraceType, prestateHash common.Hash, localInputs utils.LocalGameInputs, dir string) error {
-	provider, err := createTraceProvider(logger, r.m, r.cfg, prestateHash, traceType, localInputs, dir)
+	provider, err := createTraceProvider(logger, metrics.NewVmMetrics(r.m, traceType.String()), r.cfg, prestateHash, traceType, localInputs, dir)
 	if err != nil {
 		return fmt.Errorf("failed to create trace provider: %w", err)
 	}
@@ -183,8 +186,8 @@ func (r *Runner) runOnce(ctx context.Context, logger log.Logger, traceType types
 	return nil
 }
 
-func (r *Runner) runMTOnce(ctx context.Context, logger log.Logger, traceType types.TraceType, prestateHash common.Hash, localInputs utils.LocalGameInputs, dir string) error {
-	provider, err := createMTTraceProvider(logger, r.m, r.cfg.Cannon, r.addMTCannonPrestate, r.addMTCannonPrestateURL, types.TraceTypeCannon, localInputs, dir)
+func (r *Runner) runMTOnce(ctx context.Context, logger log.Logger, localInputs utils.LocalGameInputs, dir string) error {
+	provider, err := createMTTraceProvider(logger, metrics.NewVmMetrics(r.m, mtCannonType), r.cfg.Cannon, r.addMTCannonPrestate, r.addMTCannonPrestateURL, types.TraceTypeCannon, localInputs, dir)
 	if err != nil {
 		return fmt.Errorf("failed to create trace provider: %w", err)
 	}

diff --git a/op-e2e/e2eutils/disputegame/output_cannon_helper.go b/op-e2e/e2eutils/disputegame/output_cannon_helper.go
@@ -316,7 +316,7 @@ func (g *OutputCannonGameHelper) createCannonTraceProvider(ctx context.Context,
 		localContext = outputs.CreateLocalContext(pre, post)
 		dir := filepath.Join(cfg.Datadir, "cannon-trace")
 		subdir := filepath.Join(dir, localContext.Hex())
-		return cannon.NewTraceProviderForTest(logger, metrics.NoopMetrics, cfg, localInputs, subdir, g.MaxDepth(ctx)-splitDepth-1), nil
+		return cannon.NewTraceProviderForTest(logger, metrics.NoopMetrics.VmMetrics(types.TraceTypeCannon.String()), cfg, localInputs, subdir, g.MaxDepth(ctx)-splitDepth-1), nil
 	})
 
 	claims, err := g.Game.GetAllClaims(ctx, rpcblock.Latest)

diff --git a/op-e2e/faultproofs/precompile_test.go b/op-e2e/faultproofs/precompile_test.go
@@ -257,7 +257,7 @@ func runCannon(t *testing.T, ctx context.Context, sys *e2esys.System, inputs uti
 	cannonOpts(&cfg)
 
 	logger := testlog.Logger(t, log.LevelInfo).New("role", "cannon")
-	executor := vm.NewExecutor(logger, metrics.NoopMetrics, cfg.Cannon, vm.NewOpProgramServerExecutor(), cfg.CannonAbsolutePreState, inputs)
+	executor := vm.NewExecutor(logger, metrics.NoopMetrics.VmMetrics("cannon"), cfg.Cannon, vm.NewOpProgramServerExecutor(), cfg.CannonAbsolutePreState, inputs)
 
 	t.Log("Running cannon")
 	err := executor.DoGenerateProof(ctx, proofsDir, math.MaxUint, math.MaxUint, extraVmArgs...)