experimental: remove error parameter from FunctionListener.After (#1452)

Signed-off-by: Achille Roussel <achille.roussel@gmail.com>
This commit is contained in:
Achille
2023-05-10 15:48:26 -07:00
committed by GitHub
parent 4dd4d54cda
commit a105c43629
10 changed files with 59 additions and 91 deletions

View File

@@ -55,14 +55,14 @@ type FunctionListener interface {
// instance or parent of the result.
// - mod: the calling module.
// - def: the function definition.
// - paramValues: api.ValueType encoded parameters.
// - params: api.ValueType encoded parameters.
// - stackIterator: iterator on the call stack. At least one entry is
// guaranteed (the called function), whose Args() will be equal to
// paramValues. The iterator will be reused between calls to Before.
// params. The iterator will be reused between calls to Before.
//
// Note: api.Memory is meant for inspection, not modification.
// mod can be cast to InternalModule to read non-exported globals.
Before(ctx context.Context, mod api.Module, def api.FunctionDefinition, paramValues []uint64, stackIterator StackIterator) context.Context
Before(ctx context.Context, mod api.Module, def api.FunctionDefinition, params []uint64, stackIterator StackIterator) context.Context
// After is invoked after a function is called.
//
@@ -71,11 +71,10 @@ type FunctionListener interface {
// - ctx: the context returned by Before.
// - mod: the calling module.
// - def: the function definition.
// - err: nil if the function didn't err
// - resultValues: api.ValueType encoded results.
// - results: api.ValueType encoded results.
//
// Note: api.Memory is meant for inspection, not modification.
After(ctx context.Context, mod api.Module, def api.FunctionDefinition, err error, resultValues []uint64)
After(ctx context.Context, mod api.Module, def api.FunctionDefinition, results []uint64)
}
// FunctionListenerFunc is a function type implementing the FunctionListener
@@ -89,14 +88,14 @@ type FunctionListener interface {
type FunctionListenerFunc func(context.Context, api.Module, api.FunctionDefinition, []uint64, StackIterator)
// Before satisfies the FunctionListener interface, calls f.
func (f FunctionListenerFunc) Before(ctx context.Context, mod api.Module, def api.FunctionDefinition, paramValues []uint64, stackIterator StackIterator) context.Context {
f(ctx, mod, def, paramValues, stackIterator)
func (f FunctionListenerFunc) Before(ctx context.Context, mod api.Module, def api.FunctionDefinition, params []uint64, stackIterator StackIterator) context.Context {
f(ctx, mod, def, params, stackIterator)
return ctx
}
// After is declared to satisfy the FunctionListener interface, but it does
// nothing.
func (f FunctionListenerFunc) After(context.Context, api.Module, api.FunctionDefinition, error, []uint64) {
func (f FunctionListenerFunc) After(context.Context, api.Module, api.FunctionDefinition, []uint64) {
}
// FunctionListenerFactoryFunc is a function type implementing the
@@ -159,9 +158,9 @@ func (multi *multiFunctionListener) Before(ctx context.Context, mod api.Module,
return ctx
}
func (multi *multiFunctionListener) After(ctx context.Context, mod api.Module, def api.FunctionDefinition, err error, results []uint64) {
func (multi *multiFunctionListener) After(ctx context.Context, mod api.Module, def api.FunctionDefinition, results []uint64) {
for _, lstn := range multi.lstns {
lstn.After(ctx, mod, def, err, results)
lstn.After(ctx, mod, def, results)
}
}
@@ -312,16 +311,16 @@ func BenchmarkFunctionListener(n int, module api.Module, stack []StackFrame, lis
panic("cannot benchmark function listener with an empty stack")
}
functionDefinition := stack[0].Function.Definition()
functionParams := stack[0].Params
functionResults := stack[0].Results
stackIterator := &stackIterator{base: NewStackIterator(stack...)}
ctx := context.Background()
def := stack[0].Function.Definition()
params := stack[0].Params
results := stack[0].Results
stackIterator := &stackIterator{base: NewStackIterator(stack...)}
for i := 0; i < n; i++ {
stackIterator.index = -1
callContext := listener.Before(ctx, module, functionDefinition, functionParams, stackIterator)
listener.After(callContext, module, functionDefinition, nil, functionResults)
callContext := listener.Before(ctx, module, def, params, stackIterator)
listener.After(callContext, module, def, results)
}
}
@@ -329,8 +328,6 @@ func BenchmarkFunctionListener(n int, module api.Module, stack []StackFrame, lis
// indirect in terms of depth and breadth. The test could show a tree 3 calls deep where the there are a couple calls at
// each depth under the root. The main thing this can help prevent is accidentally swapping the context internally.
// TODO: Errors aren't handled, and the After hook should accept one along with the result values.
// TODO: The context parameter of the After hook is not the same as the Before hook. This means interceptor patterns
// are awkward. e.g. something like timing is difficult as it requires propagating a stack. Otherwise, nested calls will
// overwrite each other's "since" time. Propagating a stack is further awkward as the After hook needs to know the

View File

@@ -50,7 +50,7 @@ func (u uniqGoFuncs) Before(ctx context.Context, _ api.Module, def api.FunctionD
}
// After implements FunctionListener.After
func (u uniqGoFuncs) After(context.Context, api.Module, api.FunctionDefinition, error, []uint64) {}
func (u uniqGoFuncs) After(context.Context, api.Module, api.FunctionDefinition, []uint64) {}
// This shows how to make a listener that counts go function calls.
func Example_customListenerFactory() {

View File

@@ -27,7 +27,7 @@ func (r *recorder) Before(ctx context.Context, _ api.Module, def api.FunctionDef
return ctx
}
func (r *recorder) After(_ context.Context, _ api.Module, def api.FunctionDefinition, _ error, _ []uint64) {
func (r *recorder) After(_ context.Context, _ api.Module, def api.FunctionDefinition, _ []uint64) {
r.afterNames = append(r.afterNames, def.DebugName())
}
@@ -116,7 +116,7 @@ func TestMultiFunctionListenerFactory(t *testing.T) {
}
n := 0
f := func(ctx context.Context, mod api.Module, def api.FunctionDefinition, paramValues []uint64, stackIterator experimental.StackIterator) {
f := func(ctx context.Context, mod api.Module, def api.FunctionDefinition, params []uint64, stackIterator experimental.StackIterator) {
n++
i := 0
for stackIterator.Next() {
@@ -180,12 +180,12 @@ func BenchmarkMultiFunctionListener(b *testing.B) {
}{
{
scenario: "simple function listener",
function: func(ctx context.Context, mod api.Module, def api.FunctionDefinition, paramValues []uint64, stackIterator experimental.StackIterator) {
function: func(ctx context.Context, mod api.Module, def api.FunctionDefinition, params []uint64, stackIterator experimental.StackIterator) {
},
},
{
scenario: "stack iterator",
function: func(ctx context.Context, mod api.Module, def api.FunctionDefinition, paramValues []uint64, stackIterator experimental.StackIterator) {
function: func(ctx context.Context, mod api.Module, def api.FunctionDefinition, params []uint64, stackIterator experimental.StackIterator) {
for stackIterator.Next() {
}
},

View File

@@ -203,7 +203,9 @@ func (l *loggingListener) Before(ctx context.Context, mod api.Module, _ api.Func
// We're starting to log: increase the indentation level.
nestLevel++
l.logIndented(ctx, mod, nestLevel, true, params, nil, nil)
l.logIndented(nestLevel, l.beforePrefix, func() {
l.logParams(ctx, mod, params)
})
// We need to propagate this invocation's parameters to the after callback.
state = &logState{w: l.w, nestLevel: nestLevel}
@@ -220,34 +222,27 @@ func (l *loggingListener) Before(ctx context.Context, mod api.Module, _ api.Func
// After logs to stdout the module and function name, prefixed with '<--' and
// indented based on the call nesting level.
func (l *loggingListener) After(ctx context.Context, mod api.Module, _ api.FunctionDefinition, err error, results []uint64) {
func (l *loggingListener) After(ctx context.Context, mod api.Module, _ api.FunctionDefinition, results []uint64) {
// Note: We use the nest level directly even though it is the "next" nesting level.
// This works because our indent of zero nesting is one tab.
if state, ok := ctx.Value(logging.LoggerKey{}).(*logState); ok {
if state == unsampledLogState {
return
}
l.logIndented(ctx, mod, state.nestLevel, false, state.params, err, results)
l.logIndented(state.nestLevel, l.afterPrefix, func() {
l.logResults(ctx, mod, state.params, results)
})
}
}
// logIndented logs an indented l.w like this: "-->\t\t\t$nestLevel$funcName\n"
func (l *loggingListener) logIndented(ctx context.Context, mod api.Module, nestLevel int, isBefore bool, params []uint64, err error, results []uint64) {
// logIndented writes an indentation level and prefix prior to calling log to
// output the log line.
func (l *loggingListener) logIndented(nestLevel int, prefix string, log func()) {
for i := 1; i < nestLevel; i++ {
l.w.WriteByte('\t') //nolint
}
if isBefore { // before
l.w.WriteString(l.beforePrefix) //nolint
l.logParams(ctx, mod, params)
} else { // after
l.w.WriteString(l.afterPrefix) //nolint
if err != nil {
l.w.WriteString(" error: ") //nolint
l.w.WriteString(err.Error()) //nolint
} else {
l.logResults(ctx, mod, params, results)
}
}
l.w.WriteString(prefix) //nolint
log()
l.w.WriteByte('\n') //nolint
if f, ok := l.w.(flusher); ok {

View File

@@ -3,7 +3,6 @@ package logging_test
import (
"bytes"
"context"
"io"
"math"
"testing"
@@ -36,7 +35,6 @@ func Test_loggingListener(t *testing.T) {
isHostFunc bool
paramNames, resultNames []string
params, results []uint64
err error
expected string
}{
{
@@ -44,14 +42,6 @@ func Test_loggingListener(t *testing.T) {
functype: wasm.FunctionType{},
expected: `--> test.fn()
<--
`,
},
{
name: "error",
functype: wasm.FunctionType{},
err: io.EOF,
expected: `--> test.fn()
<-- error: EOF
`,
},
{
@@ -88,20 +78,6 @@ func Test_loggingListener(t *testing.T) {
results: []uint64{uint64(wasi.ErrnoFault)},
expected: `==> wasi_snapshot_preview1.random_get(buf=0,buf_len=8)
<== errno=EFAULT
`,
},
{
name: "wasi error",
functype: wasiFuncType,
moduleName: wasi.InternalModuleName,
funcName: wasiFuncName,
paramNames: wasiParamNames,
resultNames: wasiResultNames,
isHostFunc: true,
params: wasiParams,
err: io.EOF, // not possible as we coerce errors to numbers, but test anyway!
expected: `==> wasi_snapshot_preview1.random_get(buf=0,buf_len=8)
<== error: EOF
`,
},
{
@@ -304,7 +280,7 @@ func Test_loggingListener(t *testing.T) {
out.Reset()
ctx := l.Before(testCtx, nil, def, tc.params, nil)
l.After(ctx, nil, def, tc.err, tc.results)
l.After(ctx, nil, def, tc.results)
require.Equal(t, tc.expected, out.String())
})
}
@@ -341,8 +317,8 @@ func Test_loggingListener_indentation(t *testing.T) {
ctx := l1.Before(testCtx, nil, def1, []uint64{}, nil)
ctx1 := l2.Before(ctx, nil, def2, []uint64{}, nil)
l2.After(ctx1, nil, def2, nil, []uint64{})
l1.After(ctx, nil, def1, nil, []uint64{})
l2.After(ctx1, nil, def2, []uint64{})
l1.After(ctx, nil, def1, []uint64{})
require.Equal(t, `--> test.fn1()
--> test.fn2()
<--

View File

@@ -1183,7 +1183,7 @@ func (ce *callEngine) builtinFunctionFunctionListenerBefore(ctx context.Context,
func (ce *callEngine) builtinFunctionFunctionListenerAfter(ctx context.Context, mod api.Module, fn *function) {
base := int(ce.stackBasePointerInBytes >> 3)
fn.parent.listener.After(ctx, mod, fn.definition(), nil, ce.stack[base:base+fn.funcType.ResultNumInUint64])
fn.parent.listener.After(ctx, mod, fn.definition(), ce.stack[base:base+fn.funcType.ResultNumInUint64])
i := len(ce.contextStack) - 1
ce.ctx = ce.contextStack[i]

View File

@@ -17,7 +17,7 @@ func BenchmarkCallEngine_builtinFunctionFunctionListener(b *testing.B) {
before: func(context.Context, api.Module, api.FunctionDefinition, []uint64, experimental.StackIterator) context.Context {
return context.Background()
},
after: func(context.Context, api.Module, api.FunctionDefinition, error, []uint64) {
after: func(context.Context, api.Module, api.FunctionDefinition, []uint64) {
},
},
index: 0,

View File

@@ -589,9 +589,9 @@ func TestCallEngine_builtinFunctionFunctionListenerBefore(t *testing.T) {
funcType: &wasm.FunctionType{ParamNumInUint64: 3},
parent: &compiledFunction{
listener: mockListener{
before: func(ctx context.Context, _ api.Module, def api.FunctionDefinition, paramValues []uint64, stackIterator experimental.StackIterator) context.Context {
before: func(ctx context.Context, _ api.Module, def api.FunctionDefinition, params []uint64, stackIterator experimental.StackIterator) context.Context {
require.Equal(t, currentContext, ctx)
require.Equal(t, []uint64{2, 3, 4}, paramValues)
require.Equal(t, []uint64{2, 3, 4}, params)
assertStackIterator(t, stackIterator, []stackEntry{{def: def, args: []uint64{2, 3, 4}}})
return nextContext
},
@@ -620,9 +620,9 @@ func TestCallEngine_builtinFunctionFunctionListenerAfter(t *testing.T) {
funcType: &wasm.FunctionType{ResultNumInUint64: 1},
parent: &compiledFunction{
listener: mockListener{
after: func(ctx context.Context, mod api.Module, def api.FunctionDefinition, err error, resultValues []uint64) {
after: func(ctx context.Context, mod api.Module, def api.FunctionDefinition, results []uint64) {
require.Equal(t, currentContext, ctx)
require.Equal(t, []uint64{5}, resultValues)
require.Equal(t, []uint64{5}, results)
},
},
index: 0,
@@ -645,16 +645,16 @@ func TestCallEngine_builtinFunctionFunctionListenerAfter(t *testing.T) {
}
type mockListener struct {
before func(ctx context.Context, mod api.Module, def api.FunctionDefinition, paramValues []uint64, stackIterator experimental.StackIterator) context.Context
after func(ctx context.Context, mod api.Module, def api.FunctionDefinition, err error, resultValues []uint64)
before func(ctx context.Context, mod api.Module, def api.FunctionDefinition, params []uint64, stackIterator experimental.StackIterator) context.Context
after func(ctx context.Context, mod api.Module, def api.FunctionDefinition, results []uint64)
}
func (m mockListener) Before(ctx context.Context, mod api.Module, def api.FunctionDefinition, paramValues []uint64, stackIterator experimental.StackIterator) context.Context {
return m.before(ctx, mod, def, paramValues, stackIterator)
func (m mockListener) Before(ctx context.Context, mod api.Module, def api.FunctionDefinition, params []uint64, stackIterator experimental.StackIterator) context.Context {
return m.before(ctx, mod, def, params, stackIterator)
}
func (m mockListener) After(ctx context.Context, mod api.Module, def api.FunctionDefinition, err error, resultValues []uint64) {
m.after(ctx, mod, def, err, resultValues)
func (m mockListener) After(ctx context.Context, mod api.Module, def api.FunctionDefinition, results []uint64) {
m.after(ctx, mod, def, results)
}
func TestFunction_getSourceOffsetInWasmBinary(t *testing.T) {

View File

@@ -608,7 +608,7 @@ func (ce *callEngine) callGoFunc(ctx context.Context, m *wasm.ModuleInstance, f
if lsn != nil {
// TODO: This doesn't get the error due to use of panic to propagate them.
results := stack[:typ.ResultNumInUint64]
lsn.After(ctx, m, f.definition(), nil, results)
lsn.After(ctx, m, f.definition(), results)
}
}
@@ -4087,7 +4087,7 @@ func (ce *callEngine) callNativeFuncWithListener(ctx context.Context, m *wasm.Mo
ctx = fnl.Before(ctx, m, def, ce.peekValues(len(typ.Params)), &ce.stackIterator)
ce.stackIterator.clear()
ce.callNativeFunc(ctx, m, f)
fnl.After(ctx, m, def, nil, ce.peekValues(len(typ.Results)))
fnl.After(ctx, m, def, ce.peekValues(len(typ.Results)))
return ctx
}

View File

@@ -556,7 +556,7 @@ func RunTestModuleEngineBeforeListenerStackIterator(t *testing.T, et EngineTeste
}
fnListener := &fnListener{
beforeFn: func(ctx context.Context, mod api.Module, def api.FunctionDefinition, paramValues []uint64, si experimental.StackIterator) context.Context {
beforeFn: func(ctx context.Context, mod api.Module, def api.FunctionDefinition, params []uint64, si experimental.StackIterator) context.Context {
require.True(t, len(expectedCallstacks) > 0)
expectedCallstack := expectedCallstacks[0]
for si.Next() {
@@ -693,7 +693,7 @@ func RunTestModuleEngineBeforeListenerGlobals(t *testing.T, et EngineTester) {
}
fnListener := &fnListener{
beforeFn: func(ctx context.Context, mod api.Module, def api.FunctionDefinition, paramValues []uint64, si experimental.StackIterator) context.Context {
beforeFn: func(ctx context.Context, mod api.Module, def api.FunctionDefinition, params []uint64, si experimental.StackIterator) context.Context {
require.True(t, len(expectedGlobals) > 0)
imod := mod.(experimental.InternalModule)
@@ -803,24 +803,24 @@ func RunTestModuleEngineBeforeListenerGlobals(t *testing.T, et EngineTester) {
}
type fnListener struct {
beforeFn func(ctx context.Context, mod api.Module, def api.FunctionDefinition, paramValues []uint64, stackIterator experimental.StackIterator) context.Context
afterFn func(ctx context.Context, mod api.Module, def api.FunctionDefinition, err error, resultValues []uint64)
beforeFn func(ctx context.Context, mod api.Module, def api.FunctionDefinition, params []uint64, stackIterator experimental.StackIterator) context.Context
afterFn func(ctx context.Context, mod api.Module, def api.FunctionDefinition, results []uint64)
}
func (f *fnListener) NewFunctionListener(api.FunctionDefinition) experimental.FunctionListener {
return f
}
func (f *fnListener) Before(ctx context.Context, mod api.Module, def api.FunctionDefinition, paramValues []uint64, stackIterator experimental.StackIterator) context.Context {
func (f *fnListener) Before(ctx context.Context, mod api.Module, def api.FunctionDefinition, params []uint64, stackIterator experimental.StackIterator) context.Context {
if f.beforeFn != nil {
return f.beforeFn(ctx, mod, def, paramValues, stackIterator)
return f.beforeFn(ctx, mod, def, params, stackIterator)
}
return ctx
}
func (f *fnListener) After(ctx context.Context, mod api.Module, def api.FunctionDefinition, err error, resultValues []uint64) {
func (f *fnListener) After(ctx context.Context, mod api.Module, def api.FunctionDefinition, results []uint64) {
if f.afterFn != nil {
f.afterFn(ctx, mod, def, err, resultValues)
f.afterFn(ctx, mod, def, results)
}
}
@@ -835,7 +835,7 @@ func RunTestModuleEngineStackIteratorOffset(t *testing.T, et EngineTester) {
var tape [][]frame
fnListener := &fnListener{
beforeFn: func(ctx context.Context, mod api.Module, def api.FunctionDefinition, paramValues []uint64, si experimental.StackIterator) context.Context {
beforeFn: func(ctx context.Context, mod api.Module, def api.FunctionDefinition, params []uint64, si experimental.StackIterator) context.Context {
var stack []frame
for si.Next() {
fn := si.Function()