Skip to content

Commit

Permalink
Basic implementation of variant 2 from #1802
Browse files Browse the repository at this point in the history
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
  • Loading branch information
mstoykov committed Jun 24, 2021
1 parent 2039c56 commit 1354e4f
Show file tree
Hide file tree
Showing 4 changed files with 72 additions and 21 deletions.
8 changes: 8 additions & 0 deletions js/initcontext.go
Original file line number Diff line number Diff line change
Expand Up @@ -147,6 +147,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
}

Expand Down
71 changes: 53 additions & 18 deletions js/modules/k6/metrics/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -43,32 +43,39 @@ 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)
return common.Bind(rt, Metric{stats.New(name, t, valueType)}, ctxPtr), nil
return rt.ToValue(Metric{metric: stats.New(name, t, valueType), getContext: m.getContext}).ToObject(rt), 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
Expand Down Expand Up @@ -96,24 +103,52 @@ func (m Metric) GetName() string {
return m.metric.Name
}

type Metrics struct{}
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 (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 (*Metrics) XRate(ctx *context.Context, name string, isTime ...bool) (interface{}, error) {
return newMetric(ctx, name, stats.Rate, isTime)
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
}
7 changes: 4 additions & 3 deletions js/modules/k6/metrics/metrics_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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{
Expand Down
7 changes: 7 additions & 0 deletions js/modules/modules.go
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
package modules

import (
"context"
"fmt"
"strings"
"sync"
Expand Down Expand Up @@ -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()
Expand Down

0 comments on commit 1354e4f

Please sign in to comment.