Make jsonschema validation return the exact field name in the error m…

…essage as it is input'ed by the user, as per @roncohen code review. This requires to flatten the app attributes, so they are no longer reusable. This reverts commit b7174c2.
elastic · Nov 22, 2017 · d4ab32d · d4ab32d
1 parent ffd280f
commit d4ab32d
Show file tree

Hide file tree

Showing 33 changed files with 451 additions and 579 deletions.
diff --git a/docs/fields.asciidoc b/docs/fields.asciidoc
@@ -454,22 +454,15 @@ Sourcemap files enriched with metadata
 
 
 [float]
-== app fields
-
-App fields.
-
-
-
-[float]
-=== `sourcemap.app.name`
+=== `sourcemap.app_name`
 
 type: keyword
 
 The name of the app this sourcemap belongs to.
 
 
 [float]
-=== `sourcemap.app.version`
+=== `sourcemap.app_version`
 
 type: keyword
 

diff --git a/docs/spec/app.json b/docs/spec/app.json
@@ -3,118 +3,85 @@
     "$id": "doc/spec/app.json",
     "title": "App",
     "type": "object",
-    "allOf": [
-        {"$ref": "app_name.json"},
-        {"properties": {
-            "version": {
-                "description": "Version of the app emitting this event",
-                "type": [
-                    "string",
-                    "null"
-                ],
-                "maxLength": 1024
-            },
-            "agent": {
-                "type": "object",
-                "properties": {
-                    "name": {
-                        "type": "string",
-                        "maxLength": 1024
-                    },
-                    "version": {
-                        "type": "string",
-                        "maxLength": 1024
-                    }
+    "properties": {
+        "agent": {
+            "type": "object",
+            "properties": {
+                "name": {
+                    "type": "string",
+                    "maxLength": 1024
                 },
-                "required": [
-                    "name",
-                    "version"
-                ]
-            },
-            "argv": {
-                "type": [
-                    "array",
-                    "null"
-                ],
-                "minItems": 0
+                "version": {
+                    "type": "string",
+                    "maxLength": 1024
+                }
             },
-            "framework": {
-                "type": [
-                    "object",
-                    "null"
-                ],
-                "properties": {
-                    "name": {
-                        "type": "string",
-                        "maxLength": 1024
-                    },
-                    "version": {
-                        "type": "string",
-                        "maxLength": 1024
-                    }
+            "required": ["name", "version"]
+        },
+        "argv": {
+            "type": ["array", "null"],
+            "minItems": 0
+        },
+        "framework": {
+            "type": ["object", "null"],
+            "properties": {
+                "name": {
+                    "type": "string",
+                    "maxLength": 1024
                 },
-                "required": [
-                    "name",
-                    "version"
-                ]
+                "version": {
+                    "type": "string",
+                    "maxLength": 1024
+                }
             },
-            "language": {
-                "type": [
-                    "object",
-                    "null"
-                ],
-                "properties": {
-                    "name": {
-                        "type": "string",
-                        "maxLength": 1024
-                    },
-                    "version": {
-                        "type": [
-                            "string",
-                            "null"
-                        ],
-                        "maxLength": 1024
-                    }
+            "required": ["name", "version"]
+        },
+        "language": {
+            "type": ["object", "null"],
+            "properties": {
+                "name": {
+                    "type": "string",
+                    "maxLength": 1024
                 },
-                "required": [
-                    "name"
-                ]
-            },
-            "pid": {
-                "type": [
-                    "number",
-                    "null"
-                ]
+                "version": {
+                    "type": ["string", "null"],
+                    "maxLength": 1024
+                }
             },
-            "process_title": {
-                "type": [
-                    "string",
-                    "null"
-                ],
-                "maxLength": 1024
-            },
-            "runtime": {
-                "type": [
-                    "object",
-                    "null"
-                ],
-                "properties": {
-                    "name": {
-                        "type": "string",
-                        "maxLength": 1024
-                    },
-                    "version": {
-                        "type": "string",
-                        "maxLength": 1024
-                    }
+            "required": ["name"]
+        },
+        "name": {
+            "description": "Immutable name of the app emitting this event",
+            "type": "string",
+            "pattern": "^[a-zA-Z0-9 _-]+$",
+            "maxLength": 1024
+        },
+        "pid": {
+            "type": ["number", "null"]
+        },
+        "process_title": {
+            "type": ["string", "null"],
+            "maxLength": 1024
+        },
+        "runtime": {
+            "type": ["object", "null"],
+            "properties": {
+                "name": {
+                    "type": "string",
+                    "maxLength": 1024
                 },
-                "required": [
-                    "name",
-                    "version"
-                ]
-            }
+                "version": {
+                    "type": "string",
+                    "maxLength": 1024
+                }
+            },
+            "required": ["name", "version"]
         },
-        "required": ["agent"]
+        "version": {
+            "description": "Version of the app emitting this event",
+            "type": ["string", "null"],
+            "maxLength": 1024
         }
-    ]
+    },
+    "required": ["agent", "name"]
 }
diff --git a/docs/spec/app_name.json b/docs/spec/app_name.json
diff --git a/docs/spec/sourcemaps/payload.json b/docs/spec/sourcemaps/payload.json
@@ -1,8 +1,8 @@
 {
     "$schema": "http://json-schema.org/draft-04/schema#",
-    "$id": "docs/spec/sourcemaps/sourcemap-wrapper.json",
-    "title": "Sourcemap Wrapper",
-    "description": "Sourcemap + Metadata",
+    "$id": "docs/spec/sourcemaps/sourcemap-metadata.json",
+    "title": "Sourcemap Metadata",
+    "description": "Sourcemap Metadata",
     "type": "object",
     "properties": {
         "bundle_filepath": {
@@ -11,21 +11,19 @@
             "maxLength": 1024,
             "minLength": 1
         },
-        "app": {
-            "allOf": [
-                { "$ref": "../app_name.json" },
-                { "properties": {
-                    "version": {
-                        "description": "Version of the app emitting this event",
-                        "type": "string",
-                        "maxLength": 1024,
-                        "minLength": 1
-                    }
-                },
-                "required": ["version"]
-                }
-            ]
+        "app_version": {
+            "description": "Version of the app emitting this event",
+            "type": "string",
+            "maxLength": 1024,
+            "minLength": 1
+        },
+        "app_name": {
+            "description": "Immutable name of the app emitting this event",
+            "type": "string",
+            "pattern": "^[a-zA-Z0-9 _-]+$",
+            "maxLength": 1024,
+            "minLength": 1
         }
     },
-    "required": ["bundle_filepath"]
+    "required": ["bundle_filepath", "app_name", "app_version"]
 }
diff --git a/model/app.go b/model/app.go
@@ -5,13 +5,9 @@ import (
 	"github.com/elastic/beats/libbeat/common"
 )
 
-type AppCore struct {
-	Name    string  `json:"name"`
-	Version *string `json:"version"`
-}
-
 type App struct {
-	AppCore
+	Name         string    `json:"name"`
+	Version      *string   `json:"version"`
 	Pid          *int      `json:"pid"`
 	ProcessTitle *string   `json:"process_title"`
 	Argv         []string  `json:"argv"`
@@ -51,26 +47,14 @@ func (a *App) MinimalTransform() common.MapStr {
 	return app
 }
 
-func (a *AppCore) Transform() common.MapStr {
-	enhancer := utility.NewMapStrEnhancer()
-	app := common.MapStr{"name": a.Name}
-	enhancer.Add(app, "version", a.Version)
-	return app
-}
-
 func (a *App) Transform() common.MapStr {
 	enhancer := utility.NewMapStrEnhancer()
-	app := a.AppCore.Transform()
-
+	app := a.MinimalTransform()
+	enhancer.Add(app, "version", a.Version)
 	enhancer.Add(app, "pid", a.Pid)
 	enhancer.Add(app, "process_title", a.ProcessTitle)
 	enhancer.Add(app, "argv", a.Argv)
 
-	agent := common.MapStr{}
-	enhancer.Add(agent, "name", a.Agent.Name)
-	enhancer.Add(agent, "version", a.Agent.Version)
-	enhancer.Add(app, "agent", agent)
-
 	lang := common.MapStr{}
 	enhancer.Add(lang, "name", a.Language.Name)
 	enhancer.Add(lang, "version", a.Language.Version)

diff --git a/model/app_test.go b/model/app_test.go
@@ -43,7 +43,8 @@ func TestAppTransform(t *testing.T) {
 		},
 		{
 			App: App{
-				AppCore:      AppCore{Name: "myapp", Version: &version},
+				Name:         "myapp",
+				Version:      &version,
 				Pid:          &pid,
 				ProcessTitle: &processTitle,
 				Argv: []string{

diff --git a/processor/error/package_tests/json_schema_test.go b/processor/error/package_tests/json_schema_test.go
@@ -5,6 +5,7 @@ import (
 
 	"github.com/fatih/set"
 
+	er "github.com/elastic/apm-server/processor/error"
 	"github.com/elastic/apm-server/tests"
 )
 
@@ -27,5 +28,23 @@ func TestPayloadAttributesInSchema(t *testing.T) {
 		"errors.context.request.cookies.c2",
 		"errors.context.tags.organization_uuid",
 	)
-	tests.TestPayloadAttributesInSchema(t, "error/payload.json", undocumented, "errors/payload.json")
+	tests.TestPayloadAttributesInSchema(t, "error", undocumented, er.Schema())
+}
+
+func TestJsonSchemaKeywordLimitation(t *testing.T) {
+	fieldsPaths := []string{
+		"./../../../_meta/fields.common.yml",
+		"./../_meta/fields.yml",
+	}
+	exceptions := set.New(
+		"processor.event",
+		"processor.name",
+		"error.id",
+		"error.log.level",
+		"error.grouping_key",
+		"listening",
+		"error id icon",
+		"view errors",
+	)
+	tests.TestJsonSchemaKeywordLimitation(t, fieldsPaths, er.Schema(), exceptions)
 }
diff --git a/processor/error/payload_test.go b/processor/error/payload_test.go
@@ -13,7 +13,7 @@ import (
 )
 
 func TestPayloadTransform(t *testing.T) {
-	app := m.App{AppCore: m.AppCore{Name: "myapp"}}
+	app := m.App{Name: "myapp"}
 	timestamp := time.Now()
 
 	tests := []struct {