Define JSON schema for "core" ping

RESOLVED FIXED in Firefox 47

Status

()

Toolkit
Telemetry
P1
normal
RESOLVED FIXED
2 years ago
2 years ago

People

(Reporter: gfritzsche, Assigned: Dexter)

Tracking

(Blocks: 1 bug)

Trunk
mozilla47
Points:
2

Firefox Tracking Flags

(firefox47 fixed)

Details

(Whiteboard: [measurement:client])

Attachments

(1 attachment, 5 obsolete attachments)

(Reporter)

Description

2 years ago
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
(Reporter)

Updated

2 years ago
Priority: P2 → P1
(Reporter)

Updated

2 years ago
Points: --- → 2
(Reporter)

Updated

2 years ago
Blocks: 1251636
No longer blocks: 1220177
(Assignee)

Updated

2 years ago
Assignee: nobody → alessio.placitelli
(Assignee)

Comment 1

2 years ago
Created attachment 8725157 [details]
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
Attachment #8725157 - Flags: review?(gfritzsche)
(Reporter)

Comment 2

2 years ago
Comment on attachment 8725157 [details]
core.schema.json

This can live in-tree in toolkit/components/telemetry/schemas.
Attachment #8725157 - Flags: review?(gfritzsche)
(Assignee)

Comment 3

2 years ago
Created attachment 8725683 [details] [diff] [review]
bug1251199.patch
Attachment #8725157 - Attachment is obsolete: true
Attachment #8725683 - Flags: review?(gfritzsche)
(Assignee)

Updated

2 years ago
Attachment #8725683 - Attachment is obsolete: true
Attachment #8725683 - Flags: review?(gfritzsche)
(Assignee)

Comment 4

2 years ago
Created attachment 8725689 [details] [diff] [review]
bug1251199.patch

Sorry, the previous patch had weird indentation.
Attachment #8725689 - Flags: review?(gfritzsche)
(Reporter)

Comment 5

2 years ago
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+
(Assignee)

Comment 6

2 years ago
Created attachment 8725711 [details] [diff] [review]
bug1251199.patch

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)
(Assignee)

Updated

2 years ago
Attachment #8725711 - Flags: review?(gfritzsche)
(Assignee)

Comment 7

2 years ago
Created attachment 8725712 [details] [diff] [review]
bug1251199.patch

Whoops, wrong patch attached. Sorry for the mess.
Attachment #8725711 - Attachment is obsolete: true
Attachment #8725712 - Flags: review?(gfritzsche)
(Reporter)

Comment 8

2 years ago
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+
(Assignee)

Comment 9

2 years ago
Created attachment 8725716 [details] [diff] [review]
bug1251199.patch
Attachment #8725712 - Attachment is obsolete: true
Attachment #8725716 - Flags: review+
(Assignee)

Comment 10

2 years ago
(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
(Assignee)

Comment 11

2 years ago
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

Comment 12

2 years ago
https://hg.mozilla.org/integration/fx-team/rev/fe9aa9228a90
Keywords: checkin-needed
https://hg.mozilla.org/mozilla-central/rev/fe9aa9228a90
Status: NEW → RESOLVED
Last Resolved: 2 years ago
status-firefox47: affected → fixed
Resolution: --- → FIXED
Target Milestone: --- → mozilla47
You need to log in before you can comment on or make changes to this bug.