git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / Atom feed
* [RFC PATCH 0/3] Add a JSON Schema for trace2 events
@ 2019-06-11 23:31 Josh Steadmon
  2019-06-11 23:31 ` [RFC PATCH 1/3] trace2: correct trace2 field name documentation Josh Steadmon
                   ` (2 more replies)
  0 siblings, 3 replies; 14+ messages in thread
From: Josh Steadmon @ 2019-06-11 23:31 UTC (permalink / raw)
  To: git, git

This is a proof of concept series that formalizes the structure of trace2 event
output using JSON-Schema [1].

It provides a validator (written in Go) that verifies the events in a given
trace2 event output file match the schema. I am happy to rewrite this validator
in some other language, provided that the language has a JSON-Schema library
supporting at least draft-04.

In the long term, I think it would be useful to add a regression test that
collects the trace2 output of the entire test suite and runs the validator
against that. On my machine, this takes about 15 minutes of run time to validate
the 1.7M events created by a full test run.

If such a regression test is in place, users of the trace2 event output can be
relatively confident that the output format has not changed so long as the
schema file remains the same and the regression test is passing.

[1]: https://json-schema.org/

Josh Steadmon (3):
  trace2: correct trace2 field name documentation
  trace2: Add a JSON schema for trace2 events
  trace2: add a schema validator for trace2 events

 Documentation/technical/api-trace2.txt        |   2 +-
 t/trace_schema_validator/.gitignore           |   1 +
 t/trace_schema_validator/Makefile             |  10 +
 t/trace_schema_validator/event_schema.json    | 398 ++++++++++++++
 t/trace_schema_validator/strict_schema.json   | 511 ++++++++++++++++++
 .../trace_schema_validator.go                 |  74 +++
 6 files changed, 995 insertions(+), 1 deletion(-)
 create mode 100644 t/trace_schema_validator/.gitignore
 create mode 100644 t/trace_schema_validator/Makefile
 create mode 100644 t/trace_schema_validator/event_schema.json
 create mode 100644 t/trace_schema_validator/strict_schema.json
 create mode 100644 t/trace_schema_validator/trace_schema_validator.go

-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH 1/3] trace2: correct trace2 field name documentation
  2019-06-11 23:31 [RFC PATCH 0/3] Add a JSON Schema for trace2 events Josh Steadmon
@ 2019-06-11 23:31 ` Josh Steadmon
  2019-06-12 18:00   ` Junio C Hamano
  2019-06-14 15:53   ` Jeff Hostetler
  2019-06-11 23:31 ` [RFC PATCH 2/3] trace2: Add a JSON schema for trace2 events Josh Steadmon
  2019-06-11 23:31 ` [RFC PATCH 3/3] trace2: add a schema validator " Josh Steadmon
  2 siblings, 2 replies; 14+ messages in thread
From: Josh Steadmon @ 2019-06-11 23:31 UTC (permalink / raw)
  To: git, git

Correct the api-trace2 documentation, which lists "signal" as an
expected field for the signal event type, but which actually outputs
"signo" as the field name.

Signed-off-by: Josh Steadmon <steadmon@google.com>
---
 Documentation/technical/api-trace2.txt | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Documentation/technical/api-trace2.txt b/Documentation/technical/api-trace2.txt
index 23c3cc7a37..fd1e628944 100644
--- a/Documentation/technical/api-trace2.txt
+++ b/Documentation/technical/api-trace2.txt
@@ -668,7 +668,7 @@ completed.)
 	"event":"signal",
 	...
 	"t_abs":0.001227,  # elapsed time in seconds
-	"signal":13        # SIGTERM, SIGINT, etc.
+	"signo":13         # SIGTERM, SIGINT, etc.
 }
 ------------
 
-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH 2/3] trace2: Add a JSON schema for trace2 events
  2019-06-11 23:31 [RFC PATCH 0/3] Add a JSON Schema for trace2 events Josh Steadmon
  2019-06-11 23:31 ` [RFC PATCH 1/3] trace2: correct trace2 field name documentation Josh Steadmon
