Skip to content

Commit

Permalink
Fix data races with shared global metrics
Browse files Browse the repository at this point in the history
I don't think this would have been a problem with real-world usage of k6 (for now), but the way metrics are defined should probably be refactored to avoid shared global variables.
  • Loading branch information
na-- committed Apr 2, 2018
1 parent c68ff5f commit f157788
Show file tree
Hide file tree
Showing 3 changed files with 5 additions and 3 deletions.
4 changes: 2 additions & 2 deletions core/engine.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ type Engine struct {
logger *log.Logger

Metrics map[string]*stats.Metric
MetricsLock sync.RWMutex
MetricsLock sync.Mutex

// Assigned to metrics upon first received sample.
thresholds map[string]stats.Thresholds
Expand Down Expand Up @@ -319,7 +319,7 @@ func (e *Engine) processSamples(samples ...stats.Sample) {
for i, sample := range samples {
m, ok := e.Metrics[sample.Metric.Name]
if !ok {
m = sample.Metric
m = stats.New(sample.Metric.Name, sample.Metric.Type, sample.Metric.Contains)
m.Thresholds = e.thresholds[m.Name]
m.Submetrics = e.submetrics[m.Name]
e.Metrics[m.Name] = m
Expand Down
2 changes: 1 addition & 1 deletion core/engine_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -548,7 +548,7 @@ func TestSentReceivedMetrics(t *testing.T) {
getTestCase := func(t *testing.T, tc testCase) func(t *testing.T) {
return func(t *testing.T) {
//TODO: figure out why it fails if we uncomment this:
//t.Parallel()
t.Parallel()
r, err := js.New(&lib.SourceData{
Filename: "/script.js",
Data: []byte(`
Expand Down
2 changes: 2 additions & 0 deletions lib/metrics/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,8 @@ import (
"github.com/loadimpact/k6/stats"
)

//TODO: refactor this, using non thread-safe global variables seems like a bad idea for various reasons...

var (
// Engine-emitted.
VUs = stats.New("vus", stats.Gauge)
Expand Down

0 comments on commit f157788

Please sign in to comment.