From 7e7b0db5e5b42858cd50379a07c179266aa2edfb Mon Sep 17 00:00:00 2001 From: guscarreon Date: Mon, 9 May 2022 13:12:31 -0400 Subject: [PATCH] Put endpoint end-to-end JSON tests (#100) --- backends/aerospike_test.go | 17 +- backends/config/config_test.go | 19 +- backends/decorators/metrics.go | 6 +- backends/decorators/metrics_test.go | 199 ++++-- config/config_test.go | 80 +-- endpoints/get_test.go | 69 +- endpoints/put.go | 2 +- endpoints/put_test.go | 623 +++++++++++++----- .../allowed/key-field-included.json | 30 + .../allowed/key-field-missing.json | 28 + .../not-allowed/key-field-included.json | 26 + .../puts-max-num-values.json | 17 + .../invalid-types/type-missing.json | 26 + .../invalid-types/type-unknown.json | 27 + .../invalid-value/value-greater-than-max.json | 34 + .../invalid-value/value-missing.json | 26 + .../multiple-elements-to-store.json | 35 + .../valid-whole/no-elements-to-store.json | 13 + .../valid-whole/single-element-to-store.json | 25 + .../put-endpoint/valid-whole/ttl-missing.json | 24 + .../valid-whole/ttl-more-than-max.json | 28 + .../valid-whole/valid-type-json.json | 25 + .../valid-whole/valid-type-xml.json | 25 + metrics/metricstest/metrics_mock.go | 233 +++++-- server/listener_test.go | 106 +-- 25 files changed, 1322 insertions(+), 421 deletions(-) create mode 100644 endpoints/sample-requests/put-endpoint/custom-keys/allowed/key-field-included.json create mode 100644 endpoints/sample-requests/put-endpoint/custom-keys/allowed/key-field-missing.json create mode 100644 endpoints/sample-requests/put-endpoint/custom-keys/not-allowed/key-field-included.json create mode 100644 endpoints/sample-requests/put-endpoint/invalid-number-of-elements/puts-max-num-values.json create mode 100644 endpoints/sample-requests/put-endpoint/invalid-types/type-missing.json create mode 100644 endpoints/sample-requests/put-endpoint/invalid-types/type-unknown.json create mode 100644 endpoints/sample-requests/put-endpoint/invalid-value/value-greater-than-max.json create mode 100644 endpoints/sample-requests/put-endpoint/invalid-value/value-missing.json create mode 100644 endpoints/sample-requests/put-endpoint/valid-whole/multiple-elements-to-store.json create mode 100644 endpoints/sample-requests/put-endpoint/valid-whole/no-elements-to-store.json create mode 100644 endpoints/sample-requests/put-endpoint/valid-whole/single-element-to-store.json create mode 100644 endpoints/sample-requests/put-endpoint/valid-whole/ttl-missing.json create mode 100644 endpoints/sample-requests/put-endpoint/valid-whole/ttl-more-than-max.json create mode 100644 endpoints/sample-requests/put-endpoint/valid-whole/valid-type-json.json create mode 100644 endpoints/sample-requests/put-endpoint/valid-whole/valid-type-xml.json diff --git a/backends/aerospike_test.go b/backends/aerospike_test.go index 66477ea2..f9b1f287 100644 --- a/backends/aerospike_test.go +++ b/backends/aerospike_test.go @@ -8,6 +8,7 @@ import ( as "github.com/aerospike/aerospike-client-go" as_types "github.com/aerospike/aerospike-client-go/types" "github.com/prebid/prebid-cache/config" + "github.com/prebid/prebid-cache/metrics" "github.com/prebid/prebid-cache/metrics/metricstest" "github.com/prebid/prebid-cache/utils" "github.com/sirupsen/logrus" @@ -147,8 +148,14 @@ func TestClassifyAerospikeError(t *testing.T) { } func TestAerospikeClientGet(t *testing.T) { + mockMetrics := metricstest.CreateMockMetrics() + m := &metrics.Metrics{ + MetricEngines: []metrics.CacheMetrics{ + &mockMetrics, + }, + } aerospikeBackend := &AerospikeBackend{ - metrics: metricstest.CreateMockMetrics(), + metrics: m, } testCases := []struct { @@ -220,8 +227,14 @@ func TestAerospikeClientGet(t *testing.T) { } func TestClientPut(t *testing.T) { + mockMetrics := metricstest.CreateMockMetrics() + m := &metrics.Metrics{ + MetricEngines: []metrics.CacheMetrics{ + &mockMetrics, + }, + } aerospikeBackend := &AerospikeBackend{ - metrics: metricstest.CreateMockMetrics(), + metrics: m, } testCases := []struct { diff --git a/backends/config/config_test.go b/backends/config/config_test.go index 2c404b30..bab2f919 100644 --- a/backends/config/config_test.go +++ b/backends/config/config_test.go @@ -7,6 +7,7 @@ import ( "github.com/prebid/prebid-cache/backends" "github.com/prebid/prebid-cache/compression" "github.com/prebid/prebid-cache/config" + "github.com/prebid/prebid-cache/metrics" "github.com/prebid/prebid-cache/metrics/metricstest" "github.com/prebid/prebid-cache/utils" @@ -90,8 +91,15 @@ func TestNewMemoryOrMemcacheBackend(t *testing.T) { } for _, tc := range testCases { + mockMetrics := metricstest.CreateMockMetrics() + m := &metrics.Metrics{ + MetricEngines: []metrics.CacheMetrics{ + &mockMetrics, + }, + } + // run - actualBackend := newBaseBackend(tc.inConfig, metricstest.CreateMockMetrics()) + actualBackend := newBaseBackend(tc.inConfig, m) // assertions assert.IsType(t, tc.expectedBackend, actualBackend, tc.desc) @@ -150,9 +158,16 @@ func TestNewBaseBackend(t *testing.T) { } for _, tc := range testCases { + mockMetrics := metricstest.CreateMockMetrics() + m := &metrics.Metrics{ + MetricEngines: []metrics.CacheMetrics{ + &mockMetrics, + }, + } + // run and assert it panics panicTestFunction := func() { - newBaseBackend(tc.inConfig, metricstest.CreateMockMetrics()) + newBaseBackend(tc.inConfig, m) } assert.Panics(t, panicTestFunction, "%s backend initialized in this test should error and panic.", tc.desc) diff --git a/backends/decorators/metrics.go b/backends/decorators/metrics.go index 9021bce5..82a3e23f 100644 --- a/backends/decorators/metrics.go +++ b/backends/decorators/metrics.go @@ -2,6 +2,7 @@ package decorators import ( "context" + "fmt" "strings" "time" @@ -45,9 +46,10 @@ func (b *backendWithMetrics) Put(ctx context.Context, key string, value string, } else if strings.HasPrefix(value, utils.JSON_PREFIX) { b.metrics.RecordPutBackendJson() } else { - b.metrics.RecordPutBackendInvalid() + b.metrics.RecordPutBackendInvalid() // Never gets called here. Unreachable } - b.metrics.RecordPutBackendTTLSeconds(time.Duration(ttlSeconds) * time.Second) + ttl, _ := time.ParseDuration(fmt.Sprintf("%ds", ttlSeconds)) + b.metrics.RecordPutBackendTTLSeconds(ttl) start := time.Now() err := b.delegate.Put(ctx, key, value, ttlSeconds) diff --git a/backends/decorators/metrics_test.go b/backends/decorators/metrics_test.go index 87233228..c9f1197a 100644 --- a/backends/decorators/metrics_test.go +++ b/backends/decorators/metrics_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/prebid/prebid-cache/backends" + "github.com/prebid/prebid-cache/metrics" "github.com/prebid/prebid-cache/metrics/metricstest" "github.com/prebid/prebid-cache/utils" "github.com/stretchr/testify/assert" @@ -23,124 +24,200 @@ func (b *failedBackend) Put(ctx context.Context, key string, value string, ttlSe return b.returnError } -func TestGetSuccessMetrics(t *testing.T) { +func TestGetBackendMetrics(t *testing.T) { + // Expected values + expectedMetrics := []string{ + "RecordGetBackendTotal", + "RecordGetBackendDuration", + } + + // Test setup + mockMetrics := metricstest.CreateMockMetrics() + m := &metrics.Metrics{ + MetricEngines: []metrics.CacheMetrics{ + &mockMetrics, + }, + } - m := metricstest.CreateMockMetrics() rawBackend := backends.NewMemoryBackend() rawBackend.Put(context.Background(), "foo", "xml", 0) - backend := LogMetrics(rawBackend, m) - backend.Get(context.Background(), "foo") + backendWithMetrics := LogMetrics(rawBackend, m) + + // Run test + backendWithMetrics.Get(context.Background(), "foo") - assert.Equalf(t, int64(1), metricstest.MockCounters["gets.backends.request.total"], "Successful backend request been accounted for in the total get backend request count") - assert.Greater(t, metricstest.MockHistograms["gets.backends.duration"], 0.00, "Successful put request duration should be greater than zero") + // Assert + metricstest.AssertMetrics(t, expectedMetrics, mockMetrics) } -func TestGetErrorMetrics(t *testing.T) { +func TestGetBackendErrorMetrics(t *testing.T) { type testCase struct { - desc string - inMetricName string - outError error + desc string + expectedMetrics []string + expectedError error } testGroups := []struct { - groupName string - tests []testCase + name string + tests []testCase }{ { - "Any other error", - []testCase{ - { - "Failed get backend request should be accounted under the error label", - "gets.backends.request.error", - errors.New("other backend error"), - }, - }, - }, - { - "Special errors", + "Special backend storage GET errors", []testCase{ { "Failed get backend request should be accounted as a key not found error", - "gets.backend_error.key_not_found", + []string{ + "RecordGetBackendError", + "RecordKeyNotFoundError", + "RecordGetBackendTotal", + }, utils.NewPBCError(utils.KEY_NOT_FOUND), }, { "Failed get backend request should be accounted as a missing key (uuid) error", - "gets.backend_error.missing_key", + []string{ + "RecordGetBackendError", + "RecordMissingKeyError", + "RecordGetBackendTotal", + }, utils.NewPBCError(utils.MISSING_KEY), }, }, }, + { + "Other backend error", + []testCase{ + { + "Failed get backend request should be accounted under the error label", + []string{ + "RecordGetBackendError", + "RecordGetBackendTotal", + }, + errors.New("some backend storage service error"), + }, + }, + }, } - // Create metrics - m := metricstest.CreateMockMetrics() - - errsTotal := 0 for _, group := range testGroups { for _, test := range group.tests { - // Create backend with a mock storage that will fail and assign metrics - backend := LogMetrics(&failedBackend{test.outError}, m) + // Fresh mock metrics + mockMetrics := metricstest.CreateMockMetrics() + m := &metrics.Metrics{ + MetricEngines: []metrics.CacheMetrics{ + &mockMetrics, + }, + } + // Create backend with a mock storage that will fail and record metrics + backend := LogMetrics(&failedBackend{test.expectedError}, m) // Run test - backend.Get(context.Background(), "foo") - errsTotal++ + retrievedValue, err := backend.Get(context.Background(), "foo") - // Assert - assert.Equal(t, int64(1), metricstest.MockCounters[test.inMetricName], test.desc) - assert.Equal(t, int64(errsTotal), metricstest.MockCounters["gets.backends.request.error"], test.desc) - assert.Equal(t, int64(errsTotal), metricstest.MockCounters["gets.backends.request.total"], test.desc) + // Assertions + assert.Empty(t, retrievedValue, "%s - %s", group.name, test.desc) + assert.Equal(t, test.expectedError, err, "%s - %s", group.name, test.desc) + metricstest.AssertMetrics(t, test.expectedMetrics, mockMetrics) } } } func TestPutSuccessMetrics(t *testing.T) { + // Expected values + expectedMetrics := []string{ + "RecordPutBackendDuration", + "RecordPutBackendXml", + "RecordPutBackendTTLSeconds", + "RecordPutBackendSize", + } - m := metricstest.CreateMockMetrics() + // Test setup + mockMetrics := metricstest.CreateMockMetrics() + m := &metrics.Metrics{ + MetricEngines: []metrics.CacheMetrics{ + &mockMetrics, + }, + } backend := LogMetrics(backends.NewMemoryBackend(), m) - backend.Put(context.Background(), "foo", "xml", 0) - assert.Greater(t, metricstest.MockHistograms["puts.backends.request_duration"], 0.00, "Successful put request duration should be greater than zero") - assert.Equal(t, int64(1), metricstest.MockCounters["puts.backends.xml"], "An xml request should have been logged.") - assert.Equal(t, int64(0), metricstest.MockCounters["puts.backends.defines_ttl"], "An event for TTL defined shouldn't be logged if the TTL was 0") + // Run test + backend.Put(context.Background(), "foo", "xml", 60) + + // Assert + metricstest.AssertMetrics(t, expectedMetrics, mockMetrics) } func TestPutErrorMetrics(t *testing.T) { + // Expected values + expectedMetrics := []string{ + "RecordPutBackendError", + "RecordPutBackendXml", + "RecordPutBackendSize", + "RecordPutBackendTTLSeconds", + } - m := metricstest.CreateMockMetrics() + // Test setup + mockMetrics := metricstest.CreateMockMetrics() + m := &metrics.Metrics{ + MetricEngines: []metrics.CacheMetrics{ + &mockMetrics, + }, + } backend := LogMetrics(&failedBackend{errors.New("Failure")}, m) + + // Run test backend.Put(context.Background(), "foo", "xml", 0) - assert.Equal(t, int64(1), metricstest.MockCounters["puts.backends.xml"], "An xml request should have been logged.") - assert.Equal(t, int64(1), metricstest.MockCounters["puts.backends.request.error"], "Failed get backend request should have been accounted under the error label") + // Assert + metricstest.AssertMetrics(t, expectedMetrics, mockMetrics) } func TestJsonPayloadMetrics(t *testing.T) { + // Expected values + expectedMetrics := []string{ + "RecordPutBackendJson", + "RecordPutBackendSize", + "RecordPutBackendTTLSeconds", + "RecordPutBackendDuration", + } - m := metricstest.CreateMockMetrics() + // Test setup + mockMetrics := metricstest.CreateMockMetrics() + m := &metrics.Metrics{ + MetricEngines: []metrics.CacheMetrics{ + &mockMetrics, + }, + } backend := LogMetrics(backends.NewMemoryBackend(), m) - backend.Put(context.Background(), "foo", "json{\"key\":\"value\"", 0) - backend.Get(context.Background(), "foo") - - assert.Equal(t, int64(1), metricstest.MockCounters["puts.backends.json"], "A json request should have been logged.") -} -func TestPutSizeSampling(t *testing.T) { - - m := metricstest.CreateMockMetrics() - payload := `json{"key":"value"}` - backend := LogMetrics(backends.NewMemoryBackend(), m) - backend.Put(context.Background(), "foo", payload, 0) + // Run test + backend.Put(context.Background(), "foo", "json{\"key\":\"value\"", 0) - assert.Greater(t, metricstest.MockHistograms["puts.backends.request_size_bytes"], 0.00, "Successful put request size should be greater than zero") + // Assert + metricstest.AssertMetrics(t, expectedMetrics, mockMetrics) } func TestInvalidPayloadMetrics(t *testing.T) { + // Expected values + expectedMetrics := []string{ + "RecordPutBackendInvalid", + "RecordPutBackendSize", + "RecordPutBackendTTLSeconds", + "RecordPutBackendDuration", + } - m := metricstest.CreateMockMetrics() + // Test setup + mockMetrics := metricstest.CreateMockMetrics() + m := &metrics.Metrics{ + MetricEngines: []metrics.CacheMetrics{ + &mockMetrics, + }, + } backend := LogMetrics(backends.NewMemoryBackend(), m) + + // Run test backend.Put(context.Background(), "foo", "bar", 0) - backend.Get(context.Background(), "foo") - assert.Equal(t, int64(1), metricstest.MockCounters["puts.backends.invalid_format"], "A Put request of invalid format should have been logged.") + // Assert + metricstest.AssertMetrics(t, expectedMetrics, mockMetrics) } diff --git a/config/config_test.go b/config/config_test.go index de14a8c8..536f33c2 100644 --- a/config/config_test.go +++ b/config/config_test.go @@ -10,7 +10,7 @@ import ( "github.com/prebid/prebid-cache/utils" "github.com/sirupsen/logrus" - "github.com/sirupsen/logrus/hooks/test" + testLogrus "github.com/sirupsen/logrus/hooks/test" "github.com/spf13/viper" "github.com/stretchr/testify/assert" ) @@ -42,7 +42,7 @@ func TestEnvConfig(t *testing.T) { func TestLogValidateAndLog(t *testing.T) { // logrus entries will be recorded to this `hook` object so we can compare and assert them - hook := test.NewGlobal() + hook := testLogrus.NewGlobal() // Define object to run `validateAndLog()` on configLogObject := Log{ @@ -305,37 +305,37 @@ func TestCheckMetricsEnabled(t *testing.T) { } // logrus entries will be recorded to this `hook` object so we can compare and assert them - hook := test.NewGlobal() + hook := testLogrus.NewGlobal() //substitute logger exit function so execution doesn't get interrupted when log.Fatalf() call comes defer func() { logrus.StandardLogger().ExitFunc = nil }() var fatal bool logrus.StandardLogger().ExitFunc = func(int) { fatal = true } - for i, test := range testCases { + for i, tc := range testCases { // Reset the fatal flag to false every test fatal = false // Set test flags in metrics object - cfg.Type = test.metricType - cfg.Influx.Enabled = test.influxEnabled - cfg.Prometheus.Enabled = test.prometheusEnabled + cfg.Type = tc.metricType + cfg.Influx.Enabled = tc.influxEnabled + cfg.Prometheus.Enabled = tc.prometheusEnabled //run test cfg.validateAndLog() // Assert logrus expected entries - if assert.Equal(t, len(test.expectedLogInfo), len(hook.Entries), "Incorrect number of entries were logged to logrus in test %d: len(test.expectedLogInfo) = %d len(hook.Entries) = %d", i+1, len(test.expectedLogInfo), len(hook.Entries)) { - for j := 0; j < len(test.expectedLogInfo); j++ { - assert.Equal(t, test.expectedLogInfo[j].msg, hook.Entries[j].Message, "Test case %d log message differs", i+1) - assert.Equal(t, test.expectedLogInfo[j].lvl, hook.Entries[j].Level, "Test case %d log level differs", i+1) + if assert.Equal(t, len(tc.expectedLogInfo), len(hook.Entries), "Incorrect number of entries were logged to logrus in test %d: len(tc.expectedLogInfo) = %d len(hook.Entries) = %d", i+1, len(tc.expectedLogInfo), len(hook.Entries)) { + for j := 0; j < len(tc.expectedLogInfo); j++ { + assert.Equal(t, tc.expectedLogInfo[j].msg, hook.Entries[j].Message, "Test case %d log message differs", i+1) + assert.Equal(t, tc.expectedLogInfo[j].lvl, hook.Entries[j].Level, "Test case %d log level differs", i+1) } } else { return } // Assert log.Fatalf() was called or not - assert.Equal(t, test.expectedError, fatal, "Test case %d failed.", i+1) + assert.Equal(t, tc.expectedError, fatal, "Test case %d failed.", i+1) //Reset log after every test and assert successful reset hook.Reset() @@ -445,26 +445,26 @@ func TestEnabledFlagGetsModified(t *testing.T) { } // logrus entries will be recorded to this `hook` object so we can compare and assert them - hook := test.NewGlobal() + hook := testLogrus.NewGlobal() //substitute logger exit function so execution doesn't get interrupted when log.Fatalf() call comes defer func() { logrus.StandardLogger().ExitFunc = nil }() logrus.StandardLogger().ExitFunc = func(int) {} - for i, test := range testCases { + for i, tc := range testCases { // Reset Metrics object metricsCfg := Metrics{ - Type: test.in.metricType, + Type: tc.in.metricType, Influx: InfluxMetrics{ Host: "http://fakeurl.com", Database: "database-value", - Enabled: test.in.influxEnabled, + Enabled: tc.in.influxEnabled, }, Prometheus: PrometheusMetrics{ Port: 8080, Namespace: "prebid", Subsystem: "cache", - Enabled: test.in.prometheusEnabled, + Enabled: tc.in.prometheusEnabled, }, } @@ -472,8 +472,8 @@ func TestEnabledFlagGetsModified(t *testing.T) { metricsCfg.validateAndLog() // Assert `Enabled` flags value - assert.Equal(t, test.out.expectedInfluxEnabled, metricsCfg.Influx.Enabled, "Test case %d failed. `cfg.Influx.Enabled` carries wrong value.", i+1) - assert.Equal(t, test.out.expectedprometheusEnabled, metricsCfg.Prometheus.Enabled, "Test case %d failed. `cfg.Prometheus.Enabled` carries wrong value.", i+1) + assert.Equal(t, tc.out.expectedInfluxEnabled, metricsCfg.Influx.Enabled, "Test case %d failed. `cfg.Influx.Enabled` carries wrong value.", i+1) + assert.Equal(t, tc.out.expectedprometheusEnabled, metricsCfg.Prometheus.Enabled, "Test case %d failed. `cfg.Prometheus.Enabled` carries wrong value.", i+1) //Reset log after every test hook.Reset() @@ -483,7 +483,7 @@ func TestEnabledFlagGetsModified(t *testing.T) { func TestInfluxValidateAndLog(t *testing.T) { // logrus entries will be recorded to this `hook` object so we can compare and assert them - hook := test.NewGlobal() + hook := testLogrus.NewGlobal() type logComponents struct { msg string @@ -603,25 +603,25 @@ func TestInfluxValidateAndLog(t *testing.T) { var fatal bool logrus.StandardLogger().ExitFunc = func(int) { fatal = true } - for j, test := range testCases { + for j, tc := range testCases { // Reset the fatal flag to false every test fatal = false //run test - test.influxConfig.validateAndLog() + tc.influxConfig.validateAndLog() // Assert logrus expected entries - if assert.Equal(t, len(test.expectedLogInfo), len(hook.Entries), "Incorrect number of entries were logged to logrus in test %d: len(test.expectedLogInfo) = %d len(hook.Entries) = %d", j, len(test.expectedLogInfo), len(hook.Entries)) { - for i := 0; i < len(test.expectedLogInfo); i++ { - assert.Equal(t, test.expectedLogInfo[i].msg, hook.Entries[i].Message, "Test case %d failed", j) - assert.Equal(t, test.expectedLogInfo[i].lvl, hook.Entries[i].Level, "Test case %d failed", j) + if assert.Equal(t, len(tc.expectedLogInfo), len(hook.Entries), "Incorrect number of entries were logged to logrus in test %d: len(tc.expectedLogInfo) = %d len(hook.Entries) = %d", j, len(tc.expectedLogInfo), len(hook.Entries)) { + for i := 0; i < len(tc.expectedLogInfo); i++ { + assert.Equal(t, tc.expectedLogInfo[i].msg, hook.Entries[i].Message, "Test case %d failed", j) + assert.Equal(t, tc.expectedLogInfo[i].lvl, hook.Entries[i].Level, "Test case %d failed", j) } } else { return } // Assert log.Fatalf() was called or not - assert.Equal(t, test.expectError, fatal) + assert.Equal(t, tc.expectError, fatal) //Reset log after every test and assert successful reset hook.Reset() @@ -755,32 +755,32 @@ func TestPrometheusValidateAndLog(t *testing.T) { } // logrus entries will be recorded to this `hook` object so we can compare and assert them - hook := test.NewGlobal() + hook := testLogrus.NewGlobal() //substitute logger exit function so execution doesn't get interrupted defer func() { logrus.StandardLogger().ExitFunc = nil }() var fatal bool logrus.StandardLogger().ExitFunc = func(int) { fatal = true } - for j, test := range testCases { + for j, tc := range testCases { // Reset the fatal flag to false every test fatal = false //run test - test.prometheusConfig.validateAndLog() + tc.prometheusConfig.validateAndLog() // Assert logrus expected entries - if assert.Equal(t, len(test.expectedLogInfo), len(hook.Entries), "Incorrect number of entries were logged to logrus in test %d: len(test.expectedLogInfo) = %d len(hook.Entries) = %d", j, len(test.expectedLogInfo), len(hook.Entries)) { - for i := 0; i < len(test.expectedLogInfo); i++ { - assert.Equal(t, test.expectedLogInfo[i].msg, hook.Entries[i].Message) - assert.Equal(t, test.expectedLogInfo[i].lvl, hook.Entries[i].Level, "Expected Info entry in log") + if assert.Equal(t, len(tc.expectedLogInfo), len(hook.Entries), "Incorrect number of entries were logged to logrus in test %d: len(tc.expectedLogInfo) = %d len(hook.Entries) = %d", j, len(tc.expectedLogInfo), len(hook.Entries)) { + for i := 0; i < len(tc.expectedLogInfo); i++ { + assert.Equal(t, tc.expectedLogInfo[i].msg, hook.Entries[i].Message) + assert.Equal(t, tc.expectedLogInfo[i].lvl, hook.Entries[i].Level, "Expected Info entry in log") } } else { return } // Assert log.Fatalf() was called or not - assert.Equal(t, test.expectError, fatal) + assert.Equal(t, tc.expectError, fatal) //Reset log after every test and assert successful reset hook.Reset() @@ -791,7 +791,7 @@ func TestPrometheusValidateAndLog(t *testing.T) { func TestRequestLimitsValidateAndLog(t *testing.T) { // logrus entries will be recorded to this `hook` object so we can compare and assert them - hook := test.NewGlobal() + hook := testLogrus.NewGlobal() type logComponents struct { msg string @@ -895,7 +895,7 @@ func TestRequestLimitsValidateAndLog(t *testing.T) { func TestCompressionValidateAndLog(t *testing.T) { // logrus entries will be recorded to this `hook` object so we can compare and assert them - hook := test.NewGlobal() + hook := testLogrus.NewGlobal() type logComponents struct { msg string @@ -966,7 +966,7 @@ func TestCompressionValidateAndLog(t *testing.T) { func TestNewConfigFromFile(t *testing.T) { // logrus entries will be recorded to this `hook` object so we can compare and assert them - hook := test.NewGlobal() + hook := testLogrus.NewGlobal() type logComponents struct { msg string @@ -1044,7 +1044,7 @@ func TestNewConfigFromFile(t *testing.T) { func TestConfigurationValidateAndLog(t *testing.T) { // logrus entries will be recorded to this `hook` object so we can compare and assert them - hook := test.NewGlobal() + hook := testLogrus.NewGlobal() //substitute logger exit function so execution doesn't get interrupted defer func() { logrus.StandardLogger().ExitFunc = nil }() logrus.StandardLogger().ExitFunc = func(int) {} @@ -1100,7 +1100,7 @@ func TestPrometheusTimeoutDuration(t *testing.T) { func TestRoutesValidateAndLog(t *testing.T) { // logrus entries will be recorded to this `hook` object so we can compare and assert them - hook := test.NewGlobal() + hook := testLogrus.NewGlobal() type logComponents struct { msg string diff --git a/endpoints/get_test.go b/endpoints/get_test.go index 8172f268..021c5f08 100644 --- a/endpoints/get_test.go +++ b/endpoints/get_test.go @@ -6,6 +6,7 @@ import ( "github.com/julienschmidt/httprouter" "github.com/prebid/prebid-cache/backends" + "github.com/prebid/prebid-cache/metrics" "github.com/prebid/prebid-cache/metrics/metricstest" "github.com/sirupsen/logrus" "github.com/sirupsen/logrus/hooks/test" @@ -15,9 +16,15 @@ import ( func TestGetInvalidUUIDs(t *testing.T) { backend := backends.NewMemoryBackend() router := httprouter.New() - mockmetrics := metricstest.CreateMockMetrics() - router.GET("/cache", NewGetHandler(backend, mockmetrics, false)) + mockMetrics := metricstest.CreateMockMetrics() + m := &metrics.Metrics{ + MetricEngines: []metrics.CacheMetrics{ + &mockMetrics, + }, + } + + router.GET("/cache", NewGetHandler(backend, m, false)) getResults := doMockGet(t, router, "fdd9405b-ef2b-46da-a55a-2f526d338e16") if getResults.Code != http.StatusNotFound { @@ -41,17 +48,11 @@ func TestGetHandler(t *testing.T) { uuid string allowKeys bool } - type metricsRecords struct { - totalRequests int64 - badRequests int64 - requestErrs int64 - requestDur float64 - } type testOutput struct { responseCode int responseBody string logEntries []logEntry - metricsRecorded metricsRecords + expectedMetrics []string } testCases := []struct { @@ -74,9 +75,9 @@ func TestGetHandler(t *testing.T) { lvl: logrus.ErrorLevel, }, }, - metricsRecorded: metricsRecords{ - totalRequests: int64(1), - badRequests: int64(1), + expectedMetrics: []string{ + "RecordGetTotal", + "RecordGetBadRequest", }, }, }, @@ -95,9 +96,9 @@ func TestGetHandler(t *testing.T) { lvl: logrus.ErrorLevel, }, }, - metricsRecorded: metricsRecords{ - totalRequests: int64(1), - badRequests: int64(1), + expectedMetrics: []string{ + "RecordGetTotal", + "RecordGetBadRequest", }, }, }, @@ -111,9 +112,9 @@ func TestGetHandler(t *testing.T) { responseCode: http.StatusOK, responseBody: `{"field":"value"}`, logEntries: []logEntry{}, - metricsRecorded: metricsRecords{ - totalRequests: int64(1), - requestDur: 1.00, + expectedMetrics: []string{ + "RecordGetTotal", + "RecordGetDuration", }, }, }, @@ -129,9 +130,9 @@ func TestGetHandler(t *testing.T) { lvl: logrus.DebugLevel, }, }, - metricsRecorded: metricsRecords{ - totalRequests: int64(1), - badRequests: int64(1), + expectedMetrics: []string{ + "RecordGetTotal", + "RecordGetBadRequest", }, }, }, @@ -147,9 +148,9 @@ func TestGetHandler(t *testing.T) { lvl: logrus.ErrorLevel, }, }, - metricsRecorded: metricsRecords{ - totalRequests: int64(1), - requestErrs: int64(1), + expectedMetrics: []string{ + "RecordGetTotal", + "RecordGetError", }, }, }, @@ -160,9 +161,9 @@ func TestGetHandler(t *testing.T) { responseCode: http.StatusOK, responseBody: "xml data here", logEntries: []logEntry{}, - metricsRecorded: metricsRecords{ - totalRequests: int64(1), - requestDur: 1.00, + expectedMetrics: []string{ + "RecordGetTotal", + "RecordGetDuration", }, }, }, @@ -185,8 +186,13 @@ func TestGetHandler(t *testing.T) { // Set up test object backend := newMockBackend() router := httprouter.New() - mockmetrics := metricstest.CreateMockMetrics() - router.GET("/cache", NewGetHandler(backend, mockmetrics, test.in.allowKeys)) + mockMetrics := metricstest.CreateMockMetrics() + m := &metrics.Metrics{ + MetricEngines: []metrics.CacheMetrics{ + &mockMetrics, + }, + } + router.GET("/cache", NewGetHandler(backend, m, test.in.allowKeys)) // Run test getResults := doMockGet(t, router, test.in.uuid) @@ -206,10 +212,7 @@ func TestGetHandler(t *testing.T) { } // Assert recorded metrics - assert.Equal(t, test.out.metricsRecorded.totalRequests, metricstest.MockCounters["gets.current_url.request.total"], "%s - handle function should record every incomming GET request", test.desc) - assert.Equal(t, test.out.metricsRecorded.badRequests, metricstest.MockCounters["gets.current_url.request.bad_request"], "%s - Bad request wasn't recorded", test.desc) - assert.Equal(t, test.out.metricsRecorded.requestErrs, metricstest.MockCounters["gets.current_url.request.error"], "%s - WriteGetResponse error should have been recorded", test.desc) - assert.Equal(t, test.out.metricsRecorded.requestDur, metricstest.MockHistograms["gets.current_url.duration"], "%s - Successful GET request should have recorded duration", test.desc) + metricstest.AssertMetrics(t, test.out.expectedMetrics, mockMetrics) // Reset log hook.Reset() diff --git a/endpoints/put.go b/endpoints/put.go index 0a9a7934..e4258109 100644 --- a/endpoints/put.go +++ b/endpoints/put.go @@ -138,7 +138,7 @@ func parsePutObject(p putObject) (string, error) { } else if p.Type == utils.JSON_PREFIX { toCache = p.Type + string(p.Value) } else { - return "", utils.NewPBCError(utils.UNSUPPORTED_DATA_TO_STORE, fmt.Sprintf("Type must be one of [\"json\", \"xml\"]. Found %v", p.Type)) + return "", utils.NewPBCError(utils.UNSUPPORTED_DATA_TO_STORE, fmt.Sprintf("Type must be one of [\"json\", \"xml\"]. Found '%s'", p.Type)) } return toCache, nil diff --git a/endpoints/put_test.go b/endpoints/put_test.go index 3232712a..43d46356 100644 --- a/endpoints/put_test.go +++ b/endpoints/put_test.go @@ -6,6 +6,7 @@ import ( "encoding/json" "errors" "fmt" + "io/ioutil" "net/http" "net/http/httptest" "regexp" @@ -16,14 +17,249 @@ import ( "github.com/julienschmidt/httprouter" "github.com/prebid/prebid-cache/backends" + backendConfig "github.com/prebid/prebid-cache/backends/config" + "github.com/prebid/prebid-cache/backends/decorators" backendDecorators "github.com/prebid/prebid-cache/backends/decorators" + "github.com/prebid/prebid-cache/config" + "github.com/prebid/prebid-cache/metrics" "github.com/prebid/prebid-cache/metrics/metricstest" "github.com/prebid/prebid-cache/utils" + "github.com/sirupsen/logrus" + testLogrus "github.com/sirupsen/logrus/hooks/test" + "github.com/spf13/viper" "github.com/stretchr/testify/assert" "github.com/stretchr/testify/mock" "github.com/stretchr/testify/require" ) +func TestPutJsonTests(t *testing.T) { + testGroups := []struct { + desc string + expectError bool + tests []string + }{ + { + desc: "Valid put requests. Expect 200 response", + expectError: false, + tests: []string{ + "sample-requests/put-endpoint/valid-whole/single-element-to-store.json", + "sample-requests/put-endpoint/valid-whole/no-elements-to-store.json", + "sample-requests/put-endpoint/valid-whole/multiple-elements-to-store.json", + "sample-requests/put-endpoint/valid-whole/valid-type-json.json", + "sample-requests/put-endpoint/valid-whole/valid-type-xml.json", + "sample-requests/put-endpoint/valid-whole/ttl-more-than-max.json", + "sample-requests/put-endpoint/valid-whole/ttl-missing.json", + }, + }, + { + desc: "Request tries to store more elements than the max allowed. Return error", + expectError: true, + tests: []string{ + "sample-requests/put-endpoint/invalid-number-of-elements/puts-max-num-values.json", + }, + }, + { + desc: "Invalid 'type' field values, expect error", + expectError: true, + tests: []string{ + "sample-requests/put-endpoint/invalid-types/type-missing.json", + "sample-requests/put-endpoint/invalid-types/type-unknown.json", + }, + }, + { + desc: "invalid 'value' field values, expect error", + expectError: true, + tests: []string{ + "sample-requests/put-endpoint/invalid-value/value-missing.json", + "sample-requests/put-endpoint/invalid-value/value-greater-than-max.json", + }, + }, + { + desc: "Valid when storing under custom keys is allowed, expect 200 responses", + expectError: false, + tests: []string{ + "sample-requests/put-endpoint/custom-keys/allowed/key-field-included.json", + "sample-requests/put-endpoint/custom-keys/allowed/key-field-missing.json", + }, + }, + { + desc: "Valid when storing under custom keys is not allowed, expect 200 responses", + expectError: false, + tests: []string{ + "sample-requests/put-endpoint/custom-keys/not-allowed/key-field-included.json", + }, + }, + } + + // logrus entries will be recorded to this `hook` object so we can compare and assert them + hook := testLogrus.NewGlobal() + + //substitute logger exit function so execution doesn't get interrupted when log.Fatalf() call comes + defer func() { logrus.StandardLogger().ExitFunc = nil }() + logrus.StandardLogger().ExitFunc = func(int) {} + + for _, group := range testGroups { + for _, testFile := range group.tests { + // TEST SETUP + // Read file + testInfo, err := parseTestInfo(testFile) + if !assert.NoError(t, err, "%v", err) { + continue + } + + // Read test config + v := buildViperConfig(testInfo) + cfg := config.Configuration{} + err = v.Unmarshal(&cfg) + if !assert.NoError(t, err, "Viper could not parse configuration from test file: %s. Error:%s\n", testFile, err) { + continue + } + + // Instantiate memory backend, request, router, recorder + mockMetrics := metricstest.CreateMockMetrics() + m := &metrics.Metrics{ + MetricEngines: []metrics.CacheMetrics{ + &mockMetrics, + }, + } + + backend := backendConfig.NewBackend(cfg, m) + router := httprouter.New() + router.POST("/cache", NewPutHandler(backend, m, testInfo.ServerConfig.MaxNumValues, testInfo.ServerConfig.AllowSettingKeys)) + request, err := http.NewRequest("POST", "/cache", strings.NewReader(string(testInfo.PutRequest))) + assert.NoError(t, err, "Failed to create a POST request. Test file: %s Error: %v", testFile, err) + rr := httptest.NewRecorder() + + // RUN TEST + router.ServeHTTP(rr, request) + + // DO ASSERTIONS + // If error is expected, assert error message and non-200 status code + if group.expectError { + // Given that Prebid Cache still doesn't provide error details in an "errors" field describing the particular issues + // of each element that could not be stored, compare the entire response body that will contain the error message of + // the element that could not be stored. + assert.NotEqual(t, http.StatusOK, rr.Code, "Test %s failed. Expected error status code.", testFile) + assert.Equal(t, testInfo.ExpectedError, rr.Body.String(), "Error message differs from expected. Test file: %s", testFile) + } else { + // Given that no error is expected, assert a 200 status code was returned + if !assert.Equal(t, http.StatusOK, rr.Code, "Test %s failed. StatusCode = %d. Returned error: %s", testFile, rr.Code, rr.Body.String()) { + continue + } + + // Assert we returned the exact same elements in the 'Responses' array than in the request 'Puts' array + actualPutResponse := PutResponse{} + err = json.Unmarshal(rr.Body.Bytes(), &actualPutResponse) + if !assert.NoError(t, err, "Could not unmarshal %s. Test file: %s. Error:%s\n", rr.Body.String(), testFile, err) { + continue + } + assert.Len(t, actualPutResponse.Responses, len(testInfo.ExpectedResponse.Responses), "Actual response elements differ with expected. Test file: %s", testFile) + + // If custom keys are allowed, assert they are found in the actualPutResponse.Responses array + if testInfo.ServerConfig.AllowSettingKeys { + customKeyIndexes := []int{} + + // Unmarshal test request to extract custom keys + put := &putRequest{ + Puts: make([]putObject, 0), + } + err = json.Unmarshal(testInfo.PutRequest, put) + if !assert.NoError(t, err, "Could not put request %s. Test file: %s. Error:%s\n", testInfo.PutRequest, testFile, err) { + continue + } + for i, testInputElem := range put.Puts { + if len(testInputElem.Key) > 0 { + customKeyIndexes = append(customKeyIndexes, i) + } + } + + // Custom keys values must match and their position in the `actualPutResponse.Responses` array must be the exact same as they came in + // the incoming request + for _, index := range customKeyIndexes { + assert.Equal(t, testInfo.ExpectedResponse.Responses[index].UUID, actualPutResponse.Responses[index].UUID, "Custom key differs from expected in position %d. Test file: %s", index, testFile) + } + } + } + + // Assert logrus expected entries + assertLogEntries(t, testInfo.ExpectedLogEntries, hook.Entries, testFile) + + // Reset log after every test and assert successful reset + hook.Reset() + assert.Nil(t, hook.LastEntry()) + + // assert the put call above logged the expected metrics + metricstest.AssertMetrics(t, testInfo.ExpectedMetrics, mockMetrics) + } + } +} + +type testData struct { + ServerConfig testConfig `json:"serverConfig"` + PutRequest json.RawMessage `json:"putRequest"` + ExpectedResponse PutResponse `json:"expectedResponse"` + ExpectedLogEntries []logEntry `json:"expectedLogEntries"` + ExpectedError string `json:"expectedErrorMessage"` + ExpectedMetrics []string `json:"expectedMetrics"` +} + +type logEntry struct { + Message string `json:"message"` + Level uint32 `json:"level"` +} + +type testConfig struct { + AllowSettingKeys bool `json:"allow_setting_keys"` + MaxSizeBytes int `json:"max_size_bytes"` + MaxNumValues int `json:"max_num_values"` + MaxTTLSeconds int `json:"max_ttl_seconds"` +} + +// Remove this function in the future and make it part of the mock metrics to self-assert if possible. +func parseTestInfo(testFile string) (*testData, error) { + var jsonTest []byte + var err error + if jsonTest, err = ioutil.ReadFile(testFile); err != nil { + return nil, fmt.Errorf("Could not read test file: %s. Error: %v \n", testFile, err) + } + + testInfo := &testData{} + if err = json.Unmarshal(jsonTest, testInfo); err != nil { + return nil, fmt.Errorf("Could not unmarshal test file: %s. Error:%s\n", testFile, err) + } + return testInfo, nil +} + +func buildViperConfig(testInfo *testData) *viper.Viper { + v := viper.New() + v.SetDefault("backend.type", "memory") + v.SetDefault("compression.type", "none") + v.SetDefault("request_limits.allow_setting_keys", testInfo.ServerConfig.AllowSettingKeys) + if testInfo.ServerConfig.MaxSizeBytes == 0 { + testInfo.ServerConfig.MaxSizeBytes = 50 + } + v.SetDefault("request_limits.max_size_bytes", testInfo.ServerConfig.MaxSizeBytes) + + if testInfo.ServerConfig.MaxNumValues == 0 { + testInfo.ServerConfig.MaxNumValues = 1 + } + v.SetDefault("request_limits.max_num_values", testInfo.ServerConfig.MaxNumValues) + v.SetDefault("request_limits.max_ttl_seconds", testInfo.ServerConfig.MaxTTLSeconds) + return v +} + +// assertLogEntries asserts logrus entries with expectedLogEntries. It is a test helper function that will make a unit test fail if +// expected values are not found +func assertLogEntries(t *testing.T, expectedLogEntries []logEntry, actualLogEntries []logrus.Entry, testFile string) { + t.Helper() + + assert.Equal(t, len(expectedLogEntries), len(actualLogEntries), "Incorrect number of entries were logged to logrus in test %s: len(expectedLogEntries) = %d, len(actualLogEntries) = %d", testFile, len(expectedLogEntries), len(actualLogEntries)) + for i := 0; i < len(actualLogEntries); i++ { + assert.Equal(t, expectedLogEntries[i].Message, actualLogEntries[i].Message, "Test case %s log message differs", testFile) + assert.Equal(t, expectedLogEntries[i].Level, uint32(actualLogEntries[i].Level), "Test case %s log level differs", testFile) + } +} + // TestStatusEndpointReadiness asserts the http:///status endpoint // is responds as expected. func TestStatusEndpointReadiness(t *testing.T) { @@ -44,19 +280,11 @@ func TestStatusEndpointReadiness(t *testing.T) { // TestSuccessfulPut asserts the *PuntHandler.handle() function both successfully // stores the incomming request value and responds with an http.StatusOK code func TestSuccessfulPut(t *testing.T) { - type metricRecords struct { - totalRequests int64 - keyWasProvided int64 - badRequests int64 - requestErrs int64 - requestDur float64 - } - type testCase struct { desc string inPutBody string expectedStoredValue string - expectedMetrics metricRecords + expectedMetrics []string } testGroups := []struct { @@ -72,63 +300,77 @@ func TestSuccessfulPut(t *testing.T) { desc: "TestJSONString", inPutBody: "{\"puts\":[{\"type\":\"json\",\"value\":\"plain text\"}]}", expectedStoredValue: "\"plain text\"", - expectedMetrics: metricRecords{ - totalRequests: int64(1), - requestDur: 1.00, + expectedMetrics: []string{ + "RecordGetTotal", + "RecordGetDuration", + "RecordPutTotal", + "RecordPutDuration", }, }, { desc: "TestEscapedString", inPutBody: "{\"puts\":[{\"type\":\"json\",\"value\":\"esca\\\"ped\"}]}", expectedStoredValue: "\"esca\\\"ped\"", - expectedMetrics: metricRecords{ - totalRequests: int64(1), - requestDur: 1.00, + expectedMetrics: []string{ + "RecordGetTotal", + "RecordGetDuration", + "RecordPutTotal", + "RecordPutDuration", }, }, { desc: "TestNumber", inPutBody: "{\"puts\":[{\"type\":\"json\",\"value\":5}]}", expectedStoredValue: "5", - expectedMetrics: metricRecords{ - totalRequests: int64(1), - requestDur: 1.00, + expectedMetrics: []string{ + "RecordGetTotal", + "RecordGetDuration", + "RecordPutTotal", + "RecordPutDuration", }, }, { desc: "TestObject", inPutBody: "{\"puts\":[{\"type\":\"json\",\"value\":{\"custom_key\":\"foo\"}}]}", expectedStoredValue: "{\"custom_key\":\"foo\"}", - expectedMetrics: metricRecords{ - totalRequests: int64(1), - requestDur: 1.00, + expectedMetrics: []string{ + "RecordGetTotal", + "RecordGetDuration", + "RecordPutTotal", + "RecordPutDuration", }, }, { desc: "TestNull", inPutBody: "{\"puts\":[{\"type\":\"json\",\"value\":null}]}", expectedStoredValue: "null", - expectedMetrics: metricRecords{ - totalRequests: int64(1), - requestDur: 1.00, + expectedMetrics: []string{ + "RecordGetTotal", + "RecordGetDuration", + "RecordPutTotal", + "RecordPutDuration", }, }, { desc: "TestBoolean", inPutBody: "{\"puts\":[{\"type\":\"json\",\"value\":true}]}", expectedStoredValue: "true", - expectedMetrics: metricRecords{ - totalRequests: int64(1), - requestDur: 1.00, + expectedMetrics: []string{ + "RecordGetTotal", + "RecordGetDuration", + "RecordPutTotal", + "RecordPutDuration", }, }, { desc: "TestExtraProperty", inPutBody: "{\"puts\":[{\"type\":\"json\",\"value\":null,\"irrelevant\":\"foo\"}]}", expectedStoredValue: "null", - expectedMetrics: metricRecords{ - totalRequests: int64(1), - requestDur: 1.00, + expectedMetrics: []string{ + "RecordGetTotal", + "RecordGetDuration", + "RecordPutTotal", + "RecordPutDuration", }, }, }, @@ -141,18 +383,22 @@ func TestSuccessfulPut(t *testing.T) { desc: "Regular ", inPutBody: "{\"puts\":[{\"type\":\"xml\",\"value\":\"\"}]}", expectedStoredValue: "", - expectedMetrics: metricRecords{ - totalRequests: int64(1), - requestDur: 1.00, + expectedMetrics: []string{ + "RecordGetTotal", + "RecordGetDuration", + "RecordPutTotal", + "RecordPutDuration", }, }, { desc: "TestCrossScriptEscaping", inPutBody: "{\"puts\":[{\"type\":\"xml\",\"value\":\"esc\\\"aped\"}]}", expectedStoredValue: "esc\"aped", - expectedMetrics: metricRecords{ - totalRequests: int64(1), - requestDur: 1.00, + expectedMetrics: []string{ + "RecordGetTotal", + "RecordGetDuration", + "RecordPutTotal", + "RecordPutDuration", }, }, }, @@ -164,7 +410,12 @@ func TestSuccessfulPut(t *testing.T) { // set test router := httprouter.New() backend := backends.NewMemoryBackend() - m := metricstest.CreateMockMetrics() + mockMetrics := metricstest.CreateMockMetrics() + m := &metrics.Metrics{ + MetricEngines: []metrics.CacheMetrics{ + &mockMetrics, + }, + } router.POST("/cache", NewPutHandler(backend, m, 10, true)) router.GET("/cache", NewGetHandler(backend, m, true)) @@ -195,11 +446,7 @@ func TestSuccessfulPut(t *testing.T) { } // assert the put call above logged expected metrics - assert.Equal(t, tc.expectedMetrics.totalRequests, metricstest.MockCounters["puts.current_url.request.total"], "%s - handle function should record every incomming PUT request", tc.desc) - assert.Equal(t, tc.expectedMetrics.keyWasProvided, metricstest.MockCounters["puts.current_url.request.custom_key"], "%s - custom key was provided for put request and was not accounted for", tc.desc) - assert.Equal(t, tc.expectedMetrics.badRequests, metricstest.MockCounters["puts.current_url.request.bad_request"], "%s - Bad request wasn't recorded", tc.desc) - assert.Equal(t, tc.expectedMetrics.requestErrs, metricstest.MockCounters["puts.current_url.request.error"], "%s - WriteGetResponse error should have been recorded", tc.desc) - assert.Equal(t, tc.expectedMetrics.requestDur, metricstest.MockHistograms["puts.current_url.duration"], "%s - Successful GET request should have recorded duration", tc.desc) + metricstest.AssertMetrics(t, tc.expectedMetrics, mockMetrics) } } @@ -253,7 +500,12 @@ func TestMalformedOrInvalidValue(t *testing.T) { backend := &mockBackend{} backend.On("Put", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) - m := metricstest.CreateMockMetrics() + mockMetrics := metricstest.CreateMockMetrics() + m := &metrics.Metrics{ + MetricEngines: []metrics.CacheMetrics{ + &mockMetrics, + }, + } router.POST("/cache", NewPutHandler(backend, m, 10, true)) @@ -267,11 +519,11 @@ func TestMalformedOrInvalidValue(t *testing.T) { backend.AssertNumberOfCalls(t, "Put", tc.expectedPutCalls) // assert the put call above logged expected metrics - assert.Equal(t, int64(1), metricstest.MockCounters["puts.current_url.request.total"], "%s - handle function should record every incomming PUT request", tc.desc) - assert.Equal(t, int64(0), metricstest.MockCounters["puts.current_url.request.custom_key"], "%s - custom key was provided for put request and was not accounted for", tc.desc) - assert.Equal(t, int64(1), metricstest.MockCounters["puts.current_url.request.bad_request"], "%s - Bad request wasn't recorded", tc.desc) - assert.Equal(t, int64(0), metricstest.MockCounters["puts.current_url.request.error"], "%s - WriteGetResponse error should have been recorded", tc.desc) - assert.Equal(t, float64(0), metricstest.MockHistograms["puts.current_url.duration"], "%s - Successful GET request should have recorded duration", tc.desc) + expectedMetrics := []string{ + "RecordPutTotal", + "RecordPutBadRequest", + } + metricstest.AssertMetrics(t, expectedMetrics, mockMetrics) } } @@ -285,21 +537,26 @@ func TestNonSupportedType(t *testing.T) { backend.On("Put", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) router := httprouter.New() - m := metricstest.CreateMockMetrics() + mockMetrics := metricstest.CreateMockMetrics() + m := &metrics.Metrics{ + MetricEngines: []metrics.CacheMetrics{ + &mockMetrics, + }, + } router.POST("/cache", NewPutHandler(backend, m, 10, true)) putResponse := doPut(t, router, requestBody) require.Equal(t, http.StatusBadRequest, putResponse.Code, "Expected 400 response. Got: %d, Msg: %v", putResponse.Code, putResponse.Body.String()) - require.Equal(t, "Type must be one of [\"json\", \"xml\"]. Found yaml\n", putResponse.Body.String(), "Put() return error doesn't match expected.") + require.Equal(t, "Type must be one of [\"json\", \"xml\"]. Found 'yaml'\n", putResponse.Body.String(), "Put() return error doesn't match expected.") backend.AssertNotCalled(t, "Put") - assert.Equal(t, int64(1), metricstest.MockCounters["puts.current_url.request.total"], "Handle function should record every incomming PUT request") - assert.Equal(t, int64(0), metricstest.MockCounters["puts.current_url.request.custom_key"], "Custom key was provided for put request and was not accounted for") - assert.Equal(t, int64(1), metricstest.MockCounters["puts.current_url.request.bad_request"], "Bad request wasn't recorded") - assert.Equal(t, int64(0), metricstest.MockCounters["puts.current_url.request.error"], "WriteGetResponse error should have been recorded") - assert.Equal(t, 0.00, metricstest.MockHistograms["puts.current_url.duration"], "Successful GET request should have recorded duration") + expectedMetrics := []string{ + "RecordPutTotal", + "RecordPutBadRequest", + } + metricstest.AssertMetrics(t, expectedMetrics, mockMetrics) } func TestPutNegativeTTL(t *testing.T) { @@ -315,7 +572,12 @@ func TestPutNegativeTTL(t *testing.T) { // Set up server to run our test testRouter := httprouter.New() testBackend := backends.NewMemoryBackend() - m := metricstest.CreateMockMetrics() + mockMetrics := metricstest.CreateMockMetrics() + m := &metrics.Metrics{ + MetricEngines: []metrics.CacheMetrics{ + &mockMetrics, + }, + } testRouter.POST("/cache", NewPutHandler(testBackend, m, 10, true)) @@ -328,30 +590,22 @@ func TestPutNegativeTTL(t *testing.T) { assert.Equal(t, expectedErrorMsg, recorder.Body.String(), "Put should have failed because we passed a negative ttlseconds value.\n") assert.Equalf(t, expectedStatusCode, recorder.Code, "Expected 400 response. Got: %d", recorder.Code) - assert.Equal(t, int64(1), metricstest.MockCounters["puts.current_url.request.total"], "Handle function should record every incomming PUT request") - assert.Equal(t, int64(0), metricstest.MockCounters["puts.current_url.request.custom_key"], "Custom key was provided for put request and was not accounted for") - assert.Equal(t, int64(1), metricstest.MockCounters["puts.current_url.request.bad_request"], "Bad request wasn't recorded") - assert.Equal(t, int64(0), metricstest.MockCounters["puts.current_url.request.error"], "WriteGetResponse error should have been recorded") - assert.Equal(t, 0.00, metricstest.MockHistograms["puts.current_url.duration"], "Successful GET request should have recorded duration") + expectedMetrics := []string{ + "RecordPutTotal", + "RecordPutBadRequest", + } + metricstest.AssertMetrics(t, expectedMetrics, mockMetrics) } // TestCustomKey will assert the correct behavior when we try to store values that come with their own custom keys both // when `cfg.allowKeys` is set to `true` and `false`. It will use two custom keys, one that is already holding data in our // backend storage (36-char-key-maps-to-actual-xml-value) and one that doesn't (cust-key-maps-to-no-value-in-backend). func TestCustomKey(t *testing.T) { - type metricRecords struct { - totalRequests int64 - keyWasProvided int64 - badRequests int64 - requestErrs int64 - requestDur float64 - } - type aTest struct { desc string inCustomKey string expectedUUID string - expectedMetrics metricRecords + expectedMetrics []string } testGroups := []struct { allowSettingKeys bool @@ -364,18 +618,18 @@ func TestCustomKey(t *testing.T) { desc: "Custom key exists in cache but, because allowKeys is set to false we store the value using a random UUID and respond 200", inCustomKey: "36-char-key-maps-to-actual-xml-value", expectedUUID: `[a-z0-9]{8}-[a-z0-9]{4}-[a-z0-9]{4}-[a-z0-9]{4}-[a-z0-9]{12}`, - expectedMetrics: metricRecords{ - totalRequests: int64(1), - requestDur: 1.00, + expectedMetrics: []string{ + "RecordPutTotal", + "RecordPutDuration", }, }, { desc: "Custom key doesn't exist in cache but we can't store data under it because allowKeys is set to false. Store value with random UUID and respond 200", inCustomKey: "cust-key-maps-to-no-value-in-backend", expectedUUID: `[a-z0-9]{8}-[a-z0-9]{4}-[a-z0-9]{4}-[a-z0-9]{4}-[a-z0-9]{12}`, - expectedMetrics: metricRecords{ - totalRequests: int64(1), - requestDur: 1.00, + expectedMetrics: []string{ + "RecordPutTotal", + "RecordPutDuration", }, }, }, @@ -387,20 +641,20 @@ func TestCustomKey(t *testing.T) { desc: "Setting keys allowed but key already maps to an element in cache, don't overwrite the value in the data storage and simply respond with blank UUID and a 200 code", inCustomKey: "36-char-key-maps-to-actual-xml-value", expectedUUID: "", - expectedMetrics: metricRecords{ - totalRequests: int64(1), - keyWasProvided: int64(1), - requestDur: 1.00, + expectedMetrics: []string{ + "RecordPutTotal", + "RecordPutDuration", + "RecordPutKeyProvided", }, }, { desc: "Custom key maps to no element in cache, store value using custom key and respond with a 200 code and the custom UUID", inCustomKey: "cust-key-maps-to-no-value-in-backend", expectedUUID: "cust-key-maps-to-no-value-in-backend", - expectedMetrics: metricRecords{ - totalRequests: int64(1), - keyWasProvided: int64(1), - requestDur: 1.00, + expectedMetrics: []string{ + "RecordPutKeyProvided", + "RecordPutTotal", + "RecordPutDuration", }, }, }, @@ -412,7 +666,12 @@ func TestCustomKey(t *testing.T) { // Instantiate prebid cache prod server with mock metrics and a mock metrics that // already contains some values mockBackendWithValues := newMockBackend() - m := metricstest.CreateMockMetrics() + mockMetrics := metricstest.CreateMockMetrics() + m := &metrics.Metrics{ + MetricEngines: []metrics.CacheMetrics{ + &mockMetrics, + }, + } router := httprouter.New() putEndpointHandler := NewPutHandler(mockBackendWithValues, m, 10, tgroup.allowSettingKeys) @@ -431,11 +690,7 @@ func TestCustomKey(t *testing.T) { assert.Equal(t, http.StatusOK, recorder.Code, tc.desc) // assert the put call above logged expected metrics - assert.Equal(t, tc.expectedMetrics.totalRequests, metricstest.MockCounters["puts.current_url.request.total"], "%s - Handle function should record every incomming PUT request", tc.desc) - assert.Equal(t, tc.expectedMetrics.keyWasProvided, metricstest.MockCounters["puts.current_url.request.custom_key"], "%s - Custom key was provided for put request and was not accounted for", tc.desc) - assert.Equal(t, tc.expectedMetrics.badRequests, metricstest.MockCounters["puts.current_url.request.bad_request"], "%s - Bad request wasn't recorded", tc.desc) - assert.Equal(t, tc.expectedMetrics.requestErrs, metricstest.MockCounters["puts.current_url.request.error"], "%s - WriteGetResponse error should have been recorded", tc.desc) - assert.Equal(t, tc.expectedMetrics.requestDur, metricstest.MockHistograms["puts.current_url.duration"], "%s - Successful GET request should have recorded duration", tc.desc) + metricstest.AssertMetrics(t, tc.expectedMetrics, mockMetrics) // Assert response UUID if tc.expectedUUID == "" { @@ -450,7 +705,12 @@ func TestCustomKey(t *testing.T) { func TestRequestReadError(t *testing.T) { // Setup server and mock body request reader mockBackendWithValues := newMockBackend() - m := metricstest.CreateMockMetrics() + mockMetrics := metricstest.CreateMockMetrics() + m := &metrics.Metrics{ + MetricEngines: []metrics.CacheMetrics{ + &mockMetrics, + }, + } putEndpointHandler := NewPutHandler(mockBackendWithValues, m, 10, false) router := httprouter.New() @@ -472,11 +732,8 @@ func TestRequestReadError(t *testing.T) { assert.Equal(t, http.StatusBadRequest, recorder.Code, "Expected a bad request status code from a malformed request") // assert the put call above logged expected metrics - assert.Equal(t, int64(1), metricstest.MockCounters["puts.current_url.request.total"], "Handle function should record every incomming PUT request") - assert.Equal(t, int64(0), metricstest.MockCounters["puts.current_url.request.custom_key"], "Custom key was provided for put request and was not accounted for") - assert.Equal(t, int64(1), metricstest.MockCounters["puts.current_url.request.bad_request"], "Bad request wasn't recorded") - assert.Equal(t, int64(0), metricstest.MockCounters["puts.current_url.request.error"], "WriteGetResponse error should have been recorded") - assert.Equal(t, 0.00, metricstest.MockHistograms["puts.current_url.duration"], "Successful GET request should have recorded duration") + expectedMetrics := []string{"RecordPutTotal", "RecordPutBadRequest"} + metricstest.AssertMetrics(t, expectedMetrics, mockMetrics) } func TestTooManyPutElements(t *testing.T) { @@ -493,7 +750,12 @@ func TestTooManyPutElements(t *testing.T) { backend.On("Put", mock.Anything, mock.Anything, mock.Anything, mock.Anything).Return(nil) router := httprouter.New() - m := metricstest.CreateMockMetrics() + mockMetrics := metricstest.CreateMockMetrics() + m := &metrics.Metrics{ + MetricEngines: []metrics.CacheMetrics{ + &mockMetrics, + }, + } router.POST("/cache", NewPutHandler(backend, m, len(putElements)-1, true)) putResponse := doPut(t, router, reqBody) @@ -503,11 +765,9 @@ func TestTooManyPutElements(t *testing.T) { backend.AssertNotCalled(t, "Put") - assert.Equal(t, int64(1), metricstest.MockCounters["puts.current_url.request.total"], "Handle function should record every incomming PUT request") - assert.Equal(t, int64(0), metricstest.MockCounters["puts.current_url.request.custom_key"], "Custom key was provided for put request and was not accounted for") - assert.Equal(t, int64(1), metricstest.MockCounters["puts.current_url.request.bad_request"], "Bad request wasn't recorded") - assert.Equal(t, int64(0), metricstest.MockCounters["puts.current_url.request.error"], "WriteGetResponse error should have been recorded") - assert.Equal(t, 0.00, metricstest.MockHistograms["puts.current_url.duration"], "Successful GET request should have recorded duration") + // assert the put call above logged expected metrics + expectedMetrics := []string{"RecordPutTotal", "RecordPutBadRequest"} + metricstest.AssertMetrics(t, expectedMetrics, mockMetrics) } // TestMultiPutRequest asserts results for requests with more than one element in the "puts" array @@ -544,7 +804,12 @@ func TestMultiPutRequest(t *testing.T) { // Set up server and run router := httprouter.New() backend := backends.NewMemoryBackend() - m := metricstest.CreateMockMetrics() + mockMetrics := metricstest.CreateMockMetrics() + m := &metrics.Metrics{ + MetricEngines: []metrics.CacheMetrics{ + &mockMetrics, + }, + } router.POST("/cache", NewPutHandler(backend, m, 10, true)) router.GET("/cache", NewGetHandler(backend, m, true)) @@ -555,11 +820,9 @@ func TestMultiPutRequest(t *testing.T) { // Validate results // Assert put metrics - assert.Equal(t, int64(1), metricstest.MockCounters["puts.current_url.request.total"], "Handle function should record every incomming PUT request") - assert.Equal(t, int64(0), metricstest.MockCounters["puts.current_url.request.custom_key"], "Custom key was provided for put request and was not accounted for") - assert.Equal(t, int64(0), metricstest.MockCounters["puts.current_url.request.bad_request"], "Bad request wasn't recorded") - assert.Equal(t, int64(0), metricstest.MockCounters["puts.current_url.request.error"], "WriteGetResponse error should have been recorded") - assert.Equal(t, 1.00, metricstest.MockHistograms["puts.current_url.duration"], "Successful GET request should have recorded duration") + // assert the put call above logged expected metrics + expectedMetrics := []string{"RecordPutTotal", "RecordPutDuration"} + metricstest.AssertMetrics(t, expectedMetrics, mockMetrics) // Assert put request var parsed PutResponse @@ -577,10 +840,8 @@ func TestMultiPutRequest(t *testing.T) { } // Assert get metrics - assert.Equal(t, int64(3), metricstest.MockCounters["gets.current_url.request.total"], "Handle function should record every incomming GET request") - assert.Equal(t, int64(0), metricstest.MockCounters["gets.current_url.request.bad_request"], "Bad request wasn't recorded") - assert.Equal(t, int64(0), metricstest.MockCounters["gets.current_url.request.error"], "WriteGetResponse error should have been recorded") - assert.Equal(t, 1.00, metricstest.MockHistograms["gets.current_url.duration"], "Successful GET request should have recorded duration") + expectedMetrics = append(expectedMetrics, "RecordGetTotal", "RecordGetDuration") + metricstest.AssertMetrics(t, expectedMetrics, mockMetrics) } func TestBadPayloadSizePutError(t *testing.T) { @@ -595,7 +856,12 @@ func TestBadPayloadSizePutError(t *testing.T) { // Run client router := httprouter.New() - m := metricstest.CreateMockMetrics() + mockMetrics := metricstest.CreateMockMetrics() + m := &metrics.Metrics{ + MetricEngines: []metrics.CacheMetrics{ + &mockMetrics, + }, + } router.POST("/cache", NewPutHandler(backend, m, 10, true)) putResponse := doPut(t, router, reqBody) @@ -605,45 +871,51 @@ func TestBadPayloadSizePutError(t *testing.T) { assert.Equal(t, "POST /cache element 0 exceeded max size: Payload size 30 exceeded max 3\n", putResponse.Body.String(), "Put() return error doesn't match expected.") // metrics - assert.Equal(t, int64(1), metricstest.MockCounters["puts.current_url.request.total"], "Handle function should record every incomming PUT request") - assert.Equal(t, int64(0), metricstest.MockCounters["puts.current_url.request.custom_key"], "Custom key was provided for put request and was not accounted for") - assert.Equal(t, int64(1), metricstest.MockCounters["puts.current_url.request.bad_request"], "Bad request wasn't recorded") - assert.Equal(t, int64(0), metricstest.MockCounters["puts.current_url.request.error"], "WriteGetResponse error should have been recorded") - assert.Equal(t, 0.00, metricstest.MockHistograms["puts.current_url.duration"], "Successful GET request should have recorded duration") + expectedMetrics := []string{"RecordPutTotal", "RecordPutBadRequest"} + metricstest.AssertMetrics(t, expectedMetrics, mockMetrics) } func TestInternalPutClientError(t *testing.T) { - // Valid request + // Input reqBody := "{\"puts\":[{\"type\":\"xml\",\"value\":\"some data\"}]}" + // Expected metrics + expectedMetrics := []string{ + "RecordPutTotal", + "RecordPutError", + "RecordPutBackendXml", + "RecordPutBackendError", + "RecordPutBackendSize", + "RecordPutBackendTTLSeconds", + } + // Init test objects: + router := httprouter.New() + mockMetrics := metricstest.CreateMockMetrics() + m := &metrics.Metrics{ + MetricEngines: []metrics.CacheMetrics{ + &mockMetrics, + }, + } // Use mock client that will return an error - backend := newErrorReturningBackend() + backendWithMetrics := decorators.LogMetrics(newErrorReturningBackend(), m) - // Run client - router := httprouter.New() - m := metricstest.CreateMockMetrics() - router.POST("/cache", NewPutHandler(backend, m, 10, true)) + router.POST("/cache", NewPutHandler(backendWithMetrics, m, 10, true)) + // Run test putResponse := doPut(t, router, reqBody) // Assert expected response assert.Equal(t, http.StatusInternalServerError, putResponse.Code, "Put should have failed because we are using an MockReturnErrorBackend") assert.Equal(t, "This is a mock backend that returns this error on Put() operation\n", putResponse.Body.String(), "Put() return error doesn't match expected.") - - // metrics - assert.Equal(t, int64(1), metricstest.MockCounters["puts.current_url.request.total"], "Handle function should record every incomming PUT request") - assert.Equal(t, int64(0), metricstest.MockCounters["puts.current_url.request.custom_key"], "Custom key was provided for put request and was not accounted for") - assert.Equal(t, int64(0), metricstest.MockCounters["puts.current_url.request.bad_request"], "Bad request wasn't recorded") - assert.Equal(t, int64(1), metricstest.MockCounters["puts.current_url.request.error"], "WriteGetResponse error should have been recorded") - assert.Equal(t, 0.00, metricstest.MockHistograms["puts.current_url.duration"], "Successful GET request should have recorded duration") + metricstest.AssertMetrics(t, expectedMetrics, mockMetrics) } func TestEmptyPutRequests(t *testing.T) { type testOutput struct { - jsonResponse string - statusCode int - metricsBadRequest int64 - metricsDuration float64 + jsonResponse string + statusCode int + badRequestMetricsLogged bool + durationMetricsLogged bool } type aTest struct { desc string @@ -655,20 +927,18 @@ func TestEmptyPutRequests(t *testing.T) { desc: "No value in put element", reqBody: `{"puts":[{"type":"xml"}]}`, expected: testOutput{ - jsonResponse: `Missing value`, - statusCode: http.StatusBadRequest, - metricsBadRequest: 1, - metricsDuration: 0.00, + jsonResponse: `Missing value`, + statusCode: http.StatusBadRequest, + badRequestMetricsLogged: true, }, }, { desc: "Blank value in put element", reqBody: `{"puts":[{"type":"xml","value":""}]}`, expected: testOutput{ - jsonResponse: `{"responses":\[\{"uuid":"[a-z0-9-]+"\}\]}`, - statusCode: http.StatusOK, - metricsBadRequest: 0, - metricsDuration: 1.00, + jsonResponse: `{"responses":\[\{"uuid":"[a-z0-9-]+"\}\]}`, + statusCode: http.StatusOK, + durationMetricsLogged: true, }, }, // This test is meant to come right after the "Blank value in put element" test in order to assert the correction @@ -677,50 +947,57 @@ func TestEmptyPutRequests(t *testing.T) { desc: "All empty body. ", reqBody: "{}", expected: testOutput{ - jsonResponse: `{"responses":\[\]}`, - statusCode: http.StatusOK, - metricsBadRequest: 0, - metricsDuration: 1.00, + jsonResponse: `{"responses":\[\]}`, + statusCode: http.StatusOK, + durationMetricsLogged: true, }, }, { desc: "Empty puts arrray", reqBody: "{\"puts\":[]}", expected: testOutput{ - jsonResponse: `{"responses":\[\]}`, - statusCode: http.StatusOK, - metricsBadRequest: 0, - metricsDuration: 1.00, + jsonResponse: `{"responses":\[\]}`, + statusCode: http.StatusOK, + durationMetricsLogged: true, }, }, } - for i, test := range testCases { + for i, tc := range testCases { // Set up server backend := backends.NewMemoryBackend() - m := metricstest.CreateMockMetrics() + mockMetrics := metricstest.CreateMockMetrics() + m := &metrics.Metrics{ + MetricEngines: []metrics.CacheMetrics{ + &mockMetrics, + }, + } router := httprouter.New() router.POST("/cache", NewPutHandler(backend, m, 10, true)) rr := httptest.NewRecorder() // Create request everytime - request, err := http.NewRequest("POST", "/cache", strings.NewReader(test.reqBody)) + request, err := http.NewRequest("POST", "/cache", strings.NewReader(tc.reqBody)) assert.NoError(t, err, "[%d] Failed to create a POST request: %v", i, err) // Run router.ServeHTTP(rr, request) - //assert.Equal(t, http.StatusOK, rr.Code, "[%d] ServeHTTP(rr, request) failed = %v \n", i, rr.Result()) - assert.Equal(t, test.expected.statusCode, rr.Code, "[%d] ServeHTTP(rr, request) failed = %v - %s", i, rr.Result()) + assert.Equal(t, tc.expected.statusCode, rr.Code, "[%d] ServeHTTP(rr, request) failed = %v - %s", i, rr.Result()) // Assert expected JSON response - if !assert.Regexp(t, regexp.MustCompile(test.expected.jsonResponse), rr.Body.String(), "[%d] Response body differs from expected - %s", i, test.desc) { + if !assert.Regexp(t, regexp.MustCompile(tc.expected.jsonResponse), rr.Body.String(), "[%d] Response body differs from expected - %s", i, tc.desc) { return } // Assert metrics - assert.Equal(t, int64(1), metricstest.MockCounters["puts.current_url.request.total"], "[%d] Handle function should record every incomming PUT request - %s", i, test.desc) - assert.Equal(t, test.expected.metricsBadRequest, metricstest.MockCounters["puts.current_url.request.bad_request"], "[%d] Bad request wasn't recorded - %s", i, test.desc) - assert.Equal(t, test.expected.metricsDuration, metricstest.MockHistograms["puts.current_url.duration"], "[%d] Successful PUT request should have recorded duration - %s", i, test.desc) + expectedMetrics := []string{"RecordPutTotal"} + if tc.expected.badRequestMetricsLogged { + expectedMetrics = append(expectedMetrics, "RecordPutBadRequest") + } + if tc.expected.durationMetricsLogged { + expectedMetrics = append(expectedMetrics, "RecordPutDuration") + } + metricstest.AssertMetrics(t, expectedMetrics, mockMetrics) } } @@ -733,7 +1010,12 @@ func TestPutClientDeadlineExceeded(t *testing.T) { // Run client router := httprouter.New() - m := metricstest.CreateMockMetrics() + mockMetrics := metricstest.CreateMockMetrics() + m := &metrics.Metrics{ + MetricEngines: []metrics.CacheMetrics{ + &mockMetrics, + }, + } router.POST("/cache", NewPutHandler(backend, m, 10, true)) putResponse := doPut(t, router, reqBody) @@ -743,11 +1025,11 @@ func TestPutClientDeadlineExceeded(t *testing.T) { assert.Equal(t, "timeout writing value to the backend.\n", putResponse.Body.String(), "Put() return error doesn't match expected.") // Assert this request is accounted under the "puts.current_url.request.error" metrics - assert.Equal(t, int64(1), metricstest.MockCounters["puts.current_url.request.total"], "Handle function should record every incomming PUT request") - assert.Equal(t, int64(0), metricstest.MockCounters["puts.current_url.request.custom_key"], "Custom key was provided for put request and was not accounted for") - assert.Equal(t, int64(0), metricstest.MockCounters["puts.current_url.request.bad_request"], "Bad request wasn't recorded") - assert.Equal(t, int64(1), metricstest.MockCounters["puts.current_url.request.error"], "WriteGetResponse error should have been recorded") - assert.Equal(t, 0.00, metricstest.MockHistograms["puts.current_url.duration"], "Successful GET request should have recorded duration") + expectedMetrics := []string{ + "RecordPutTotal", + "RecordPutError", + } + metricstest.AssertMetrics(t, expectedMetrics, mockMetrics) } // TestParseRequest asserts *PutHandler's parseRequest(r *http.Request) method @@ -857,7 +1139,7 @@ func TestParsePutObject(t *testing.T) { }, testOut{ value: "", - err: utils.NewPBCError(utils.UNSUPPORTED_DATA_TO_STORE, "Type must be one of [\"json\", \"xml\"]. Found unknown"), + err: utils.NewPBCError(utils.UNSUPPORTED_DATA_TO_STORE, "Type must be one of [\"json\", \"xml\"]. Found 'unknown'"), }, }, { @@ -1023,7 +1305,12 @@ func benchmarkPutHandler(b *testing.B, testCase string) { //Set up server ready to run router := httprouter.New() backend := backends.NewMemoryBackend() - m := metricstest.CreateMockMetrics() + mockMetrics := metricstest.CreateMockMetrics() + m := &metrics.Metrics{ + MetricEngines: []metrics.CacheMetrics{ + &mockMetrics, + }, + } router.POST("/cache", NewPutHandler(backend, m, 10, true)) router.GET("/cache", NewGetHandler(backend, m, true)) diff --git a/endpoints/sample-requests/put-endpoint/custom-keys/allowed/key-field-included.json b/endpoints/sample-requests/put-endpoint/custom-keys/allowed/key-field-included.json new file mode 100644 index 00000000..f0a22740 --- /dev/null +++ b/endpoints/sample-requests/put-endpoint/custom-keys/allowed/key-field-included.json @@ -0,0 +1,30 @@ +{ + "description": "Prebid Cache has been configured to allow to store elements under custom keys. Store data under element-defined 'key' value.", + "serverConfig": { + "allow_setting_keys": true + }, + "putRequest": { + "puts":[ + { + "type":"xml", + "ttlseconds":60, + "value":"other_XML_content", + "key":"the-custom-thrity-six-character-uuid" + } + ] + }, + "expectedMetrics": [ + "RecordPutTotal", + "RecordPutKeyProvided", + "RecordPutBackendXml", + "RecordPutBackendSize", + "RecordPutBackendTTLSeconds", + "RecordPutBackendDuration", + "RecordPutDuration" + ], + "expectedResponse": { + "responses": [ + {"uuid": "the-custom-thrity-six-character-uuid"} + ] + } +} diff --git a/endpoints/sample-requests/put-endpoint/custom-keys/allowed/key-field-missing.json b/endpoints/sample-requests/put-endpoint/custom-keys/allowed/key-field-missing.json new file mode 100644 index 00000000..b31b56ee --- /dev/null +++ b/endpoints/sample-requests/put-endpoint/custom-keys/allowed/key-field-missing.json @@ -0,0 +1,28 @@ +{ + "description": "Prebid Cache has been configured to allow to store elements under custom keys but element came with empty 'key' field. Store under random UUID", + "serverConfig": { + "allow_setting_keys": true + }, + "putRequest": { + "puts": [ + { + "type": "xml", + "ttlseconds": 60, + "value": "__video_info__<\\/VAST>\r\n" + } + ] + }, + "expectedMetrics": [ + "RecordPutTotal", + "RecordPutBackendXml", + "RecordPutBackendSize", + "RecordPutBackendTTLSeconds", + "RecordPutBackendDuration", + "RecordPutDuration" + ], + "expectedResponse": { + "responses": [ + {"uuid": "random"} + ] + } +} diff --git a/endpoints/sample-requests/put-endpoint/custom-keys/not-allowed/key-field-included.json b/endpoints/sample-requests/put-endpoint/custom-keys/not-allowed/key-field-included.json new file mode 100644 index 00000000..68894318 --- /dev/null +++ b/endpoints/sample-requests/put-endpoint/custom-keys/not-allowed/key-field-included.json @@ -0,0 +1,26 @@ +{ + "description": "Put request wants to store element under a custom key but custom keys are not allowed in Prebid Cache's config. Store under a random UUID", + "putRequest": { + "puts":[ + { + "type":"xml", + "ttlseconds":60, + "value":"other_XML_content", + "key":"the-custom-thrity-six-character-uuid" + } + ] + }, + "expectedMetrics": [ + "RecordPutTotal", + "RecordPutBackendXml", + "RecordPutBackendSize", + "RecordPutBackendTTLSeconds", + "RecordPutBackendDuration", + "RecordPutDuration" + ], + "expectedResponse": { + "responses": [ + {"uuid": "random"} + ] + } +} diff --git a/endpoints/sample-requests/put-endpoint/invalid-number-of-elements/puts-max-num-values.json b/endpoints/sample-requests/put-endpoint/invalid-number-of-elements/puts-max-num-values.json new file mode 100644 index 00000000..ef438a08 --- /dev/null +++ b/endpoints/sample-requests/put-endpoint/invalid-number-of-elements/puts-max-num-values.json @@ -0,0 +1,17 @@ +{ + "description": "Put request wants to store more elements than allowed in the 'max_num_values' configuration. Don't store and return error", + "serverConfig": { + "max_num_values": 1 + }, + "putRequest": { + "puts": [ + {"type":"xml","ttlseconds":5,"value":"__video_info__<\\/VAST>\r\n"}, + {"type":"json","ttlseconds":5,"value":"{\"field\":100}"} + ] + }, + "expectedMetrics": [ + "RecordPutTotal", + "RecordPutBadRequest" + ], + "expectedErrorMessage": "More keys than allowed: 1\n" +} diff --git a/endpoints/sample-requests/put-endpoint/invalid-types/type-missing.json b/endpoints/sample-requests/put-endpoint/invalid-types/type-missing.json new file mode 100644 index 00000000..9db3f721 --- /dev/null +++ b/endpoints/sample-requests/put-endpoint/invalid-types/type-missing.json @@ -0,0 +1,26 @@ +{ + "description": "Prebid Cache only allows to store JSON or XML types and the type field is required. Respond with error", + "putRequest": { + "puts": [ + { + "ttlseconds": 60, + "value": "__video_info__<\\/VAST>\r\n" + } + ] + }, + "expectedLogEntries": [ + { + "message": "POST /cache Error while writing to the back-end: Type must be one of [\"json\", \"xml\"]. Found ''", + "level": 2 + }, + { + "message": "POST /cache had an unexpected error:Type must be one of [\"json\", \"xml\"]. Found ''", + "level": 2 + } + ], + "expectedMetrics": [ + "RecordPutTotal", + "RecordPutBadRequest" + ], + "expectedErrorMessage": "Type must be one of [\"json\", \"xml\"]. Found ''\n" +} diff --git a/endpoints/sample-requests/put-endpoint/invalid-types/type-unknown.json b/endpoints/sample-requests/put-endpoint/invalid-types/type-unknown.json new file mode 100644 index 00000000..790b06fb --- /dev/null +++ b/endpoints/sample-requests/put-endpoint/invalid-types/type-unknown.json @@ -0,0 +1,27 @@ +{ + "description": "Prebid Cache only allows to store JSON or XML types and the type 'unknown' is not supported. Respond with error", + "putRequest": { + "puts": [ + { + "type": "unknown", + "ttlseconds": 60, + "value": "some-value" + } + ] + }, + "expectedLogEntries": [ + { + "message": "POST /cache Error while writing to the back-end: Type must be one of [\"json\", \"xml\"]. Found 'unknown'", + "level": 2 + }, + { + "message": "POST /cache had an unexpected error:Type must be one of [\"json\", \"xml\"]. Found 'unknown'", + "level": 2 + } + ], + "expectedMetrics": [ + "RecordPutTotal", + "RecordPutBadRequest" + ], + "expectedErrorMessage": "Type must be one of [\"json\", \"xml\"]. Found 'unknown'\n" +} diff --git a/endpoints/sample-requests/put-endpoint/invalid-value/value-greater-than-max.json b/endpoints/sample-requests/put-endpoint/invalid-value/value-greater-than-max.json new file mode 100644 index 00000000..9acd14a9 --- /dev/null +++ b/endpoints/sample-requests/put-endpoint/invalid-value/value-greater-than-max.json @@ -0,0 +1,34 @@ +{ + "description": "Put request wants to store an element with a size that exceeds the 'max_size_bytes' value. Don't store and return error", + "serverConfig": { + "max_size_bytes": 5 + }, + "putRequest": { + "puts": [ + { + "type": "xml", + "ttlseconds": 5, + "value": "\r\n<\\/VAST>\r\n" + } + ] + }, + "expectedLogEntries": [ + { + "message": "POST /cache Error while writing to the back-end: POST /cache element 0 exceeded max size: Payload size 73 exceeded max 5", + "level": 2 + }, + { + "message": "POST /cache had an unexpected error:POST /cache element 0 exceeded max size: Payload size 73 exceeded max 5", + "level": 2 + } + ], + "expectedMetrics": [ + "RecordPutTotal", + "RecordPutBackendXml", + "RecordPutBackendSize", + "RecordPutBackendTTLSeconds", + "RecordPutBackendError", + "RecordPutBadRequest" + ], + "expectedErrorMessage": "POST /cache element 0 exceeded max size: Payload size 73 exceeded max 5\n" +} diff --git a/endpoints/sample-requests/put-endpoint/invalid-value/value-missing.json b/endpoints/sample-requests/put-endpoint/invalid-value/value-missing.json new file mode 100644 index 00000000..9523215c --- /dev/null +++ b/endpoints/sample-requests/put-endpoint/invalid-value/value-missing.json @@ -0,0 +1,26 @@ +{ + "description": "Prebid Cache does not allow the storage of empty values. Respond with error", + "putRequest": { + "puts": [ + { + "type": "xml", + "ttlseconds": 60 + } + ] + }, + "expectedLogEntries": [ + { + "message": "POST /cache Error while writing to the back-end: Missing value.", + "level": 2 + }, + { + "message": "POST /cache had an unexpected error:Missing value.", + "level": 2 + } + ], + "expectedMetrics": [ + "RecordPutTotal", + "RecordPutBadRequest" + ], + "expectedErrorMessage": "Missing value.\n" +} diff --git a/endpoints/sample-requests/put-endpoint/valid-whole/multiple-elements-to-store.json b/endpoints/sample-requests/put-endpoint/valid-whole/multiple-elements-to-store.json new file mode 100644 index 00000000..407d5848 --- /dev/null +++ b/endpoints/sample-requests/put-endpoint/valid-whole/multiple-elements-to-store.json @@ -0,0 +1,35 @@ +{ + "description": "Put request wants to store multiple elements but no more than the maximum allowed by the 'max_num_values' config. Store them under a random UUIDs", + "serverConfig": { + "max_num_values": 2 + }, + "putRequest": { + "puts": [ + { + "type": "xml", + "ttlseconds": 60, + "value": "__video_info__<\\/VAST>\r\n" + }, + { + "type": "json", + "ttlseconds": 60, + "value": "{\"an_int_field\": 1}" + } + ] + }, + "expectedMetrics": [ + "RecordPutTotal", + "RecordPutBackendXml", + "RecordPutBackendJson", + "RecordPutBackendSize", + "RecordPutBackendTTLSeconds", + "RecordPutBackendDuration", + "RecordPutDuration" + ], + "expectedResponse": { + "responses": [ + {"uuid": "random"}, + {"uuid": "random"} + ] + } +} diff --git a/endpoints/sample-requests/put-endpoint/valid-whole/no-elements-to-store.json b/endpoints/sample-requests/put-endpoint/valid-whole/no-elements-to-store.json new file mode 100644 index 00000000..f5e20835 --- /dev/null +++ b/endpoints/sample-requests/put-endpoint/valid-whole/no-elements-to-store.json @@ -0,0 +1,13 @@ +{ + "description": "Put request with empty 'puts' array does not return an error, we simply respond with an emtpy 'responses' array.", + "putRequest": { + "puts": [] + }, + "expectedMetrics": [ + "RecordPutTotal", + "RecordPutDuration" + ], + "expectedResponse": { + "responses": [ ] + } +} diff --git a/endpoints/sample-requests/put-endpoint/valid-whole/single-element-to-store.json b/endpoints/sample-requests/put-endpoint/valid-whole/single-element-to-store.json new file mode 100644 index 00000000..9d541889 --- /dev/null +++ b/endpoints/sample-requests/put-endpoint/valid-whole/single-element-to-store.json @@ -0,0 +1,25 @@ +{ + "description": "Put request wants to store a single element of valid type no larger than the maximum size allowed. Store under a random UUID", + "putRequest": { + "puts": [ + { + "type": "xml", + "ttlseconds": 60, + "value": "__video_info__<\\/VAST>\r\n" + } + ] + }, + "expectedMetrics": [ + "RecordPutTotal", + "RecordPutBackendXml", + "RecordPutBackendSize", + "RecordPutBackendTTLSeconds", + "RecordPutBackendDuration", + "RecordPutDuration" + ], + "expectedResponse": { + "responses": [ + {"uuid": "random"} + ] + } +} diff --git a/endpoints/sample-requests/put-endpoint/valid-whole/ttl-missing.json b/endpoints/sample-requests/put-endpoint/valid-whole/ttl-missing.json new file mode 100644 index 00000000..a25e896a --- /dev/null +++ b/endpoints/sample-requests/put-endpoint/valid-whole/ttl-missing.json @@ -0,0 +1,24 @@ +{ + "description": "Object to store doesn't come with a time-to-live value. Prebid Cache allows for a zero time-to-live value and responds with a random UUID.", + "putRequest": { + "puts": [ + { + "type": "xml", + "value": "__video_info__<\\/VAST>\r\n" + } + ] + }, + "expectedMetrics": [ + "RecordPutTotal", + "RecordPutBackendXml", + "RecordPutBackendSize", + "RecordPutBackendTTLSeconds", + "RecordPutBackendDuration", + "RecordPutDuration" + ], + "expectedResponse": { + "responses": [ + {"uuid": "random"} + ] + } +} diff --git a/endpoints/sample-requests/put-endpoint/valid-whole/ttl-more-than-max.json b/endpoints/sample-requests/put-endpoint/valid-whole/ttl-more-than-max.json new file mode 100644 index 00000000..5e51d947 --- /dev/null +++ b/endpoints/sample-requests/put-endpoint/valid-whole/ttl-more-than-max.json @@ -0,0 +1,28 @@ +{ + "description": "Put request wants to store object for more seconds than Prebid Cache maximum. Cap at the 'max_ttl_seconds' value and store successfully", + "serverConfig": { + "max_ttl_seconds": 5 + }, + "putRequest": { + "puts": [ + { + "type": "xml", + "ttlseconds": 6, + "value": "__video_info__<\\/VAST>\r\n" + } + ] + }, + "expectedMetrics": [ + "RecordPutTotal", + "RecordPutBackendXml", + "RecordPutBackendSize", + "RecordPutBackendTTLSeconds", + "RecordPutBackendDuration", + "RecordPutDuration" + ], + "expectedResponse": { + "responses": [ + {"uuid": "random"} + ] + } +} diff --git a/endpoints/sample-requests/put-endpoint/valid-whole/valid-type-json.json b/endpoints/sample-requests/put-endpoint/valid-whole/valid-type-json.json new file mode 100644 index 00000000..a6c0fff5 --- /dev/null +++ b/endpoints/sample-requests/put-endpoint/valid-whole/valid-type-json.json @@ -0,0 +1,25 @@ +{ + "description": "Store JSON type value, which Prebid Cache allows. Store under a random UUID", + "putRequest": { + "puts": [ + { + "type": "json", + "ttlseconds": 60, + "value": "{\"field\":100}" + } + ] + }, + "expectedMetrics": [ + "RecordPutTotal", + "RecordPutBackendJson", + "RecordPutBackendSize", + "RecordPutBackendTTLSeconds", + "RecordPutBackendDuration", + "RecordPutDuration" + ], + "expectedResponse": { + "responses": [ + {"uuid": "random"} + ] + } +} diff --git a/endpoints/sample-requests/put-endpoint/valid-whole/valid-type-xml.json b/endpoints/sample-requests/put-endpoint/valid-whole/valid-type-xml.json new file mode 100644 index 00000000..a7b62b70 --- /dev/null +++ b/endpoints/sample-requests/put-endpoint/valid-whole/valid-type-xml.json @@ -0,0 +1,25 @@ +{ + "description": "Prebid Cache allows the storage of XML type values. Store under a random UUID because the 'key' field missing and custom keys are not allowed anyways", + "putRequest": { + "puts": [ + { + "type": "xml", + "ttlseconds": 60, + "value": "__video_info__<\\/VAST>\r\n" + } + ] + }, + "expectedMetrics": [ + "RecordPutTotal", + "RecordPutBackendXml", + "RecordPutBackendSize", + "RecordPutBackendTTLSeconds", + "RecordPutBackendDuration", + "RecordPutDuration" + ], + "expectedResponse": { + "responses": [ + {"uuid": "random"} + ] + } +} diff --git a/metrics/metricstest/metrics_mock.go b/metrics/metricstest/metrics_mock.go index 9065bb24..727dec6c 100644 --- a/metrics/metricstest/metrics_mock.go +++ b/metrics/metricstest/metrics_mock.go @@ -1,143 +1,236 @@ package metricstest import ( + "testing" "time" "github.com/prebid/prebid-cache/config" - "github.com/prebid/prebid-cache/metrics" + "github.com/stretchr/testify/mock" ) -const mockDuration time.Duration = time.Second +func AssertMetrics(t *testing.T, expectedMetrics []string, actualMetrics MockMetrics) { + t.Helper() -var MockHistograms map[string]float64 -var MockCounters map[string]int64 - -func CreateMockMetrics() *metrics.Metrics { - MockHistograms = make(map[string]float64, 6) - MockHistograms["puts.current_url.duration"] = 0.00 - MockHistograms["gets.current_url.duration"] = 0.00 - MockHistograms["puts.backends.request_duration"] = 0.00 - MockHistograms["puts.backends.request_size_bytes"] = 0.00 - MockHistograms["puts.backends.request_ttl_seconds"] = 0.00 - MockHistograms["gets.backends.duration"] = 0.00 - MockHistograms["connections.connections_opened"] = 0.00 + // All the names of our metric interface methods + allMetrics := map[string]struct{}{ + "RecordAcceptConnectionErrors": {}, + "RecordCloseConnectionErrors": {}, + "RecordConnectionClosed": {}, + "RecordConnectionOpen": {}, + "RecordGetBackendDuration": {}, + "RecordGetBackendError": {}, + "RecordGetBackendTotal": {}, + "RecordGetBadRequest": {}, + "RecordGetDuration": {}, + "RecordGetError": {}, + "RecordGetTotal": {}, + "RecordKeyNotFoundError": {}, + "RecordMissingKeyError": {}, + "RecordPutBackendDuration": {}, + "RecordPutBackendError": {}, + "RecordPutBackendInvalid": {}, + "RecordPutBackendJson": {}, + "RecordPutBackendSize": {}, + "RecordPutBackendTTLSeconds": {}, + "RecordPutBackendXml": {}, + "RecordPutBadRequest": {}, + "RecordPutDuration": {}, + "RecordPutError": {}, + "RecordPutKeyProvided": {}, + "RecordPutTotal": {}, + } - MockCounters = make(map[string]int64, 16) - MockCounters["puts.current_url.request.total"] = 0 - MockCounters["puts.current_url.request.error"] = 0 - MockCounters["puts.current_url.request.bad_request"] = 0 - MockCounters["puts.current_url.request.custom_key"] = 0 - MockCounters["gets.current_url.request.total"] = 0 - MockCounters["gets.current_url.request.error"] = 0 - MockCounters["gets.current_url.request.bad_request"] = 0 - MockCounters["puts.backends.add"] = 0 - MockCounters["puts.backends.json"] = 0 - MockCounters["puts.backends.xml"] = 0 - MockCounters["puts.backends.invalid_format"] = 0 - MockCounters["puts.backends.request.error"] = 0 - MockCounters["puts.backends.request.bad_request"] = 0 - MockCounters["gets.backends.request.total"] = 0 - MockCounters["gets.backends.request.error"] = 0 - MockCounters["gets.backends.request.bad_request"] = 0 - MockCounters["gets.backend_error.key_not_found"] = 0 - MockCounters["gets.backend_error.missing_key"] = 0 - MockCounters["connections.connection_error.accept"] = 0 - MockCounters["connections.connection_error.close"] = 0 + // Assert the metrics found in the expectedMetrics array where called. If a given element is not a known metric, throw error. + for _, metricName := range expectedMetrics { + _, exists := allMetrics[metricName] + if exists { + actualMetrics.AssertCalled(t, metricName) + delete(allMetrics, metricName) + } else { + t.Errorf("Cannot assert unrecognized metric '%s' was called", metricName) + } + } - return &metrics.Metrics{ - MetricEngines: []metrics.CacheMetrics{ - &MockMetrics{ - MetricsName: "Mockmetrics", - }, - }, + // Assert the metrics not found in the expectedMetrics array where not called + for metricName := range allMetrics { + actualMetrics.AssertNotCalled(t, metricName) } } -type MockMetrics struct { - MetricsName string +// MetricsRecorded is a structure used to document the exepected metrics to be recorded when running unit tests +type MetricsRecorded struct { + // Connection metrics + RecordAcceptConnectionErrors int64 `json:"RecordAcceptConnectionErrors"` + RecordCloseConnectionErrors int64 `json:"RecordCloseConnectionErrors"` + RecordConnectionClosed int64 `json:"RecordConnectionClosed"` + RecordConnectionOpen int64 `json:"RecordConnectionOpen"` + + // Get metrics + RecordGetBackendDuration float64 `json:"RecordGetBackendDuration"` + RecordGetBackendError int64 `json:"RecordGetBackendError"` + RecordGetBackendTotal int64 `json:"RecordGetBackendTotal"` + RecordGetBadRequest int64 `json:"RecordGetBadRequest"` + RecordGetDuration float64 `json:"RecordGetDuration"` + RecordGetError int64 `json:"RecordGetError"` + RecordGetTotal int64 `json:"RecordGetTotal"` + + // Put metrics + RecordKeyNotFoundError int64 `json:"RecordKeyNotFoundError"` + RecordMissingKeyError int64 `json:"RecordMissingKeyError"` + RecordPutBackendDuration float64 `json:"RecordPutBackendDuration"` + RecordPutBackendError int64 `json:"RecordPutBackendError"` + RecordPutBackendInvalid int64 `json:"RecordPutBackendInvalid"` + RecordPutBackendJson int64 `json:"RecordPutBackendJson"` + RecordPutBackendSize float64 `json:"RecordPutBackendSize"` + RecordPutBackendTTLSeconds float64 `json:"RecordPutBackendTTLSeconds"` + RecordPutBackendXml int64 `json:"RecordPutBackendXml"` + RecordPutBadRequest int64 `json:"RecordPutBadRequest"` + RecordPutDuration float64 `json:"RecordPutDuration"` + RecordPutError int64 `json:"RecordPutError"` + RecordPutKeyProvided int64 `json:"RecordPutKeyProvided"` + RecordPutTotal int64 `json:"RecordPutTotal"` } -func (m *MockMetrics) Export(cfg config.Metrics) { +func CreateMockMetrics() MockMetrics { + mockMetrics := MockMetrics{} + + mockMetrics.On("RecordAcceptConnectionErrors") + mockMetrics.On("RecordCloseConnectionErrors") + mockMetrics.On("RecordConnectionClosed") + mockMetrics.On("RecordConnectionOpen") + mockMetrics.On("RecordGetBackendDuration", mock.Anything) + mockMetrics.On("RecordGetBackendError") + mockMetrics.On("RecordGetBackendTotal") + mockMetrics.On("RecordGetBadRequest") + mockMetrics.On("RecordGetDuration", mock.Anything) + mockMetrics.On("RecordGetError") + mockMetrics.On("RecordGetTotal") + mockMetrics.On("RecordKeyNotFoundError") + mockMetrics.On("RecordMissingKeyError") + mockMetrics.On("RecordPutBackendDuration", mock.Anything) + mockMetrics.On("RecordPutBackendError") + mockMetrics.On("RecordPutBackendInvalid") + mockMetrics.On("RecordPutBackendJson") + mockMetrics.On("RecordPutBackendSize", mock.Anything) + mockMetrics.On("RecordPutBackendTTLSeconds", mock.Anything) + mockMetrics.On("RecordPutBackendXml") + mockMetrics.On("RecordPutBadRequest") + mockMetrics.On("RecordPutDuration", mock.Anything) + mockMetrics.On("RecordPutError") + mockMetrics.On("RecordPutKeyProvided") + mockMetrics.On("RecordPutTotal") + + return mockMetrics +} + +type MockMetrics struct { + mock.Mock } + +func (m *MockMetrics) Export(cfg config.Metrics) {} func (m *MockMetrics) GetEngineRegistry() interface{} { return nil } func (m *MockMetrics) GetMetricsEngineName() string { return "" } - func (m *MockMetrics) RecordPutError() { - MockCounters["puts.current_url.request.error"] = MockCounters["puts.current_url.request.error"] + 1 + m.Called() + return } func (m *MockMetrics) RecordPutBadRequest() { - MockCounters["puts.current_url.request.bad_request"] = MockCounters["puts.current_url.request.bad_request"] + 1 + m.Called() + return } func (m *MockMetrics) RecordPutTotal() { - MockCounters["puts.current_url.request.total"] = MockCounters["puts.current_url.request.total"] + 1 + m.Called() + return } func (m *MockMetrics) RecordPutDuration(duration time.Duration) { - MockHistograms["puts.current_url.duration"] = mockDuration.Seconds() + m.Called() + return } func (m *MockMetrics) RecordPutKeyProvided() { - MockCounters["puts.current_url.request.custom_key"] = MockCounters["puts.current_url.request.custom_key"] + 1 + m.Called() + return } func (m *MockMetrics) RecordGetError() { - MockCounters["gets.current_url.request.error"] = MockCounters["gets.current_url.request.error"] + 1 + m.Called() + return } func (m *MockMetrics) RecordGetBadRequest() { - MockCounters["gets.current_url.request.bad_request"] = MockCounters["gets.current_url.request.bad_request"] + 1 + m.Called() + return } func (m *MockMetrics) RecordGetTotal() { - MockCounters["gets.current_url.request.total"] = MockCounters["gets.current_url.request.total"] + 1 + m.Called() + return } func (m *MockMetrics) RecordGetDuration(duration time.Duration) { - MockHistograms["gets.current_url.duration"] = mockDuration.Seconds() + m.Called() + return } func (m *MockMetrics) RecordPutBackendXml() { - MockCounters["puts.backends.xml"] = MockCounters["puts.backends.xml"] + 1 + m.Called() + return } func (m *MockMetrics) RecordPutBackendJson() { - MockCounters["puts.backends.json"] = MockCounters["puts.backends.json"] + 1 + m.Called() + return } func (m *MockMetrics) RecordPutBackendInvalid() { - MockCounters["puts.backends.invalid_format"] = MockCounters["puts.backends.invalid_format"] + 1 + m.Called() + return } func (m *MockMetrics) RecordPutBackendDuration(duration time.Duration) { - MockHistograms["puts.backends.request_duration"] = mockDuration.Seconds() + m.Called() + return } func (m *MockMetrics) RecordPutBackendError() { - MockCounters["puts.backends.request.error"] = MockCounters["puts.backends.request.error"] + 1 + m.Called() + return } func (m *MockMetrics) RecordPutBackendSize(sizeInBytes float64) { - MockHistograms["puts.backends.request_size_bytes"] = sizeInBytes + m.Called() + return } func (m *MockMetrics) RecordPutBackendTTLSeconds(duration time.Duration) { - MockHistograms["puts.backends.request_ttl_seconds"] = mockDuration.Seconds() + m.Called() + return } func (m *MockMetrics) RecordGetBackendDuration(duration time.Duration) { - MockHistograms["gets.backends.duration"] = mockDuration.Seconds() + m.Called() + return } func (m *MockMetrics) RecordGetBackendTotal() { - MockCounters["gets.backends.request.total"] = MockCounters["gets.backends.request.total"] + 1 + m.Called() + return } func (m *MockMetrics) RecordGetBackendError() { - MockCounters["gets.backends.request.error"] = MockCounters["gets.backends.request.error"] + 1 + m.Called() + return } func (m *MockMetrics) RecordKeyNotFoundError() { - MockCounters["gets.backend_error.key_not_found"] = MockCounters["gets.backend_error.key_not_found"] + 1 + m.Called() + return } func (m *MockMetrics) RecordMissingKeyError() { - MockCounters["gets.backend_error.missing_key"] = MockCounters["gets.backend_error.missing_key"] + 1 + m.Called() + return } func (m *MockMetrics) RecordConnectionOpen() { - MockHistograms["connections.connections_opened"] = MockHistograms["connections.connections_opened"] + 1 + m.Called() + return } func (m *MockMetrics) RecordConnectionClosed() { - MockHistograms["connections.connections_opened"] = MockHistograms["connections.connections_opened"] - 1 + m.Called() + return } func (m *MockMetrics) RecordCloseConnectionErrors() { - MockCounters["connections.connection_error.close"] = MockCounters["connections.connection_error.close"] + 1 + m.Called() + return } func (m *MockMetrics) RecordAcceptConnectionErrors() { - MockCounters["connections.connection_error.accept"] = MockCounters["connections.connection_error.accept"] + 1 + m.Called() + return } diff --git a/server/listener_test.go b/server/listener_test.go index abe5b180..047e36c7 100644 --- a/server/listener_test.go +++ b/server/listener_test.go @@ -6,56 +6,78 @@ import ( "testing" "time" + "github.com/prebid/prebid-cache/metrics" "github.com/prebid/prebid-cache/metrics/metricstest" "github.com/stretchr/testify/assert" ) -func TestNormalConnectionMetrics(t *testing.T) { - doTest(t, true, true) -} - -func TestAcceptErrorMetrics(t *testing.T) { - doTest(t, false, false) -} - -func TestCloseErrorMetrics(t *testing.T) { - doTest(t, true, false) -} - -func doTest(t *testing.T, allowAccept bool, allowClose bool) { - m := metricstest.CreateMockMetrics() - - var listener net.Listener = &mockListener{ - listenSuccess: allowAccept, - closeSuccess: allowClose, +func TestConnections(t *testing.T) { + testCases := []struct { + desc string + allowAccept, allowClose bool + expectedConnectionError error + expectedMetrics []string + }{ + { + desc: "net.Listener will fail when attempting to open a connection. Expect error and RecordAcceptConnectionErrors to be called", + allowAccept: false, + allowClose: false, + expectedConnectionError: errors.New("Failed to open connection"), + expectedMetrics: []string{ + "RecordAcceptConnectionErrors", + }, + }, + { + desc: "net.Listener will fail when attempting to open a connection. Expect error and RecordAcceptConnectionErrors to be called", + allowAccept: false, + allowClose: true, + expectedConnectionError: errors.New("Failed to open connection"), + expectedMetrics: []string{ + "RecordAcceptConnectionErrors", + }, + }, + { + desc: "net.Listener will open and close connections successfully. Expect ConnectionOpen and ConnectionClosed metrics to be logged", + allowAccept: true, + allowClose: true, + expectedConnectionError: nil, + expectedMetrics: []string{ + "RecordConnectionOpen", + "RecordConnectionClosed", + }, + }, + { + desc: "net.Listener will open a connection but will fail when trying to close it. Expect ConnectionOpen and a CloseConnectionErrors to be accounted for in the metrics", + allowAccept: true, + allowClose: false, + expectedConnectionError: errors.New("Failed to close connection."), + expectedMetrics: []string{ + "RecordCloseConnectionErrors", + "RecordConnectionOpen", + }, + }, } - listener = &monitorableListener{listener, m} - conn, err := listener.Accept() - if !allowAccept { - if err == nil { - t.Error("The listener.Accept() error should propagate from the underlying listener.") + for _, tc := range testCases { + mockMetrics := metricstest.CreateMockMetrics() + m := &metrics.Metrics{ + MetricEngines: []metrics.CacheMetrics{ + &mockMetrics, + }, } - assert.Equal(t, metricstest.MockHistograms["connections.connections_opened"], 0.00, "Should not log any connections") - assert.Equal(t, int64(1), metricstest.MockCounters["connections.connection_error.accept"], "Metrics engine should not log an accept connection error") - assert.Equal(t, int64(0), metricstest.MockCounters["connections.connection_error.close"], "Metrics engine should have logged a close connection error") - return - } - assert.Equal(t, int64(0), metricstest.MockCounters["connections.connection_error.accept"], "Metrics engine should not log an accept connection error") - assert.Equal(t, metricstest.MockHistograms["connections.connections_opened"], 1.00, "Should not log any connections") - - err = conn.Close() - if allowClose { - assert.Equal(t, metricstest.MockHistograms["connections.connections_opened"], 0.00, "Should not log any connections") - assert.Equal(t, int64(0), metricstest.MockCounters["connections.connection_error.accept"], "Metrics engine should not log an accept connection error") - assert.Equal(t, int64(0), metricstest.MockCounters["connections.connection_error.close"], "Metrics engine should have logged a close connection error") - } else { - if err == nil { - t.Error("The connection.Close() error should propagate from the underlying listener.") + + var listener net.Listener = &mockListener{ + listenSuccess: tc.allowAccept, + closeSuccess: tc.allowClose, + } + + listener = &monitorableListener{listener, m} + conn, err := listener.Accept() + if tc.allowAccept { + err = conn.Close() } - assert.Equal(t, metricstest.MockHistograms["connections.connections_opened"], 1.00, "Should not log any connections") - assert.Equal(t, int64(0), metricstest.MockCounters["connections.connection_error.accept"], "Metrics engine should not log an accept connection error") - assert.Equal(t, int64(1), metricstest.MockCounters["connections.connection_error.close"], "Metrics engine should have logged a close connection error") + assert.Equal(t, tc.expectedConnectionError, err, tc.desc) + metricstest.AssertMetrics(t, tc.expectedMetrics, mockMetrics) } }