1251199 - Define JSON schema for "core" ping

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect, P1)

Description

[Georg Fritzsche [:gfritzsche]]

We are starting to use schemas in the pipeline.

I think we should start on adding a schema for the "core" pings now.
They can live in toolkit/components/telemetry/schemas.

On the pipeline side schemas are tracked in this repo:
https://github.com/mozilla-services/mozilla-pipeline-schemas

Comment 1

[Alessio Placitelli [:Dexter]]

Attachment: core.schema.json

This schema validates v1 core pings. The schema works with the sample data I have and successfully marks as invalid weird pings.

The meta section of the ping is not validated as it doesn't come directly from the client itself but it's generated server-side. Sam's main ping validator is not validating this section either [1].

[1] - https://github.com/mozilla-services/mozilla-pipeline-schemas/blob/master/telemetry/main.schema.json

Comment 2

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8725157 [details]
core.schema.json

This can live in-tree in toolkit/components/telemetry/schemas.

Comment 3

[Alessio Placitelli [:Dexter]]

Attachment: bug1251199.patch

Comment 4

[Alessio Placitelli [:Dexter]]

Attachment: bug1251199.patch

Sorry, the previous patch had weird indentation.

Comment 5

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8725689 [details] [diff] [review]
bug1251199.patch

Review of attachment 8725689 [details] [diff] [review]:
-----------------------------------------------------------------

::: toolkit/components/telemetry/schemas/core.schema.json
@@ +21,5 @@
> +    },
> +    "locale" : {
> +      "type" : "string"
> +    },
> +    "meta" : {

What is this? The pipeline meta information?
Do we need this in here?

@@ +36,5 @@
> +      "minimum": 0
> +    },
> +    "v" : {
> +      "type" : "integer",
> +      "minimum": 1

This is the schema for version 1, so lets call it version 1.
We need to do schema updates anyway.

Comment 6

[Alessio Placitelli [:Dexter]]

Attachment: bug1251199.patch

Thanks. I'm not sure if there's a better way to enforce v = 1.

Comment 7

[Alessio Placitelli [:Dexter]]

Attachment: bug1251199.patch

Whoops, wrong patch attached. Sorry for the mess.

Comment 8

[Georg Fritzsche [:gfritzsche]]

Comment on attachment 8725712 [details] [diff] [review]
bug1251199.patch

Review of attachment 8725712 [details] [diff] [review]:
-----------------------------------------------------------------

Lets put it in a pipeline schema PR and wait a bit for feedback.

::: toolkit/components/telemetry/schemas/core.schema.json
@@ +34,5 @@
> +    },
> +    "v" : {
> +      "type" : "integer",
> +      "minimum": 1,
> +      "maximum": 1,

Let's use "enum".

Comment 9

[Alessio Placitelli [:Dexter]]

Attachment: bug1251199.patch

Comment 10

[Alessio Placitelli [:Dexter]]

(In reply to Georg Fritzsche [:gfritzsche] from comment #8)
> Comment on attachment 8725712 [details] [diff] [review]
> bug1251199.patch
> 
> Review of attachment 8725712 [details] [diff] [review]:
> -----------------------------------------------------------------
> 
> Lets put it in a pipeline schema PR and wait a bit for feedback.

https://github.com/mozilla-services/mozilla-pipeline-schemas/pull/3

Comment 11

[Alessio Placitelli [:Dexter]]

This adds a schema file to the local tree, no test coverage (will come as a separate bug), so no try push.

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/fx-team/rev/fe9aa9228a90

Comment 13

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/fe9aa9228a90