@ 2019-06-11 23:31 ` Josh Steadmon
  2019-06-14 15:59   ` Jeff Hostetler
  2019-06-11 23:31 ` [RFC PATCH 3/3] trace2: add a schema validator " Josh Steadmon
  2 siblings, 1 reply; 14+ messages in thread
From: Josh Steadmon @ 2019-06-11 23:31 UTC (permalink / raw)
  To: git, git

Define a JSON schema[1] that can be used to validate trace2 event
objects. This can be used to add regression tests to verify that the
event output format does not change unexpectedly.

Two versions of the schema are provided:
* event_schema.json is more permissive. It verifies that all expected
  fields are present in each trace event, but it allows traces to have
  unexpected additional fields. This allows the schema to be specified
  more concisely by factoring out the common fields into a reusable
  sub-schema.
* strict_schema.json is more restrictive. It verifies that all expected
  fields are present and no unexpected fields are present in each trace
  event. Due to this additional restriction, the common fields cannot be
  factored out into a re-usable subschema (at least as-of draft-07) [2],
  and must be repeated for each event definition.

[1]: https://json-schema.org/
[2]: https://json-schema.org/understanding-json-schema/reference/combining.html#allof

Signed-off-by: Josh Steadmon <steadmon@google.com>
---
 t/trace_schema_validator/event_schema.json  | 398 +++++++++++++++
 t/trace_schema_validator/strict_schema.json | 511 ++++++++++++++++++++
 2 files changed, 909 insertions(+)
 create mode 100644 t/trace_schema_validator/event_schema.json
 create mode 100644 t/trace_schema_validator/strict_schema.json

diff --git a/t/trace_schema_validator/event_schema.json b/t/trace_schema_validator/event_schema.json
new file mode 100644
index 0000000000..5f6cfb11e7
--- /dev/null
+++ b/t/trace_schema_validator/event_schema.json
@@ -0,0 +1,398 @@
+{
+	"$schema": "http://json-schema.org/draft-07/schema#",
+	"$id": "http://git-scm.com/schemas/event_schema.json",
+	"title": "trace2 permissive schema",
+	"description": "Permissive schema for trace2 event output that does not fail in the presence of unexpected fields.",
+
+	"definitions": {
+		"event_common_fields": {
+			"type": "object",
+			"properties": {
+				"sid": { "type": "string" },
+				"thread": { "type": "string" },
+				"time": {
+					"type": "string",
+					"format": "date-time"
+				},
+				"file": { "type": "string" },
+				"line": { "type": "integer" },
+				"repo": { "type": "integer" }
+			},
+			"required": [ "sid", "thread" ]
+		},
+
+		"version_event": {
+			"allOf": [
+				{ "$ref": "#/definitions/event_common_fields" },
+				{
+					"properties": {
+						"event": { "const": "version" },
+						"evt": { "const": "1" },
+						"exe": { "type": "string" }
+					},
+					"required": [ "event", "evt", "exe" ]
+				}
+			]
+		},
+
+		"start_event": {
+			"allOf": [
+				{ "$ref": "#/definitions/event_common_fields" },
+				{
+					"properties": {
+						"event": { "const": "start" },
+						"t_abs": { "type": "number" },
+						"argv": {
+							"type": "array",
+							"items": { "type": "string" }
+						}
+					},
+					"required": [ "event", "t_abs", "argv" ]
+				}
+			]
+		},
+
+		"exit_event": {
+			"allOf": [
+				{ "$ref": "#/definitions/event_common_fields" },
+				{
+					"properties": {
+						"event": { "const": "exit" },
+						"t_abs": { "type": "number" },
+						"code": { "type": "integer" }
+					},
+					"required": [ "event", "t_abs", "code" ]
+				}
+			]
+		},
+
+		"atexit_event": {
+			"allOf": [
+				{ "$ref": "#/definitions/event_common_fields" },
+				{
+					"properties": {
+						"event": { "const": "atexit" },
+						"t_abs": { "type": "number" },
+						"code": { "type": "integer" }
+					},
+					"required": [ "event", "t_abs", "code" ]
+				}
+			]
+		},
+
+		"signal_event": {
+			"allOf": [
+				{ "$ref": "#/definitions/event_common_fields" },
+				{
+					"properties": {
+						"event": { "const": "signal" },
+						"t_abs": { "type": "number" },
+						"signo": { "type": "integer" }
+					},
+					"required": [ "event", "t_abs", "signo" ]
+				}
+			]
+		},
+
+		"error_event": {
+			"allOf": [
+				{ "$ref": "#/definitions/event_common_fields" },
+				{
+					"properties": {
+						"event": { "const": "error" },
+						"msg": { "type": "string" },
+						"fmt": { "type": "string" }
+					},
+					"required": [ "event", "msg", "fmt" ]
+				}
+			]
+		},
+
+		"cmd_path_event": {
+			"allOf": [
+				{ "$ref": "#/definitions/event_common_fields" },
+				{
+					"properties": {
+						"event": { "const": "cmd_path" },
+						"path": { "type": "string" }
+					},
+					"required": [ "event", "path" ]
+				}
+			]
+		},
+
+		"cmd_name_event": {
+			"allOf": [
+				{ "$ref": "#/definitions/event_common_fields" },
+				{
+					"properties": {
+						"event": { "const": "cmd_name" },
+						"name": { "type": "string" },
+						"hierarchy": { "type": "string" }
+					},
+					"required": [ "event", "name", "hierarchy" ]
+				}
+			]
+		},
+
+		"cmd_mode_event": {
+			"allOf": [
+				{ "$ref": "#/definitions/event_common_fields" },
+				{
+					"properties": {
+						"event": { "const": "cmd_mode" },
+						"name": { "type": "string" }
+					},
+					"required": [ "event", "name" ]
+				}
+			]
+		},
+
+		"alias_event": {
+			"allOf": [
+				{ "$ref": "#/definitions/event_common_fields" },
+				{
+					"properties": {
+						"event": { "const": "alias" },
+						"alias": { "type": "string" },
+						"argv": {
+							"type": "array",
+							"items": { "type": "string" }
+						}
+					},
+					"required": [ "event", "alias", "argv" ]
+				}
+			]
+		},
+
+		"child_start_event": {
+			"allOf": [
+				{ "$ref": "#/definitions/event_common_fields" },
+				{
+					"properties": {
+						"event": { "const": "child_start" },
+						"child_id": { "type": "integer" },
+						"child_class": { "type": "string" },
+						"use_shell": { "type": "boolean" },
+						"argv": {
+							"type": "array",
+							"items": { "type": "string" }
+						},
+						"hook_name": { "type": "string" },
+						"cd": { "type": "string" }
+					},
+					"required": [
+						"event", "child_id", "child_class", "use_shell", "argv"
+					]
+				}
+			]
+		},
+
+		"child_exit_event": {
+			"allOf": [
+				{ "$ref": "#/definitions/event_common_fields" },
+				{
+					"properties": {
+						"event": { "const": "child_exit" },
+						"child_id": { "type": "integer" },
+						"pid": { "type": "integer" },
+						"code": { "type": "integer" },
+						"t_rel": { "type": "number" }
+					},
+					"required": [ "event", "child_id", "pid", "code", "t_rel" ]
+				}
+			]
+		},
+
+		"exec_event": {
+			"allOf": [
+				{ "$ref": "#/definitions/event_common_fields" },
+				{
+					"properties": {
+						"event": { "const": "exec" },
+						"exec_id": { "type": "integer" },
+						"exe": { "type": "string" },
+						"argv": {
+							"type": "array",
+							"items": { "type": "string" }
+						}
+					},
+					"required": [ "event", "exec_id", "exe", "argv" ]
+				}
+			]
+		},
+
+		"exec_result_event": {
+			"allOf": [
+				{ "$ref": "#/definitions/event_common_fields" },
+				{
+					"properties": {
+						"event": { "const": "exec_result" },
+						"exec_id": { "type": "integer" },
+						"code": { "type": "integer" }
+					},
+					"required": [ "event", "exec_id", "code" ]
+				}
+			]
+		},
+
+		"thread_start_event": {
+			"allOf": [
+				{ "$ref": "#/definitions/event_common_fields" },
+				{
+					"properties": {
+						"event": { "const": "thread_start" },
+						"thread": { "type": "string" }
+					},
+					"required": [ "event", "thread" ]
+				}
+			]
+		},
+
+		"thread_exit_event": {
+			"allOf": [
+				{ "$ref": "#/definitions/event_common_fields" },
+				{
+					"properties": {
+						"event": { "const": "thread_exit" },
+						"thread": { "type": "string" },
+						"t_rel": { "type": "number" }
+					},
+					"required": [ "event", "thread", "t_rel" ]
+				}
+			]
+		},
+
+		"def_param_event": {
+			"allOf": [
+				{ "$ref": "#/definitions/event_common_fields" },
+				{
+					"properties": {
+						"event": { "const": "def_param" },
+						"param": { "type": "string" },
+						"value": { "type": "string" }
+					},
+					"required": [ "event", "param", "value" ]
+				}
+			]
+		},
+
+		"def_repo_event": {
+			"allOf": [
+				{ "$ref": "#/definitions/event_common_fields" },
+				{
+					"properties": {
+						"event": { "const": "def_repo" },
+						"repo": { "type": "integer" },
+						"worktree": { "type": "string" }
+					},
+					"required": [ "event", "repo", "worktree" ]
+				}
+			]
+		},
+
+		"region_enter_event": {
+			"allOf": [
+				{ "$ref": "#/definitions/event_common_fields" },
+				{
+					"properties": {
+						"event": { "const": "region_enter" },
+						"repo": { "type": "integer" },
+						"nesting": { "type": "integer" },
+						"category": { "type": "string" },
+						"label": { "type": "string" },
+						"msg": { "type": "string" }
+					},
+					"required": [ "event", "nesting" ]
+				}
+			]
+		},
+
+		"region_leave_event": {
+			"allOf": [
+				{ "$ref": "#/definitions/event_common_fields" },
+				{
+					"properties": {
+						"event": { "const": "region_leave" },
+						"repo": { "type": "integer" },
+						"t_rel": { "type": "number" },
+						"nesting": { "type": "integer" },
+						"category": { "type": "string" },
+						"label": { "type": "string" },
+						"msg": { "type": "string" }
+					},
+					"required": [ "event", "t_rel", "nesting" ]
+				}
+			]
+		},
+
+		"data_event": {
+			"allOf": [
+				{ "$ref": "#/definitions/event_common_fields" },
+				{
+					"properties": {
+						"event": { "const": "data" },
+						"repo": { "type": "integer" },
+						"t_abs": { "type": "number" },
+						"t_rel": { "type": "number" },
+						"nesting": { "type": "integer" },
+						"category": { "type": "string" },
+						"key": { "type": "string" },
+						"value": { "type": "string" }
+					},
+					"required": [
+						"event", "t_abs", "t_rel", "nesting", "category", "key",
+						"value"
+					]
+				}
+			]
+		},
+
+		"data-json_event": {
+			"allOf": [
+				{ "$ref": "#/definitions/event_common_fields" },
+				{
+					"properties": {
+						"event": { "const": "data-json" },
+						"repo": { "type": "integer" },
+						"t_abs": { "type": "number" },
+						"t_rel": { "type": "number" },
+						"nesting": { "type": "integer" },
+						"category": { "type": "string" },
+						"key": { "type": "string" },
+						"value": true
+					},
+					"required": [
+						"event", "t_abs", "t_rel", "nesting", "category", "key",
+						"value"
+					]
+				}
+			]
+		}
+	},
+
+	"oneOf": [
+		{ "$ref": "#/definitions/version_event" },
+		{ "$ref": "#/definitions/start_event" },
+		{ "$ref": "#/definitions/exit_event" },
+		{ "$ref": "#/definitions/atexit_event" },
+		{ "$ref": "#/definitions/signal_event" },
+		{ "$ref": "#/definitions/error_event" },
+		{ "$ref": "#/definitions/cmd_path_event" },
+		{ "$ref": "#/definitions/cmd_name_event" },
+		{ "$ref": "#/definitions/cmd_mode_event" },
+		{ "$ref": "#/definitions/alias_event" },
+		{ "$ref": "#/definitions/child_start_event" },
+		{ "$ref": "#/definitions/child_exit_event" },
+		{ "$ref": "#/definitions/exec_event" },
+		{ "$ref": "#/definitions/exec_result_event" },
+		{ "$ref": "#/definitions/thread_start_event" },
+		{ "$ref": "#/definitions/thread_exit_event" },
+		{ "$ref": "#/definitions/def_param_event" },
+		{ "$ref": "#/definitions/def_repo_event" },
+		{ "$ref": "#/definitions/region_enter_event" },
+		{ "$ref": "#/definitions/region_leave_event" },
+		{ "$ref": "#/definitions/data_event" },
+		{ "$ref": "#/definitions/data-json_event" }
+	]
+}
diff --git a/t/trace_schema_validator/strict_schema.json b/t/trace_schema_validator/strict_schema.json
new file mode 100644
index 0000000000..239d852ee9
--- /dev/null
+++ b/t/trace_schema_validator/strict_schema.json
@@ -0,0 +1,511 @@
+{
+	"$schema": "http://json-schema.org/draft-07/schema#",
+	"$id": "http://git-scm.com/schemas/event_schema.json",
+	"title": "trace2 strict schema",
+	"description": "Strict schema for trace2 event output that verifies there are no unexpected fields.",
+
+	"definitions": {
+		"version_event": {
+			"type": "object",
+			"properties": {
+				"sid": { "type": "string" },
+				"thread": { "type": "string" },
+				"time": {
+					"type": "string",
+					"format": "date-time"
+				},
+				"file": { "type": "string" },
+				"line": { "type": "integer" },
+				"repo": { "type": "integer" },
+				"event": { "const": "version" },
+				"evt": { "const": "1" },
+				"exe": { "type": "string" }
+			},
+			"required": [ "sid", "thread", "event", "evt", "exe" ],
+			"additionalProperties": false
+		},
+
+		"start_event": {
+			"type": "object",
+			"properties": {
+				"sid": { "type": "string" },
+				"thread": { "type": "string" },
+				"time": {
+					"type": "string",
+					"format": "date-time"
+				},
+				"file": { "type": "string" },
+				"line": { "type": "integer" },
+				"repo": { "type": "integer" },
+				"event": { "const": "start" },
+				"t_abs": { "type": "number" },
+				"argv": {
+					"type": "array",
+					"items": { "type": "string" }
+				}
+			},
+			"required": [ "sid", "thread", "time", "event", "t_abs", "argv" ],
+			"additionalProperties": false
+		},
+
+		"exit_event": {
+			"type": "object",
+			"properties": {
+				"sid": { "type": "string" },
+				"thread": { "type": "string" },
+				"time": {
+					"type": "string",
+					"format": "date-time"
+				},
+				"file": { "type": "string" },
+				"line": { "type": "integer" },
+				"repo": { "type": "integer" },
+				"event": { "const": "exit" },
+				"t_abs": { "type": "number" },
+				"code": { "type": "integer" }
+			},
+			"required": [ "sid", "thread", "event", "t_abs", "code" ],
+			"additionalProperties": false
+		},
+
+		"atexit_event": {
+			"type": "object",
+			"properties": {
+				"sid": { "type": "string" },
+				"thread": { "type": "string" },
+				"time": {
+					"type": "string",
+					"format": "date-time"
+				},
+				"file": { "type": "string" },
+				"line": { "type": "integer" },
+				"repo": { "type": "integer" },
+				"event": { "const": "atexit" },
+				"t_abs": { "type": "number" },
+				"code": { "type": "integer" }
+			},
+			"required": [ "sid", "thread", "time", "event", "t_abs", "code" ],
+			"additionalProperties": false
+		},
+
+		"signal_event": {
+			"type": "object",
+			"properties": {
+				"sid": { "type": "string" },
+				"thread": { "type": "string" },
+				"time": {
+					"type": "string",
+					"format": "date-time"
+				},
+				"file": { "type": "string" },
+				"line": { "type": "integer" },
+				"repo": { "type": "integer" },
+				"event": { "const": "signal" },
+				"t_abs": { "type": "number" },
+				"signo": { "type": "integer" }
+			},
+			"required": [ "sid", "thread", "event", "t_abs", "signo" ],
+			"additionalProperties": false
+		},
+
+		"error_event": {
+			"type": "object",
+			"properties": {
+				"sid": { "type": "string" },
+				"thread": { "type": "string" },
+				"time": {
+					"type": "string",
+					"format": "date-time"
+				},
+				"file": { "type": "string" },
+				"line": { "type": "integer" },
+				"repo": { "type": "integer" },
+				"event": { "const": "error" },
+				"msg": { "type": "string" },
+				"fmt": { "type": "string" }
+			},
+			"required": [ "sid", "thread", "event", "msg", "fmt" ],
+			"additionalProperties": false
+		},
+
+		"cmd_path_event": {
+			"type": "object",
+			"properties": {
+				"sid": { "type": "string" },
+				"thread": { "type": "string" },
+				"time": {
+					"type": "string",
+					"format": "date-time"
+				},
+				"file": { "type": "string" },
+				"line": { "type": "integer" },
+				"repo": { "type": "integer" },
+				"event": { "const": "cmd_path" },
+				"path": { "type": "string" }
+			},
+			"required": [ "sid", "thread", "event", "path" ],
+			"additionalProperties": false
+		},
+
+		"cmd_name_event": {
+			"type": "object",
+			"properties": {
+				"sid": { "type": "string" },
+				"thread": { "type": "string" },
+				"time": {
+					"type": "string",
+					"format": "date-time"
+				},
+				"file": { "type": "string" },
+				"line": { "type": "integer" },
+				"repo": { "type": "integer" },
+				"event": { "const": "cmd_name" },
+				"name": { "type": "string" },
+				"hierarchy": { "type": "string" }
+			},
+			"required": [ "sid", "thread", "event", "name", "hierarchy"
+			],
+			"additionalProperties": false
+		},
+
+		"cmd_mode_event": {
+			"type": "object",
+			"properties": {
+				"sid": { "type": "string" },
+				"thread": { "type": "string" },
+				"time": {
+					"type": "string",
+					"format": "date-time"
+				},
+				"file": { "type": "string" },
+				"line": { "type": "integer" },
+				"repo": { "type": "integer" },
+				"event": { "const": "cmd_mode" },
+				"name": { "type": "string" }
+			},
+			"required": [ "sid", "thread", "event", "name" ],
+			"additionalProperties": false
+		},
+
+		"alias_event": {
+			"type": "object",
+			"properties": {
+				"sid": { "type": "string" },
+				"thread": { "type": "string" },
+				"time": {
+					"type": "string",
+					"format": "date-time"
+				},
+				"file": { "type": "string" },
+				"line": { "type": "integer" },
+				"repo": { "type": "integer" },
+				"event": { "const": "alias" },
+				"alias": { "type": "string" },
+				"argv": {
+					"type": "array",
+					"items": { "type": "string" }
+				}
+			},
+			"required": [ "sid", "thread", "event", "alias", "argv" ],
+			"additionalProperties": false
+		},
+
+		"child_start_event": {
+			"type": "object",
+			"properties": {
+				"sid": { "type": "string" },
+				"thread": { "type": "string" },
+				"time": {
+					"type": "string",
+					"format": "date-time"
+				},
+				"file": { "type": "string" },
+				"line": { "type": "integer" },
+				"repo": { "type": "integer" },
+				"event": { "const": "child_start" },
+				"child_id": { "type": "integer" },
+				"child_class": { "type": "string" },
+				"use_shell": { "type": "boolean" },
+				"argv": {
+					"type": "array",
+					"items": { "type": "string" }
+				},
+				"hook_name": { "type": "string" },
+				"cd": { "type": "string" }
+			},
+			"required": [
+				"sid", "thread", "event", "child_id", "child_class",
+				"use_shell", "argv"
+			],
+			"additionalProperties": false
+		},
+
+		"child_exit_event": {
+			"type": "object",
+			"properties": {
+				"sid": { "type": "string" },
+				"thread": { "type": "string" },
+				"time": {
+					"type": "string",
+					"format": "date-time"
+				},
+				"file": { "type": "string" },
+				"line": { "type": "integer" },
+				"repo": { "type": "integer" },
+				"event": { "const": "child_exit" },
+				"child_id": { "type": "integer" },
+				"pid": { "type": "integer" },
+				"code": { "type": "integer" },
+				"t_rel": { "type": "number" }
+			},
+			"required": [
+				"sid", "thread", "event", "child_id", "pid", "code", "t_rel"
+			],
+			"additionalProperties": false
+		},
+
+		"exec_event": {
+			"type": "object",
+			"properties": {
+				"sid": { "type": "string" },
+				"thread": { "type": "string" },
+				"time": {
+					"type": "string",
+					"format": "date-time"
+				},
+				"file": { "type": "string" },
+				"line": { "type": "integer" },
+				"repo": { "type": "integer" },
+				"event": { "const": "exec" },
+				"exec_id": { "type": "integer" },
+				"exe": { "type": "string" },
+				"argv": {
+					"type": "array",
+					"items": { "type": "string" }
+				}
+			},
+			"required": [ "sid", "thread", "event", "exec_id", "exe", "argv" ],
+			"additionalProperties": false
+		},
+
+		"exec_result_event": {
+			"type": "object",
+			"properties": {
+				"sid": { "type": "string" },
+				"thread": { "type": "string" },
+				"time": {
+					"type": "string",
+					"format": "date-time"
+				},
+				"file": { "type": "string" },
+				"line": { "type": "integer" },
+				"repo": { "type": "integer" },
+				"event": { "const": "exec_result" },
+				"exec_id": { "type": "integer" },
+				"code": { "type": "integer" }
+			},
+			"required": [ "sid", "thread", "event", "exec_id", "code" ],
+			"additionalProperties": false
+		},
+
+		"thread_start_event": {
+			"type": "object",
+			"properties": {
+				"sid": { "type": "string" },
+				"thread": { "type": "string" },
+				"time": {
+					"type": "string",
+					"format": "date-time"
+				},
+				"file": { "type": "string" },
+				"line": { "type": "integer" },
+				"repo": { "type": "integer" },
+				"event": { "const": "thread_start" }
+			},
+			"required": [ "sid", "thread", "event" ],
+			"additionalProperties": false
+		},
+
+		"thread_exit_event": {
+			"type": "object",
+			"properties": {
+				"sid": { "type": "string" },
+				"thread": { "type": "string" },
+				"time": {
+					"type": "string",
+					"format": "date-time"
+				},
+				"file": { "type": "string" },
+				"line": { "type": "integer" },
+				"repo": { "type": "integer" },
+				"event": { "const": "thread_exit" },
+				"t_rel": { "type": "number" }
+			},
+			"required": [ "sid", "thread", "event", "t_rel" ],
+			"additionalProperties": false
+		},
+
+		"def_param_event": {
+			"type": "object",
+			"properties": {
+				"sid": { "type": "string" },
+				"thread": { "type": "string" },
+				"time": {
+					"type": "string",
+					"format": "date-time"
+				},
+				"file": { "type": "string" },
+				"line": { "type": "integer" },
+				"repo": { "type": "integer" },
+				"event": { "const": "def_param" },
+				"param": { "type": "string" },
+				"value": { "type": "string" }
+			},
+			"required": [ "sid", "thread", "event", "param", "value" ],
+			"additionalProperties": false
+		},
+
+		"def_repo_event": {
+			"type": "object",
+			"properties": {
+				"sid": { "type": "string" },
+				"thread": { "type": "string" },
+				"time": {
+					"type": "string",
+					"format": "date-time"
+				},
+				"file": { "type": "string" },
+				"line": { "type": "integer" },
+				"repo": { "type": "integer" },
+				"event": { "const": "def_repo" },
+				"worktree": { "type": "string" }
+			},
+			"required": [ "sid", "thread", "event", "repo", "worktree" ],
+			"additionalProperties": false
+		},
+
+		"region_enter_event": {
+			"type": "object",
+			"properties": {
+				"sid": { "type": "string" },
+				"thread": { "type": "string" },
+				"time": {
+					"type": "string",
+					"format": "date-time"
+				},
+				"file": { "type": "string" },
+				"line": { "type": "integer" },
+				"repo": { "type": "integer" },
+				"event": { "const": "region_enter" },
+				"nesting": { "type": "integer" },
+				"category": { "type": "string" },
+				"label": { "type": "string" },
+				"msg": { "type": "string" }
+			},
+			"required": [ "sid", "thread", "event", "nesting" ],
+			"additionalProperties": false
+		},
+
+		"region_leave_event": {
+			"type": "object",
+			"properties": {
+				"sid": { "type": "string" },
+				"thread": { "type": "string" },
+				"time": {
+					"type": "string",
+					"format": "date-time"
+				},
+				"file": { "type": "string" },
+				"line": { "type": "integer" },
+				"repo": { "type": "integer" },
+				"event": { "const": "region_leave" },
+				"t_rel": { "type": "number" },
+				"nesting": { "type": "integer" },
+				"category": { "type": "string" },
+				"label": { "type": "string" },
+				"msg": { "type": "string" }
+			},
+			"required": [ "sid", "thread", "event", "t_rel", "nesting" ],
+			"additionalProperties": false
+		},
+
+		"data_event": {
+			"type": "object",
+			"properties": {
+				"sid": { "type": "string" },
+				"thread": { "type": "string" },
+				"time": {
+					"type": "string",
+					"format": "date-time"
+				},
+				"file": { "type": "string" },
+				"line": { "type": "integer" },
+				"repo": { "type": "integer" },
+				"event": { "const": "data" },
+				"t_abs": { "type": "number" },
+				"t_rel": { "type": "number" },
+				"nesting": { "type": "integer" },
+				"category": { "type": "string" },
+				"key": { "type": "string" },
+				"value": { "type": "string" }
+			},
+			"required": [
+				"sid", "thread", "event", "t_abs", "t_rel", "nesting",
+				"category","key", "value"
+			],
+			"additionalProperties": false
+		},
+
+		"data-json_event": {
+			"type": "object",
+			"properties": {
+				"sid": { "type": "string" },
+				"thread": { "type": "string" },
+				"time": {
+					"type": "string",
+					"format": "date-time"
+				},
+				"file": { "type": "string" },
+				"line": { "type": "integer" },
+				"repo": { "type": "integer" },
+				"event": { "const": "data-json" },
+				"t_abs": { "type": "number" },
+				"t_rel": { "type": "number" },
+				"nesting": { "type": "integer" },
+				"category": { "type": "string" },
+				"key": { "type": "string" },
+				"value": true
+			},
+			"required": [
+				"sid", "thread", "event", "t_abs", "t_rel", "nesting",
+				"category", "key", "value"
+			],
+			"additionalProperties": false
+		}
+	},
+
+	"oneOf": [
+		{ "$ref": "#/definitions/version_event" },
+		{ "$ref": "#/definitions/start_event" },
+		{ "$ref": "#/definitions/exit_event" },
+		{ "$ref": "#/definitions/atexit_event" },
+		{ "$ref": "#/definitions/signal_event" },
+		{ "$ref": "#/definitions/error_event" },
+		{ "$ref": "#/definitions/cmd_path_event" },
+		{ "$ref": "#/definitions/cmd_name_event" },
+		{ "$ref": "#/definitions/cmd_mode_event" },
+		{ "$ref": "#/definitions/alias_event" },
+		{ "$ref": "#/definitions/child_start_event" },
+		{ "$ref": "#/definitions/child_exit_event" },
+		{ "$ref": "#/definitions/exec_event" },
+		{ "$ref": "#/definitions/exec_result_event" },
+		{ "$ref": "#/definitions/thread_start_event" },
+		{ "$ref": "#/definitions/thread_exit_event" },
+		{ "$ref": "#/definitions/def_param_event" },
+		{ "$ref": "#/definitions/def_repo_event" },
+		{ "$ref": "#/definitions/region_enter_event" },
+		{ "$ref": "#/definitions/region_leave_event" },
+		{ "$ref": "#/definitions/data_event" },
+		{ "$ref": "#/definitions/data-json_event" }
+	]
+}
-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply	[flat|nested] 14+ messages in thread

* [RFC PATCH 3/3] trace2: add a schema validator for trace2 events
  2019-06-11 23:31 [RFC PATCH 0/3] Add a JSON Schema for trace2 events Josh Steadmon
  2019-06-11 23:31 ` [RFC PATCH 1/3] trace2: correct trace2 field name documentation Josh Steadmon
  2019-06-11 23:31 ` [RFC PATCH 2/3] trace2: Add a JSON schema for trace2 events Josh Steadmon
@ 2019-06-11 23:31 ` " Josh Steadmon
  2019-06-12 13:28   ` Ævar Arnfjörð Bjarmason
  2 siblings, 1 reply; 14+ messages in thread
From: Josh Steadmon @ 2019-06-11 23:31 UTC (permalink / raw)
  To: git, git

trace_schema_validator can be used to verify that trace2 event output
conforms to the expectations set by the API documentation and codified
in event_schema.json (or strict_schema.json). This allows us to build a
regression test to verify that trace2 output does not change
unexpectedly.

Signed-off-by: Josh Steadmon <steadmon@google.com>
---
 t/trace_schema_validator/.gitignore           |  1 +
 t/trace_schema_validator/Makefile             | 10 +++
 .../trace_schema_validator.go                 | 74 +++++++++++++++++++
 3 files changed, 85 insertions(+)
 create mode 100644 t/trace_schema_validator/.gitignore
 create mode 100644 t/trace_schema_validator/Makefile
 create mode 100644 t/trace_schema_validator/trace_schema_validator.go

diff --git a/t/trace_schema_validator/.gitignore b/t/trace_schema_validator/.gitignore
new file mode 100644
index 0000000000..c3f1e04e9e
--- /dev/null
+++ b/t/trace_schema_validator/.gitignore
@@ -0,0 +1 @@
+trace_schema_validator
diff --git a/t/trace_schema_validator/Makefile b/t/trace_schema_validator/Makefile
new file mode 100644
index 0000000000..ed22675e5d
--- /dev/null
+++ b/t/trace_schema_validator/Makefile
@@ -0,0 +1,10 @@
+.PHONY: fetch_deps clean
+
+trace_schema_validator: fetch_deps trace_schema_validator.go
+	go build
+
+fetch_deps:
+	go get github.com/xeipuuv/gojsonschema
+
+clean:
+	rm -f trace_schema_validator
diff --git a/t/trace_schema_validator/trace_schema_validator.go b/t/trace_schema_validator/trace_schema_validator.go
new file mode 100644
index 0000000000..51dc9ec608
--- /dev/null
+++ b/t/trace_schema_validator/trace_schema_validator.go
@@ -0,0 +1,74 @@
+// trace_schema_validator validates individual lines of an input file against a
+// provided JSON-Schema for git trace2 event output.
+//
+// Traces can be collected by setting the GIT_TRACE2_EVENT environment variable
+// to an absolute path and running any Git command; traces will be appended to
+// the file.
+//
+// Traces can then be verified like so:
+//   trace_schema_validator \
+//     --trace2_event_file /path/to/trace/output \
+//     --schema_file /path/to/schema
+package main
+
+import (
+	"bufio"
+	"flag"
+	"log"
+	"os"
+	"path/filepath"
+
+	"github.com/xeipuuv/gojsonschema"
+)
+
+// Required flags
+var schemaFile = flag.String("schema_file", "", "JSON-Schema filename")
+var trace2EventFile = flag.String("trace2_event_file", "", "trace2 event filename")
+
+func main() {
+	flag.Parse()
+	if *schemaFile == "" || *trace2EventFile == "" {
+		log.Fatal("Both --schema_file and --trace2_event_file are required.")
+	}
+	schemaURI, err := filepath.Abs(*schemaFile)
+	if err != nil {
+		log.Fatal("Can't get absolute path for schema file: ", err)
+	}
+	schemaURI = "file://" + schemaURI
+
+	schemaLoader := gojsonschema.NewReferenceLoader(schemaURI)
+	schema, err := gojsonschema.NewSchema(schemaLoader)
+	if err != nil {
+		log.Fatal("Problem loading schema: ", err)
+	}
+
+	tracesFile, err := os.Open(*trace2EventFile)
+	if err != nil {
+		log.Fatal("Problem opening trace file: ", err)
+	}
+	defer tracesFile.Close()
+
+	scanner := bufio.NewScanner(tracesFile)
+
+	count := 0
+	for ; scanner.Scan(); count++ {
+		event := gojsonschema.NewStringLoader(scanner.Text())
+		result, err := schema.Validate(event)
+		if err != nil {
+			log.Fatal(err)
+		}
+		if !result.Valid() {
+			log.Print("Trace event is invalid: ", scanner.Text())
+			for _, desc := range result.Errors() {
+				log.Print("- ", desc)
+			}
+			os.Exit(1)
+		}
+	}
+
+	if err := scanner.Err(); err != nil {
+		log.Fatal("Scanning error: ", err)
+	}
+
+	log.Print("Validated events: ", count)
+}
-- 
2.22.0.410.gd8fdbe21b5-goog


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 3/3] trace2: add a schema validator for trace2 events
  2019-06-11 23:31 ` [RFC PATCH 3/3] trace2: add a schema validator " Josh Steadmon
