Skip to content

Commit

Permalink
Bugfix Json Schema, Improve json schema tests
Browse files Browse the repository at this point in the history 
* Fix bugs uncovered with improved tests
* Do automatic validation for keyword limitations
* Remove files for invalid payloads, instead
  create invalid payloads out of valid payload
* Allow to define simple and conditional rules for testing payloads.
  • Loading branch information
@simitt
simitt committed Jun 19, 2018
1 parent b723f5e commit ff076ff
Show file tree
Hide file tree
Showing 62 changed files with 646 additions and 747 deletions.
5 changes: 2 additions & 3 deletions docs/spec/context.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,8 @@
"description": "A flat mapping of user-defined tags with string values.",
"regexProperties": true,
"patternProperties": {
"^[^.*\"]*$": {
"type": "string",
"maxLength": 1024
"^[^.*\"]{0,512}[^.*\"]{0,512}$": {
"type": ["string", "null"]
}
},
"additionalProperties": false
Expand Down
4 changes: 2 additions & 2 deletions docs/spec/errors/error.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,7 +91,7 @@
"required": ["message"]
},
"timestamp": {
"type": "string",
"type": ["string","null"],
"format": "date-time",
"pattern": "Z$",
"description": "Recorded time of the error, UTC based and formatted as YYYY-MM-DDTHH:mm:ss.sssZ"
Expand All @@ -101,7 +101,7 @@
"description": "Data for correlating errors with transactions",
"properties": {
"id": {
"type": "string",
"type": ["string", "null"],
"description": "UUID for the transaction",
"pattern": "^[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}$"
}
Expand Down
4 changes: 3 additions & 1 deletion docs/spec/transactions/mark.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,9 @@
"description": "A mark captures the timing in milliseconds of a significant event during the lifetime of a transaction. Every mark is a simple key value pair, where the value has to be a number, and can be set by the user or the agent.",
"regexProperties": true,
"patternProperties": {
"^[^.*\"]*$": { "type": "number" }
"^[^.*\"]{0,512}[^.*\"]{0,512}$": {
"type": ["number", "null"]
}
},
"additionalProperties": false
}
9 changes: 4 additions & 5 deletions docs/spec/transactions/transaction.json
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,12 +22,12 @@
"maxLength": 1024
},
"result": {
"type": "string",
"type": ["string", "null"],
"description": "The result of the transaction. For HTTP-related transactions, this should be the status code formatted like 'HTTP 2xx'.",
"maxLength": 1024
},
"timestamp": {
"type": "string",
"type": ["string", "null"],
"pattern": "Z$",
"format": "date-time",
"description": "Recorded time of the transaction, UTC based and formatted as YYYY-MM-DDTHH:mm:ss.sssZ"
Expand All @@ -49,9 +49,8 @@
"description": "A mark captures the timing of a significant event during the lifetime of a transaction. Marks are organized into groups and can be set by the user or the agent.",
"regexProperties": true,
"patternProperties": {
"^[^.*\"]*$": {
"$ref": "mark.json",
"maxLength": 1024
"^[^.*\"]{0,512}[^.*\"]{0,512}$": {
"$ref": "mark.json"
}
},
"additionalProperties": false
Expand Down
129 changes: 102 additions & 27 deletions processor/error/package_tests/json_schema_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,108 @@ import (
"github.com/elastic/apm-server/tests"
)

func TestTransactionJsonSchema(t *testing.T) {
type obj = map[string]interface{}

testData := tests.JsonSchemaTestData{
Proc: er.NewProcessor(),
RequiredKeys: tests.NewSet(
"errors",
"errors.exception.message",
"errors.log.message",
"errors.exception.stacktrace.filename",
"errors.exception.stacktrace.lineno",
"errors.log.stacktrace.filename",
"errors.log.stacktrace.lineno",
"errors.context.request.method",
"errors.context.request.url",
),
ConditionalRequiredKeys: map[string]tests.Condition{
"errors.exception": tests.Condition{Absence: []string{"errors.log"}},
"errors.log": tests.Condition{Absence: []string{"errors.exception"}},
},
KeywordExceptionKeys: tests.NewSet(
"processor.event", "processor.name", "listening",
"error.grouping_key", "error.id", "transaction.id",
"context.tags", "view errors", "error id icon"),
TemplateToSchemaMapping: map[string]string{
"context.system.": "system.",
"context.process.": "process.",
"context.service.": "service.",
"context.request.": "errors.context.request.",
"context.user.": "errors.context.user.",
"span.": "errors.spans.",
"error.": "errors.",
},
LoadTemplatePaths: []string{
"./../../../_meta/fields.common.yml",
"./../_meta/fields.yml"},
TestData: []tests.TestData{
// add test data for testing
// * specific edge cases
// * multiple allowed dataypes
// * regex pattern, time formats
// * length restrictions, other than keyword length restrictions

{Key: "service.name", Invalid: []interface{}{tests.Str1024Special, tests.Str1025}, Valid: []interface{}{tests.Str1024}},
{Key: "errors", Invalid: []interface{}{"errors", []interface{}{}}},
{Key: "errors.id",
Invalid: []interface{}{"123", "z5925e55-b43f-4340-a8e0-df1906ecbf7a", "z5925e55-b43f-4340-a8e0-df1906ecbf7ia"},
Valid: []interface{}{"85925e55-B43f-4340-a8e0-df1906ecbf7a"}},
{Key: "errors.exception.code", Invalid: []interface{}{false}, Valid: []interface{}{"sucess", 200}},
{Key: "errors.exception.attributes", Invalid: []interface{}{123}, Valid: []interface{}{map[string]interface{}{}}},
{Key: "errors.transaction.id",
Invalid: []interface{}{"123", "z5925e55-b43f-4340-a8e0-df1906ecbf7a", "z5925e55-b43f-4340-a8e0-df1906ecbf7ia"},
Valid: []interface{}{"85925e55-B43f-4340-a8e0-df1906ecbf7a"}},
{Key: "errors.timestamp",
Invalid: []interface{}{
"2017-05-30T18:53Z", "2017-05-30T18:53:27.000+00:20", "2017-05-30T18:53:27.Z",
"2017-05-30T18:53:27a123Z", "2017-05-30T18:53:27ZNOTCORRECT",
},
Valid: []interface{}{"2017-05-30T18:53:42.281Z"}},
{Key: "errors.log.stacktrace.post_context",
Invalid: []interface{}{[]interface{}{123}, "test"},
Valid: []interface{}{[]interface{}{}, []interface{}{"context"}}},
{Key: "errors.log.stacktrace.pre_context",
Invalid: []interface{}{[]interface{}{123}, "test"},
Valid: []interface{}{[]interface{}{}, []interface{}{"context"}}},
{Key: "errors.exception.stacktrace.post_context",
Invalid: []interface{}{[]interface{}{123}, "test"},
Valid: []interface{}{[]interface{}{}, []interface{}{"context"}}},
{Key: "errors.exception.stacktrace.pre_context",
Invalid: []interface{}{[]interface{}{123}, "test"},
Valid: []interface{}{[]interface{}{}, []interface{}{"context"}}},
{Key: "errors.context.custom",
Invalid: []interface{}{"context",
obj{"what.ever": 123},
obj{"what*ever": 123},
obj{"what\"ever": 123},
map[int]interface{}{123: 123},
},
Valid: []interface{}{
obj{"whatever": obj{"comes": obj{"end": -45}}},
obj{"whatever": 123},
}},
{Key: "errors.context.request.body", Invalid: []interface{}{102}, Valid: []interface{}{obj{}, tests.Str1025}},
{Key: "errors.context.request.env", Invalid: []interface{}{102, "a"}, Valid: []interface{}{obj{}}},
{Key: "errors.context.request.cookies", Invalid: []interface{}{123, ""}, Valid: []interface{}{obj{}}},
{Key: "errors.context.tags",
Invalid: []interface{}{"tags",
obj{tests.Str1025: "hello"},
obj{"invali*d": "hello"},
obj{"invali\"d": "hello"},
obj{"invali.d": "hello"},
obj{tests.Str1024Special: 123},
obj{tests.Str1024Special: obj{}},
},
Valid: []interface{}{obj{tests.Str1024Special: "hello"}},
},
{Key: "errors.context.user.id", Invalid: []interface{}{tests.Str1025, obj{}}, Valid: []interface{}{123, tests.Str1024Special}},
},
}
testData.TestJsonSchema(t)
}

//Check whether attributes are added to the example payload but not to the schema
func TestPayloadAttributesInSchema(t *testing.T) {
//only add attributes that should not be documented by the schema
Expand All @@ -28,30 +130,3 @@ func TestPayloadAttributesInSchema(t *testing.T) {
)
tests.TestPayloadAttributesInSchema(t, "error", undocumented, er.Schema())
}

func TestJsonSchemaKeywordLimitation(t *testing.T) {
fieldsPaths := []string{
"./../../../_meta/fields.common.yml",
"./../_meta/fields.yml",
}
exceptions := tests.NewSet(
"processor.event",
"processor.name",
"error.id",
"error.log.level",
"error.grouping_key",
"transaction.id",
"listening",
"error id icon",
"view errors",
)
tests.TestJsonSchemaKeywordLimitation(t, fieldsPaths, er.Schema(), exceptions)
}

func TestErrorPayloadSchema(t *testing.T) {
testData := []tests.SchemaTestData{
{File: "data/invalid/error_payload/no_service.json", Error: "missing properties: \"service\""},
{File: "data/invalid/error_payload/no_errors.json", Error: "missing properties: \"errors\""},
}
tests.TestDataAgainstProcessor(t, er.NewProcessor(), testData)
}
9 changes: 4 additions & 5 deletions processor/error/schema.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -288,9 +288,8 @@ var errorSchema = `{
"description": "A flat mapping of user-defined tags with string values.",
"regexProperties": true,
"patternProperties": {
"^[^.*\"]*$": {
"type": "string",
"maxLength": 1024
"^[^.*\"]{0,512}[^.*\"]{0,512}$": {
"type": ["string", "null"]
}
},
"additionalProperties": false
Expand Down Expand Up @@ -525,7 +524,7 @@ var errorSchema = `{
"required": ["message"]
},
"timestamp": {
"type": "string",
"type": ["string","null"],
"format": "date-time",
"pattern": "Z$",
"description": "Recorded time of the error, UTC based and formatted as YYYY-MM-DDTHH:mm:ss.sssZ"
Expand All @@ -535,7 +534,7 @@ var errorSchema = `{
"description": "Data for correlating errors with transactions",
"properties": {
"id": {
"type": "string",
"type": ["string", "null"],
"description": "UUID for the transaction",
"pattern": "^[a-fA-F0-9]{8}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{4}-[a-fA-F0-9]{12}$"
}
Expand Down
18 changes: 16 additions & 2 deletions processor/sourcemap/package_tests/json_schema_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,8 +3,11 @@ package package_tests
import (
"testing"

"github.com/stretchr/testify/assert"

"github.com/elastic/apm-server/processor/sourcemap"
"github.com/elastic/apm-server/tests"
"github.com/elastic/apm-server/tests/loader"
)

//Check whether attributes are added to the example payload but not to the schema
Expand All @@ -18,11 +21,22 @@ func TestPayloadAttributesInSchema(t *testing.T) {
}

func TestSourcemapPayloadSchema(t *testing.T) {
testData := []tests.SchemaTestData{
testData := []struct {
File string
Error string
}{
{File: "data/invalid/sourcemap/no_service_version.json", Error: "missing properties: \"service_version\""},
{File: "data/invalid/sourcemap/no_bundle_filepath.json", Error: "missing properties: \"bundle_filepath\""},
{File: "data/invalid/sourcemap/not_allowed_empty_values.json", Error: "length must be >= 1, but got 0"},
{File: "data/invalid/sourcemap/not_allowed_null_values.json", Error: "expected string, but got null"},
}
tests.TestDataAgainstProcessor(t, sourcemap.NewProcessor(), testData)

for _, d := range testData {
data, err := loader.LoadData(d.File)
assert.Nil(t, err)
err = sourcemap.NewProcessor().Validate(data)
if assert.Error(t, err) {
assert.Contains(t, err.Error(), d.Error)
}
}
}
Loading

0 comments on commit ff076ff

Please sign in to comment.