From 112c94b1f4fc44b8d1e048a60e0b06ad2695c7a0 Mon Sep 17 00:00:00 2001 From: Mihail Stoykov Date: Thu, 24 Jun 2021 13:03:41 +0300 Subject: [PATCH] Basic implementation of variant 2 from #1802 In this case we give the Module a function to get the context and it needs to that whenever it needs it instead of just caching it's value ... Especially between function call or something like that --- js/initcontext.go | 8 +++ js/modules/k6/metrics/metrics.go | 87 +++++++++++++++++++-------- js/modules/k6/metrics/metrics_test.go | 7 ++- js/modules/modules.go | 7 +++ 4 files changed, 82 insertions(+), 27 deletions(-) diff --git a/js/initcontext.go b/js/initcontext.go index ca443566468b..18786552284f 100644 --- a/js/initcontext.go +++ b/js/initcontext.go @@ -148,6 +148,14 @@ func (i *InitContext) requireModule(name string) (goja.Value, error) { if perInstance, ok := mod.(modules.HasModuleInstancePerVU); ok { mod = perInstance.NewModuleInstancePerVU() } + + if withContext, ok := mod.(modules.HasWithContext); ok { + withContext.WithContext(func() context.Context { + return *i.ctxPtr + }) + return i.runtime.ToValue(mod), nil + } + return i.runtime.ToValue(common.Bind(i.runtime, mod, i.ctxPtr)), nil } diff --git a/js/modules/k6/metrics/metrics.go b/js/modules/k6/metrics/metrics.go index d950eb5ea74f..a61dcb72b447 100644 --- a/js/modules/k6/metrics/metrics.go +++ b/js/modules/k6/metrics/metrics.go @@ -43,41 +43,47 @@ func checkName(name string) bool { } type Metric struct { - metric *stats.Metric + metric *stats.Metric + getContext func() context.Context } // ErrMetricsAddInInitContext is error returned when adding to metric is done in the init context var ErrMetricsAddInInitContext = common.NewInitContextError("Adding to metrics in the init context is not supported") -func newMetric(ctxPtr *context.Context, name string, t stats.MetricType, isTime []bool) (interface{}, error) { - if lib.GetState(*ctxPtr) != nil { +func (m *MetricsModule) newMetric(call goja.ConstructorCall, t stats.MetricType) (*goja.Object, error) { + ctx := m.getContext() + if lib.GetState(ctx) != nil { return nil, errors.New("metrics must be declared in the init context") } + rt := common.GetRuntime(ctx) // NOTE we can get this differently as well + // TODO this kind of conversions can possibly be automated by the parts of common.Bind that are curently automating + // it and some wrapping + name := call.Argument(0).String() + isTime := call.Argument(1).ToBoolean() // TODO: move verification outside the JS if !checkName(name) { return nil, common.NewInitContextError(fmt.Sprintf("Invalid metric name: '%s'", name)) } valueType := stats.Default - if len(isTime) > 0 && isTime[0] { + if isTime { valueType = stats.Time } - rt := common.GetRuntime(*ctxPtr) - bound := common.Bind(rt, Metric{stats.New(name, t, valueType)}, ctxPtr) - o := rt.NewObject() - err := o.DefineDataProperty("name", rt.ToValue(name), goja.FLAG_FALSE, goja.FLAG_FALSE, goja.FLAG_TRUE) + mo := Metric{metric: stats.New(name, t, valueType), getContext: m.getContext} + mobj := rt.ToValue(mo).ToObject(rt) + + err := mobj.DefineDataProperty("name", rt.ToValue(name), goja.FLAG_FALSE, goja.FLAG_FALSE, goja.FLAG_TRUE) if err != nil { return nil, err } - if err = o.Set("add", rt.ToValue(bound["add"])); err != nil { - return nil, err - } - return o, nil + + return mobj, nil } -func (m Metric) Add(ctx context.Context, v goja.Value, addTags ...map[string]string) (bool, error) { +func (m Metric) Add(v goja.Value, addTags ...map[string]string) (bool, error) { + ctx := m.getContext() state := lib.GetState(ctx) if state == nil { return false, ErrMetricsAddInInitContext @@ -100,24 +106,57 @@ func (m Metric) Add(ctx context.Context, v goja.Value, addTags ...map[string]str return true, nil } -type Metrics struct{} +// GetName returns the metric name +func (m Metric) GetName() string { + return m.metric.Name +} + +type MetricsModule struct { + getContext func() context.Context +} -func New() *Metrics { - return &Metrics{} +func (m *MetricsModule) WithContext(getContext func() context.Context) { + m.getContext = getContext } -func (*Metrics) XCounter(ctx *context.Context, name string, isTime ...bool) (interface{}, error) { - return newMetric(ctx, name, stats.Counter, isTime) +func New() *MetricsModule { + return &MetricsModule{} } -func (*Metrics) XGauge(ctx *context.Context, name string, isTime ...bool) (interface{}, error) { - return newMetric(ctx, name, stats.Gauge, isTime) +// This is not possible after common.Bind as it wraps the object and doesn't return the original one. +func (m *MetricsModule) ReturnMetricType(metric Metric) string { + return metric.metric.Type.String() } -func (*Metrics) XTrend(ctx *context.Context, name string, isTime ...bool) (interface{}, error) { - return newMetric(ctx, name, stats.Trend, isTime) +// Counter ... // NOTE we still need to use goja.ConstructorCall somewhere to have access to the +func (m *MetricsModule) XCounter(call goja.ConstructorCall, rt *goja.Runtime) *goja.Object { + v, err := m.newMetric(call, stats.Counter) + if err != nil { + common.Throw(rt, err) + } + return v } -func (*Metrics) XRate(ctx *context.Context, name string, isTime ...bool) (interface{}, error) { - return newMetric(ctx, name, stats.Rate, isTime) +func (m *MetricsModule) XGauge(call goja.ConstructorCall, rt *goja.Runtime) *goja.Object { + v, err := m.newMetric(call, stats.Gauge) + if err != nil { + common.Throw(rt, err) + } + return v +} + +func (m *MetricsModule) XTrend(call goja.ConstructorCall, rt *goja.Runtime) *goja.Object { + v, err := m.newMetric(call, stats.Trend) + if err != nil { + common.Throw(rt, err) + } + return v +} + +func (m *MetricsModule) XRate(call goja.ConstructorCall, rt *goja.Runtime) *goja.Object { + v, err := m.newMetric(call, stats.Rate) + if err != nil { + common.Throw(rt, err) + } + return v } diff --git a/js/modules/k6/metrics/metrics_test.go b/js/modules/k6/metrics/metrics_test.go index 28d20de48bf8..219ffd357bc0 100644 --- a/js/modules/k6/metrics/metrics_test.go +++ b/js/modules/k6/metrics/metrics_test.go @@ -64,8 +64,9 @@ func TestMetrics(t *testing.T) { ctxPtr := new(context.Context) *ctxPtr = common.WithRuntime(context.Background(), rt) - rt.Set("metrics", common.Bind(rt, New(), ctxPtr)) - + m := New() + m.WithContext(func() context.Context { return *ctxPtr }) + rt.Set("metrics", m) root, _ := lib.NewGroup("", nil) child, _ := root.Group("child") samples := make(chan stats.SampleContainer, 1000) @@ -86,7 +87,7 @@ func TestMetrics(t *testing.T) { t.Run("ExitInit", func(t *testing.T) { *ctxPtr = lib.WithState(*ctxPtr, state) _, err := rt.RunString(fmt.Sprintf(`new metrics.%s("my_metric")`, fn)) - assert.EqualError(t, err, "metrics must be declared in the init context at apply (native)") + assert.Contains(t, err.Error(), "metrics must be declared in the init context") }) groups := map[string]*lib.Group{ diff --git a/js/modules/modules.go b/js/modules/modules.go index c7c27ff0450f..f8d119b12971 100644 --- a/js/modules/modules.go +++ b/js/modules/modules.go @@ -21,6 +21,7 @@ package modules import ( + "context" "fmt" "strings" "sync" @@ -69,6 +70,12 @@ type HasModuleInstancePerVU interface { NewModuleInstancePerVU() interface{} } +// HasWithContext should be implemented by modules that need access to the context, which should be all of them +type HasWithContext interface { // TODO rename? + // This specifically is a function *returning* context as it can change between invocations + WithContext(func() context.Context) // this can be a different object that just has a context getter +} + // checks that modules implement HasModuleInstancePerVU // this is done here as otherwise there will be a loop if the module imports this package var _ HasModuleInstancePerVU = http.New()