@ 2019-06-12 13:28   ` Ævar Arnfjörð Bjarmason
  2019-06-12 16:23     ` Josh Steadmon
  0 siblings, 1 reply; 14+ messages in thread
From: Ævar Arnfjörð Bjarmason @ 2019-06-12 13:28 UTC (permalink / raw)
  To: Josh Steadmon; +Cc: git, git


On Wed, Jun 12 2019, Josh Steadmon wrote:

> trace_schema_validator can be used to verify that trace2 event output
> conforms to the expectations set by the API documentation and codified
> in event_schema.json (or strict_schema.json). This allows us to build a
> regression test to verify that trace2 output does not change
> unexpectedly.

Does this actually work for you? As seen in my code at
https://public-inbox.org/git/87zhnuwdkp.fsf@evledraar.gmail.com/ our
test suite emits various lines of JSON that aren't even validly encoded,
so I can't imagine we're passing any sort of proper parser validatior,
let alone a schema validator.

In terms of implementation I think it would make sense to have a *.sh
wrapper for this already, then we could test via prereqs if we have some
of the existing validators (seems there's a list at
https://json-schema.org/implementations.html) and e.g. run a dummy test
against some small list of git commands, and then you could also pass it
an env variable with "here's the trace file" so you could do:

    GIT_TRACE2_EVENT=/tmp/git.events prove <all testss> && VALIDATE_THIS=/tmp/git.events ./<that new test>.sh

And it would validate that file, if set.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 3/3] trace2: add a schema validator for trace2 events
  2019-06-12 13:28   ` Ævar Arnfjörð Bjarmason
@ 2019-06-12 16:23     ` Josh Steadmon
  2019-06-12 19:18       ` Jeff King
  2019-06-21 11:53       ` Jakub Narebski
  0 siblings, 2 replies; 14+ messages in thread
