Skip to content

Commit

Permalink
Refactor CSV output (grafana#2560)
Browse files Browse the repository at this point in the history
This PR started with using enumer for the newly added time format but also includes:
1. fixing for the tests as rfc3339 is time zone dependant and those were failing in none UTC time zones
2. rfc3339 is written with three `3` but was written with 2 in most cases previously
3. A bunch of fixes to the tests to user require instead of assert
  • Loading branch information
mstoykov authored Jun 13, 2022

Unverified

This user has not yet uploaded their public signing key.
1 parent e98e25c commit 4bb7b1e
Showing 6 changed files with 141 additions and 78 deletions.
31 changes: 19 additions & 12 deletions output/csv/config.go
Original file line number Diff line number Diff line change
@@ -38,15 +38,25 @@ type Config struct {
// Samples.
FileName null.String `json:"file_name" envconfig:"K6_CSV_FILENAME"`
SaveInterval types.NullDuration `json:"save_interval" envconfig:"K6_CSV_SAVE_INTERVAL"`
TimeFormat TimeFormat `json:"time_format" envconfig:"K6_CSV_TIME_FORMAT"`
TimeFormat null.String `json:"time_format" envconfig:"K6_CSV_TIME_FORMAT"`
}

// TimeFormat custom enum type
//go:generate enumer -type=TimeFormat -transform=snake -trimprefix TimeFormat -output time_format_gen.go
type TimeFormat uint8

// valid defined values for TimeFormat
const (
TimeFormatUnix TimeFormat = iota
TimeFormatRFC3339
)

// NewConfig creates a new Config instance with default values for some fields.
func NewConfig() Config {
return Config{
FileName: null.StringFrom("file.csv"),
SaveInterval: types.NullDurationFrom(1 * time.Second),
TimeFormat: Unix,
FileName: null.NewString("file.csv", false),
SaveInterval: types.NewNullDuration(1*time.Second, false),
TimeFormat: null.NewString("unix", false),
}
}

@@ -58,18 +68,18 @@ func (c Config) Apply(cfg Config) Config {
if cfg.SaveInterval.Valid {
c.SaveInterval = cfg.SaveInterval
}
c.TimeFormat = cfg.TimeFormat
if cfg.TimeFormat.Valid {
c.TimeFormat = cfg.TimeFormat
}
return c
}

// ParseArg takes an arg string and converts it to a config
func ParseArg(arg string, logger *logrus.Logger) (Config, error) {
c := Config{}
c := NewConfig()

if !strings.Contains(arg, "=") {
c.FileName = null.StringFrom(arg)
c.SaveInterval = types.NullDurationFrom(1 * time.Second)
c.TimeFormat = Unix
return c, nil
}

@@ -94,10 +104,7 @@ func ParseArg(arg string, logger *logrus.Logger) (Config, error) {
case "fileName":
c.FileName = null.StringFrom(r[1])
case "timeFormat":
c.TimeFormat = TimeFormat(r[1])
if !c.TimeFormat.IsValid() {
return c, fmt.Errorf("unknown value %q as argument for csv output timeFormat, expected 'unix' or 'rfc3399'", arg)
}
c.TimeFormat = null.StringFrom(r[1])

default:
return c, fmt.Errorf("unknown key %q as argument for csv output", r[0])
39 changes: 24 additions & 15 deletions output/csv/config_test.go
Original file line number Diff line number Diff line change
@@ -30,6 +30,7 @@ import (

"github.com/sirupsen/logrus/hooks/test"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"

"go.k6.io/k6/lib/testutils"
"go.k6.io/k6/lib/types"
@@ -39,36 +40,36 @@ func TestNewConfig(t *testing.T) {
config := NewConfig()
assert.Equal(t, "file.csv", config.FileName.String)
assert.Equal(t, "1s", config.SaveInterval.String())
assert.Equal(t, TimeFormat("unix"), config.TimeFormat)
assert.Equal(t, "unix", config.TimeFormat.String)
}

func TestApply(t *testing.T) {
configs := []Config{
{
FileName: null.StringFrom(""),
SaveInterval: types.NullDurationFrom(2 * time.Second),
TimeFormat: "unix",
TimeFormat: null.StringFrom("unix"),
},
{
FileName: null.StringFrom("newPath"),
SaveInterval: types.NewNullDuration(time.Duration(1), false),
TimeFormat: "unix",
TimeFormat: null.StringFrom("rfc3339"),
},
}
expected := []struct {
FileName string
SaveInterval string
TimeFormat TimeFormat
TimeFormat string
}{
{
FileName: "",
SaveInterval: "2s",
TimeFormat: TimeFormat("unix"),
TimeFormat: "unix",
},
{
FileName: "newPath",
SaveInterval: "1s",
TimeFormat: TimeFormat("unix"),
TimeFormat: "rfc3339",
},
}

@@ -81,7 +82,7 @@ func TestApply(t *testing.T) {

assert.Equal(t, expected.FileName, baseConfig.FileName.String)
assert.Equal(t, expected.SaveInterval, baseConfig.SaveInterval.String())
assert.Equal(t, expected.TimeFormat, baseConfig.TimeFormat)
assert.Equal(t, expected.TimeFormat, baseConfig.TimeFormat.String)
})
}
}
@@ -95,26 +96,32 @@ func TestParseArg(t *testing.T) {
"test_file.csv": {
config: Config{
FileName: null.StringFrom("test_file.csv"),
SaveInterval: types.NullDurationFrom(1 * time.Second),
SaveInterval: types.NewNullDuration(1*time.Second, false),
TimeFormat: null.NewString("unix", false),
},
},
"save_interval=5s": {
config: Config{
FileName: null.NewString("file.csv", false),
SaveInterval: types.NullDurationFrom(5 * time.Second),
TimeFormat: null.NewString("unix", false),
},
expectedLogEntries: []string{
"CSV output argument 'save_interval' is deprecated, please use 'saveInterval' instead.",
},
},
"saveInterval=5s": {
config: Config{
FileName: null.NewString("file.csv", false),
SaveInterval: types.NullDurationFrom(5 * time.Second),
TimeFormat: null.NewString("unix", false),
},
},
"file_name=test.csv,save_interval=5s": {
config: Config{
FileName: null.StringFrom("test.csv"),
SaveInterval: types.NullDurationFrom(5 * time.Second),
TimeFormat: null.NewString("unix", false),
},
expectedLogEntries: []string{
"CSV output argument 'file_name' is deprecated, please use 'fileName' instead.",
@@ -125,6 +132,7 @@ func TestParseArg(t *testing.T) {
config: Config{
FileName: null.StringFrom("test.csv"),
SaveInterval: types.NullDurationFrom(5 * time.Second),
TimeFormat: null.NewString("unix", false),
},
expectedLogEntries: []string{
"CSV output argument 'save_interval' is deprecated, please use 'saveInterval' instead.",
@@ -133,10 +141,11 @@ func TestParseArg(t *testing.T) {
"filename=test.csv,save_interval=5s": {
expectedErr: true,
},
"fileName=test.csv,timeFormat=rfc3399": {
"fileName=test.csv,timeFormat=rfc3339": {
config: Config{
FileName: null.StringFrom("test.csv"),
TimeFormat: "rfc3399",
FileName: null.StringFrom("test.csv"),
SaveInterval: types.NewNullDuration(1*time.Second, false),
TimeFormat: null.StringFrom("rfc3339"),
},
},
}
@@ -153,11 +162,11 @@ func TestParseArg(t *testing.T) {

if testCase.expectedErr {
assert.Error(t, err)
} else {
assert.NoError(t, err)
return
}
assert.Equal(t, testCase.config.FileName.String, config.FileName.String)
assert.Equal(t, testCase.config.SaveInterval.String(), config.SaveInterval.String())

require.NoError(t, err)
assert.Equal(t, testCase.config, config)

var entries []string
for _, v := range hook.AllEntries() {
19 changes: 0 additions & 19 deletions output/csv/consts.go

This file was deleted.

13 changes: 9 additions & 4 deletions output/csv/output.go
Original file line number Diff line number Diff line change
@@ -86,6 +86,10 @@ func newOutput(params output.Params) (*Output, error) {
if err != nil {
return nil, err
}
timeFormat, err := TimeFormatString(config.TimeFormat.String)
if err != nil {
return nil, err
}

saveInterval := config.SaveInterval.TimeDuration()
fname := config.FileName.String
@@ -99,7 +103,7 @@ func newOutput(params output.Params) (*Output, error) {
csvWriter: stdoutWriter,
row: make([]string, 3+len(resTags)+1),
saveInterval: saveInterval,
timeFormat: config.TimeFormat,
timeFormat: timeFormat,
closeFn: func() error { return nil },
logger: logger,
params: params,
@@ -117,7 +121,7 @@ func newOutput(params output.Params) (*Output, error) {
ignoredTags: ignoredTags,
row: make([]string, 3+len(resTags)+1),
saveInterval: saveInterval,
timeFormat: config.TimeFormat,
timeFormat: timeFormat,
logger: logger,
params: params,
}
@@ -209,9 +213,10 @@ func SampleToRow(sample *metrics.Sample, resTags []string, ignoredTags []string,
) []string {
row[0] = sample.Metric.Name

if timeFormat == RFC3399 {
switch timeFormat {
case TimeFormatRFC3339:
row[1] = sample.Time.Format(time.RFC3339)
} else {
case TimeFormatUnix:
row[1] = strconv.FormatInt(sample.Time.Unix(), 10)
}

67 changes: 39 additions & 28 deletions output/csv/output_test.go
Original file line number Diff line number Diff line change
@@ -89,7 +89,7 @@ func TestSampleToRow(t *testing.T) {
},
resTags: []string{"tag1"},
ignoredTags: []string{"tag2"},
timeFormat: "",
timeFormat: "unix",
},
{
testname: "Two res tags, three extra tags",
@@ -107,10 +107,10 @@ func TestSampleToRow(t *testing.T) {
},
resTags: []string{"tag1", "tag2"},
ignoredTags: []string{},
timeFormat: "",
timeFormat: "unix",
},
{
testname: "Two res tags, two ignored, with RFC3399 timestamp",
testname: "Two res tags, two ignored, with RFC3339 timestamp",
sample: &metrics.Sample{
Time: time.Unix(1562324644, 0),
Metric: testMetric,
@@ -126,7 +126,7 @@ func TestSampleToRow(t *testing.T) {
},
resTags: []string{"tag1", "tag3"},
ignoredTags: []string{"tag4", "tag6"},
timeFormat: "rfc3399",
timeFormat: "rfc3339",
},
}

@@ -162,7 +162,7 @@ func TestSampleToRow(t *testing.T) {
{
baseRow: []string{
"my_metric",
"2019-07-05T11:04:04Z",
time.Unix(1562324644, 0).Format(time.RFC3339),
"1.000000",
"val1",
"val3",
@@ -177,7 +177,8 @@ func TestSampleToRow(t *testing.T) {
for i := range testData {
testname, sample := testData[i].testname, testData[i].sample
resTags, ignoredTags := testData[i].resTags, testData[i].ignoredTags
timeFormat := TimeFormat(testData[i].timeFormat)
timeFormat, err := TimeFormatString(testData[i].timeFormat)
require.NoError(t, err)
expectedRow := expected[i]

t.Run(testname, func(t *testing.T) {
@@ -317,34 +318,44 @@ func TestRun(t *testing.T) {
},
fileName: "test",
fileReaderFunc: readUnCompressedFile,
timeFormat: "rfc3399",
outputContent: "metric_name,timestamp,metric_value,check,error,extra_tags\n" + "my_metric,2019-07-05T11:04:04Z,1.000000,val1,val3,url=val2\n" + "my_metric,2019-07-05T11:04:04Z,1.000000,val1,val3,name=val4&url=val2\n",
timeFormat: "rfc3339",
outputContent: "metric_name,timestamp,metric_value,check,error,extra_tags\n" +
"my_metric," + time.Unix(1562324644, 0).Format(time.RFC3339) + ",1.000000,val1,val3,url=val2\n" +
"my_metric," + time.Unix(1562324644, 0).Format(time.RFC3339) + ",1.000000,val1,val3,name=val4&url=val2\n",
},
}

for _, data := range testData {
mem := afero.NewMemMapFs()
output, err := newOutput(output.Params{
Logger: testutils.NewLogger(t),
FS: mem,
ConfigArgument: data.fileName,
ScriptOptions: lib.Options{
SystemTags: metrics.NewSystemTagSet(metrics.TagError | metrics.TagCheck),
},
})

output.timeFormat = TimeFormat(data.timeFormat)
for i, data := range testData {
name := fmt.Sprint(i)
data := data
t.Run(name, func(t *testing.T) {
t.Parallel()
mem := afero.NewMemMapFs()
env := make(map[string]string)
if data.timeFormat != "" {
env["K6_CSV_TIME_FORMAT"] = data.timeFormat
}

require.NoError(t, err)
require.NotNil(t, output)
output, err := newOutput(output.Params{
Logger: testutils.NewLogger(t),
FS: mem,
Environment: env,
ConfigArgument: data.fileName,
ScriptOptions: lib.Options{
SystemTags: metrics.NewSystemTagSet(metrics.TagError | metrics.TagCheck),
},
})
require.NoError(t, err)
require.NotNil(t, output)

require.NoError(t, output.Start())
output.AddMetricSamples(data.samples)
time.Sleep(1 * time.Second)
require.NoError(t, output.Stop())
require.NoError(t, output.Start())
output.AddMetricSamples(data.samples)
time.Sleep(1 * time.Second)
require.NoError(t, output.Stop())

finalOutput := data.fileReaderFunc(data.fileName, mem)
assert.Equal(t, data.outputContent, sortExtraTagsForTest(t, finalOutput))
finalOutput := data.fileReaderFunc(data.fileName, mem)
assert.Equal(t, data.outputContent, sortExtraTagsForTest(t, finalOutput))
})
}
}

Loading

0 comments on commit 4bb7b1e

Please sign in to comment.