Closed Bug 1251199 Opened 4 years ago Closed 4 years ago

Define JSON schema for "core" ping

Categories

(Toolkit :: Telemetry, defect, P1)

defect
Points:
2

Tracking

()

RESOLVED FIXED
mozilla47
Tracking Status
firefox47 --- fixed

People

(Reporter: gfritzsche, Assigned: Dexter)

References

(Blocks 1 open bug)

Details

(Whiteboard: [measurement:client])

Attachments

(1 file, 5 obsolete files)

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
Priority: P2 → P1
Points: --- → 2
Blocks: core-ping
No longer blocks: ut-android
Assignee: nobody → alessio.placitelli
Attached file core.schema.json (obsolete) —
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
Attachment #8725157 - Flags: review?(gfritzsche)
Comment on attachment 8725157 [details]
core.schema.json

This can live in-tree in toolkit/components/telemetry/schemas.
Attachment #8725157 - Flags: review?(gfritzsche)
Attached patch bug1251199.patch (obsolete) — Splinter Review
Attachment #8725157 - Attachment is obsolete: true
Attachment #8725683 - Flags: review?(gfritzsche)
Attachment #8725683 - Attachment is obsolete: true
Attachment #8725683 - Flags: review?(gfritzsche)
Attached patch bug1251199.patch (obsolete) — Splinter Review
Sorry, the previous patch had weird indentation.
Attachment #8725689 - Flags: review?(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.
Attachment #8725689 - Flags: review?(gfritzsche) → feedback+
Attached patch bug1251199.patch (obsolete) — Splinter Review
Thanks. I'm not sure if there's a better way to enforce v = 1.
Attachment #8725689 - Attachment is obsolete: true
Attachment #8725711 - Flags: review?(gfritzsche)
Attachment #8725711 - Flags: review?(gfritzsche)
Attached patch bug1251199.patch (obsolete) — Splinter Review
Whoops, wrong patch attached. Sorry for the mess.
Attachment #8725711 - Attachment is obsolete: true
Attachment #8725712 - Flags: review?(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".
Attachment #8725712 - Flags: review?(gfritzsche) → review+
Attached patch bug1251199.patchSplinter Review
Attachment #8725712 - Attachment is obsolete: true
Attachment #8725716 - Flags: review+
(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
This adds a schema file to the local tree, no test coverage (will come as a separate bug), so no try push.
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/fe9aa9228a90
Status: NEW → RESOLVED
Closed: 4 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.