From: Josh Steadmon @ 2019-06-12 16:23 UTC (permalink / raw)
  To: Ævar Arnfjörð Bjarmason; +Cc: git, git

On 2019.06.12 15:28, Ævar Arnfjörð Bjarmason wrote:
> 
> On Wed, Jun 12 2019, Josh Steadmon wrote:
> 
> > trace_schema_validator can be used to verify that trace2 event output
> > conforms to the expectations set by the API documentation and codified
> > in event_schema.json (or strict_schema.json). This allows us to build a
> > regression test to verify that trace2 output does not change
> > unexpectedly.
> 
> Does this actually work for you? As seen in my code at
> https://public-inbox.org/git/87zhnuwdkp.fsf@evledraar.gmail.com/ our
> test suite emits various lines of JSON that aren't even validly encoded,
> so I can't imagine we're passing any sort of proper parser validatior,
> let alone a schema validator.

Yes, it seems that gojsonschema (and its dependencies) are not very strict about
encoding. I also had an alternate Python implementation, and it failed to parse
lines that were not properly encoded. I just had that version print out a
warning with the number of failed decodings. I believe it was ~20 out of 1.7M
events.

> In terms of implementation I think it would make sense to have a *.sh
> wrapper for this already, then we could test via prereqs if we have some
> of the existing validators (seems there's a list at
> https://json-schema.org/implementations.html) and e.g. run a dummy test
> against some small list of git commands, and then you could also pass it
> an env variable with "here's the trace file" so you could do:
> 
>     GIT_TRACE2_EVENT=/tmp/git.events prove <all testss> && VALIDATE_THIS=/tmp/git.events ./<that new test>.sh
> 
> And it would validate that file, if set.

The problem with the existing validators is that they expect each file to be a
complete JSON entity, whereas the trace output is one object per line. You can
of course loop over the lines in a shell script, but in my testing this approach
took multiple hours on the full test suite trace output, vs. 15 minutes for the
implementation in this patch.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 1/3] trace2: correct trace2 field name documentation
  2019-06-11 23:31 ` [RFC PATCH 1/3] trace2: correct trace2 field name documentation Josh Steadmon
@ 2019-06-12 18:00   ` Junio C Hamano
  2019-06-12 18:14     ` Josh Steadmon
  2019-06-14 15:53   ` Jeff Hostetler
  1 sibling, 1 reply; 14+ messages in thread
From: Junio C Hamano @ 2019-06-12 18:00 UTC (permalink / raw)
  To: Josh Steadmon; +Cc: git, git

Josh Steadmon <steadmon@google.com> writes:

> Correct the api-trace2 documentation, which lists "signal" as an
> expected field for the signal event type, but which actually outputs
> "signo" as the field name.


Nice.  Is this one of the findings made by your validation, by the way?

>
> Signed-off-by: Josh Steadmon <steadmon@google.com>
> ---
>  Documentation/technical/api-trace2.txt | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
>
> diff --git a/Documentation/technical/api-trace2.txt b/Documentation/technical/api-trace2.txt
> index 23c3cc7a37..fd1e628944 100644
> --- a/Documentation/technical/api-trace2.txt
> +++ b/Documentation/technical/api-trace2.txt
> @@ -668,7 +668,7 @@ completed.)
>  	"event":"signal",
>  	...
>  	"t_abs":0.001227,  # elapsed time in seconds
> -	"signal":13        # SIGTERM, SIGINT, etc.
> +	"signo":13         # SIGTERM, SIGINT, etc.
>  }
>  ------------

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 1/3] trace2: correct trace2 field name documentation
  2019-06-12 18:00   ` Junio C Hamano
@ 2019-06-12 18:14     ` Josh Steadmon
  0 siblings, 0 replies; 14+ messages in thread
From: Josh Steadmon @ 2019-06-12 18:14 UTC (permalink / raw)
  To: Junio C Hamano; +Cc: git, git

On 2019.06.12 11:00, Junio C Hamano wrote:
> Josh Steadmon <steadmon@google.com> writes:
> 
> > Correct the api-trace2 documentation, which lists "signal" as an
> > expected field for the signal event type, but which actually outputs
> > "signo" as the field name.
> 
> 
> Nice.  Is this one of the findings made by your validation, by the way?

Yes. I based both versions of the schema purely off the documentation, then
tested against real output. This was the only discrepancy that was uncovered.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 3/3] trace2: add a schema validator for trace2 events
  2019-06-12 16:23     ` Josh Steadmon
@ 2019-06-12 19:18       ` Jeff King
  2019-06-20 18:15         ` Josh Steadmon
  2019-06-21 11:53       ` Jakub Narebski
  1 sibling, 1 reply; 14+ messages in thread
From: Jeff King @ 2019-06-12 19:18 UTC (permalink / raw)
  To: Josh Steadmon; +Cc: Ævar Arnfjörð Bjarmason, git, git

On Wed, Jun 12, 2019 at 09:23:41AM -0700, Josh Steadmon wrote:

> The problem with the existing validators is that they expect each file to be a
> complete JSON entity, whereas the trace output is one object per line. You can
> of course loop over the lines in a shell script, but in my testing this approach
> took multiple hours on the full test suite trace output, vs. 15 minutes for the
> implementation in this patch.

It seems like it should be easy to turn a sequence of entities into a
single entity, with something like:

  echo '['
  sed 's/$/,/' <one-per-line
  echo ']'

You could even turn a sequence of files into a single entity (which
might be even faster to validate, since it would be one invocation for
the entire test suite) with something like:

  echo '{'
  for fn in $FILES; do
	echo "\"$fn\": "
	cat $fn
	echo ","
  done
  echo '}'

though I suspect the resulting error messages might not be as good.

Obviously neither of those is particularly robust if the individual JSON
is not well-formed. But then, if we are mostly interested in testing
whether it's well-formed and expect it to be in the normal case, that
might be a good optimization.

I also wouldn't be surprised if "jq" could do this in a more robust way.

-Peff

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 1/3] trace2: correct trace2 field name documentation
  2019-06-11 23:31 ` [RFC PATCH 1/3] trace2: correct trace2 field name documentation Josh Steadmon
  2019-06-12 18:00   ` Junio C Hamano
@ 2019-06-14 15:53   ` Jeff Hostetler
  1 sibling, 0 replies; 14+ messages in thread
From: Jeff Hostetler @ 2019-06-14 15:53 UTC (permalink / raw)
  To: Josh Steadmon, git



On 6/11/2019 7:31 PM, Josh Steadmon wrote:
> Correct the api-trace2 documentation, which lists "signal" as an
> expected field for the signal event type, but which actually outputs
> "signo" as the field name.
> 
> Signed-off-by: Josh Steadmon <steadmon@google.com>
> ---
>   Documentation/technical/api-trace2.txt | 2 +-
>   1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/Documentation/technical/api-trace2.txt b/Documentation/technical/api-trace2.txt
> index 23c3cc7a37..fd1e628944 100644
> --- a/Documentation/technical/api-trace2.txt
> +++ b/Documentation/technical/api-trace2.txt
> @@ -668,7 +668,7 @@ completed.)
>   	"event":"signal",
>   	...
>   	"t_abs":0.001227,  # elapsed time in seconds
> -	"signal":13        # SIGTERM, SIGINT, etc.
> +	"signo":13         # SIGTERM, SIGINT, etc.
>   }
>   ------------
>   
> 

Nice!

Thanks,
Jeff

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 2/3] trace2: Add a JSON schema for trace2 events
  2019-06-11 23:31 ` [RFC PATCH 2/3] trace2: Add a JSON schema for trace2 events Josh Steadmon
@ 2019-06-14 15:59   ` Jeff Hostetler
  2019-06-20 17:26     ` Josh Steadmon
  0 siblings, 1 reply; 14+ messages in thread
From: Jeff Hostetler @ 2019-06-14 15:59 UTC (permalink / raw)
  To: Josh Steadmon, git



On 6/11/2019 7:31 PM, Josh Steadmon wrote:
> Define a JSON schema[1] that can be used to validate trace2 event
> objects. This can be used to add regression tests to verify that the
> event output format does not change unexpectedly.
> 
> Two versions of the schema are provided:
> * event_schema.json is more permissive. It verifies that all expected
>    fields are present in each trace event, but it allows traces to have
>    unexpected additional fields. This allows the schema to be specified
>    more concisely by factoring out the common fields into a reusable
>    sub-schema.
> * strict_schema.json is more restrictive. It verifies that all expected
>    fields are present and no unexpected fields are present in each trace
>    event. Due to this additional restriction, the common fields cannot be
>    factored out into a re-usable subschema (at least as-of draft-07) [2],
>    and must be repeated for each event definition.
> 
[...]
> +
> +		"data-json_event": {
> +			"allOf": [
> +				{ "$ref": "#/definitions/event_common_fields" },
> +				{
> +					"properties": {
> +						"event": { "const": "data-json" },
> +						"repo": { "type": "integer" },
> +						"t_abs": { "type": "number" },
> +						"t_rel": { "type": "number" },
> +						"nesting": { "type": "integer" },
> +						"category": { "type": "string" },
> +						"key": { "type": "string" },
> +						"value": true
> +					},
> +					"required": [
> +						"event", "t_abs", "t_rel", "nesting", "category", "key",
> +						"value"
> +					]
> +				}
> +			]
> +		}
> +	},
[...]

Here you have "value" as just a boolean rather than a sub-object.
Is that a limitation of the schema tools?  I guess this is reasonable
since the contents of the sub-object are variable and you wouldn't be
able to verify it anyway.

Jeff


^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 2/3] trace2: Add a JSON schema for trace2 events
  2019-06-14 15:59   ` Jeff Hostetler
@ 2019-06-20 17:26     ` Josh Steadmon
  0 siblings, 0 replies; 14+ messages in thread
From: Josh Steadmon @ 2019-06-20 17:26 UTC (permalink / raw)
  To: Jeff Hostetler; +Cc: git

On 2019.06.14 11:59, Jeff Hostetler wrote:
> 
> 
> On 6/11/2019 7:31 PM, Josh Steadmon wrote:
> > Define a JSON schema[1] that can be used to validate trace2 event
> > objects. This can be used to add regression tests to verify that the
> > event output format does not change unexpectedly.
> > 
> > Two versions of the schema are provided:
> > * event_schema.json is more permissive. It verifies that all expected
> >    fields are present in each trace event, but it allows traces to have
> >    unexpected additional fields. This allows the schema to be specified
> >    more concisely by factoring out the common fields into a reusable
> >    sub-schema.
> > * strict_schema.json is more restrictive. It verifies that all expected
> >    fields are present and no unexpected fields are present in each trace
> >    event. Due to this additional restriction, the common fields cannot be
> >    factored out into a re-usable subschema (at least as-of draft-07) [2],
> >    and must be repeated for each event definition.
> > 
> [...]
> > +
> > +		"data-json_event": {
> > +			"allOf": [
> > +				{ "$ref": "#/definitions/event_common_fields" },
> > +				{
> > +					"properties": {
> > +						"event": { "const": "data-json" },
> > +						"repo": { "type": "integer" },
> > +						"t_abs": { "type": "number" },
> > +						"t_rel": { "type": "number" },
> > +						"nesting": { "type": "integer" },
> > +						"category": { "type": "string" },
> > +						"key": { "type": "string" },
> > +						"value": true
> > +					},
> > +					"required": [
> > +						"event", "t_abs", "t_rel", "nesting", "category", "key",
> > +						"value"
> > +					]
> > +				}
> > +			]
> > +		}
> > +	},
> [...]
> 
> Here you have "value" as just a boolean rather than a sub-object.
> Is that a limitation of the schema tools?  I guess this is reasonable
> since the contents of the sub-object are variable and you wouldn't be
> able to verify it anyway.

Yeah, I just listed it as "true" (which in JSON-Schema means that a value is
expected but we're not making any restrictions on what it looks like) because of
not being able to verify the contents of the sub-object. But we should actually
at least verify that it's an object versus some other type, so I'll fix that if
there's enough interest for a v2 of this series.

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 3/3] trace2: add a schema validator for trace2 events
  2019-06-12 19:18       ` Jeff King
@ 2019-06-20 18:15         ` Josh Steadmon
  0 siblings, 0 replies; 14+ messages in thread
From: Josh Steadmon @ 2019-06-20 18:15 UTC (permalink / raw)
  To: Jeff King; +Cc: Ævar Arnfjörð Bjarmason, git, git

On 2019.06.12 15:18, Jeff King wrote:
> On Wed, Jun 12, 2019 at 09:23:41AM -0700, Josh Steadmon wrote:
> 
> > The problem with the existing validators is that they expect each file to be a
> > complete JSON entity, whereas the trace output is one object per line. You can
> > of course loop over the lines in a shell script, but in my testing this approach
> > took multiple hours on the full test suite trace output, vs. 15 minutes for the
> > implementation in this patch.
> 
> It seems like it should be easy to turn a sequence of entities into a
> single entity, with something like:
> 
>   echo '['
>   sed 's/$/,/' <one-per-line
>   echo ']'
> 
> You could even turn a sequence of files into a single entity (which
> might be even faster to validate, since it would be one invocation for
> the entire test suite) with something like:
> 
>   echo '{'
>   for fn in $FILES; do
> 	echo "\"$fn\": "
> 	cat $fn
> 	echo ","
>   done
>   echo '}'
> 
> though I suspect the resulting error messages might not be as good.
> 
> Obviously neither of those is particularly robust if the individual JSON
> is not well-formed. But then, if we are mostly interested in testing
> whether it's well-formed and expect it to be in the normal case, that
> might be a good optimization.

Yeah, as I noted in my reply to Ævar, ~20 of the trace lines generated by the
test suite are not properly encoded. So if we do something like:

  $ GIT_TRACE2_EVENT=$(pwd)/one-per-line make test
  $ (echo '[' ; sed 's/$/,/' < one-per-line ; echo ']') > list
  $ validate list

then most validators will only tell us that the file as a whole is malformed.
If we validate line-by-line, then we can just count how many malformed lines we
have and make sure it's within expectations.

Alternatively, we could just explicitly disable tracing on the tests that
generate the malformed traces.

> 
> I also wouldn't be surprised if "jq" could do this in a more robust way.

I'll go take a look at jq.

> -Peff

^ permalink raw reply	[flat|nested] 14+ messages in thread

* Re: [RFC PATCH 3/3] trace2: add a schema validator for trace2 events
  2019-06-12 16:23     ` Josh Steadmon
  2019-06-12 19:18       ` Jeff King
@ 2019-06-21 11:53       ` Jakub Narebski
  1 sibling, 0 replies; 14+ messages in thread
From: Jakub Narebski @ 2019-06-21 11:53 UTC (permalink / raw)
  To: Josh Steadmon; +Cc: Ævar Arnfjörð Bjarmason, git, git

Josh Steadmon <steadmon@google.com> writes:
> On 2019.06.12 15:28, Ævar Arnfjörð Bjarmason wrote: 
>> On Wed, Jun 12 2019, Josh Steadmon wrote:
>> 
>>> trace_schema_validator can be used to verify that trace2 event output
>>> conforms to the expectations set by the API documentation and codified
>>> in event_schema.json (or strict_schema.json). This allows us to build a
>>> regression test to verify that trace2 output does not change
>>> unexpectedly.
>> 
>> Does this actually work for you? As seen in my code at
>> https://public-inbox.org/git/87zhnuwdkp.fsf@evledraar.gmail.com/ our
>> test suite emits various lines of JSON that aren't even validly encoded,
>> so I can't imagine we're passing any sort of proper parser validatior,
>> let alone a schema validator.
[...]
> The problem with the existing validators is that they expect each file to be a
> complete JSON entity, whereas the trace output is one object per line. You can
> of course loop over the lines in a shell script, but in my testing this approach
> took multiple hours on the full test suite trace output, vs. 15 minutes for the
> implementation in this patch.

Isn't this JSON-Lines (http://jsonlines.org/), also known as
Line-delimited JSON (LDJSON), newline-delimited JSON (NDJSON) format?

Best,
--
Jakub Narębski

^ permalink raw reply	[flat|nested] 14+ messages in thread

end of thread, back to index

Thread overview: 14+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2019-06-11 23:31 [RFC PATCH 0/3] Add a JSON Schema for trace2 events Josh Steadmon
2019-06-11 23:31 ` [RFC PATCH 1/3] trace2: correct trace2 field name documentation Josh Steadmon
2019-06-12 18:00   ` Junio C Hamano
2019-06-12 18:14     ` Josh Steadmon
2019-06-14 15:53   ` Jeff Hostetler
2019-06-11 23:31 ` [RFC PATCH 2/3] trace2: Add a JSON schema for trace2 events Josh Steadmon
2019-06-14 15:59   ` Jeff Hostetler
2019-06-20 17:26     ` Josh Steadmon
2019-06-11 23:31 ` [RFC PATCH 3/3] trace2: add a schema validator " Josh Steadmon
2019-06-12 13:28   ` Ævar Arnfjörð Bjarmason
2019-06-12 16:23     ` Josh Steadmon
2019-06-12 19:18       ` Jeff King
2019-06-20 18:15         ` Josh Steadmon
2019-06-21 11:53       ` Jakub Narebski

git@vger.kernel.org list mirror (unofficial, one of many)

Archives are clonable:
	git clone --mirror https://public-inbox.org/git
	git clone --mirror http://ou63pmih66umazou.onion/git
	git clone --mirror http://czquwvybam4bgbro.onion/git
	git clone --mirror http://hjrcffqmbrq6wope.onion/git

Newsgroups are available over NNTP:
	nntp://news.public-inbox.org/inbox.comp.version-control.git
	nntp://ou63pmih66umazou.onion/inbox.comp.version-control.git
	nntp://czquwvybam4bgbro.onion/inbox.comp.version-control.git
	nntp://hjrcffqmbrq6wope.onion/inbox.comp.version-control.git
	nntp://news.gmane.org/gmane.comp.version-control.git

 note: .onion URLs require Tor: https://www.torproject.org/

AGPL code for this site: git clone https://public-inbox.org/ public-inbox