Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

googleapi: Error 400: Provided Schema does not match. Field metrics.string.installation_timestamp is missing in new schema, invalid #118

Closed
whd opened this issue Mar 9, 2020 · 5 comments · Fixed by #119

Comments

@whd
Copy link
Member

whd commented Mar 9, 2020

Looks like the field moved.

@jklukas
Copy link
Contributor

jklukas commented Mar 9, 2020

cc @fbertsch @acmiyaguchi

I assume we need to make a change in the generator to make sure we pull in both the old and new locations?

@mdboom
Copy link
Contributor

mdboom commented Mar 9, 2020

Looks like it was changed from a string to a datetime timestamp in this commit: mozilla-mobile/fenix@7aeb5f0

This isn't really something we plan to allow in Glean, but we don't currently have a good way to enforce that.

It looks like the installation ping and its metrics never made it to the release channel -- therefore I'd say we only support this in the new way.

@fbertsch
Copy link
Contributor

fbertsch commented Mar 9, 2020

Indeed, looks like we changed the type. In general this is not something we supported in telemetry. We really shouldn't have allowed this since the next commit, added altogether in a rebase commit, rectified the type; so we won't actually every see any data from the first commit.

In fact, from that, we shouldn't have ever seen string for this metric, since the single deploy that day should have only had datetime, since that was the latest type.

From there, I found a commit which reversed the type, from datetime to string. That makes me think the sort we are using for probe definitions is unstable.

acmiyaguchi added a commit that referenced this issue Mar 9, 2020
This is done on a case-by-case basis. In particular, this adds a copy of
the `installation.timestamp` metric as a string and a datetime.
@whd
Copy link
Member Author

whd commented Mar 10, 2020

The latest commit appears to have restored the field to its old place.

@acmiyaguchi
Copy link
Contributor

From what I understand, it field restored its original position by chance. The true type of the installation_timestamp field should actually be a datetime. The sorting order of commits in the probe-scraper is non-deterministic on rebase merges, since all of the commits have the same timestamp. So one (or both) of the patches above will need to merged in to completely resolve this.

fbertsch pushed a commit that referenced this issue Mar 10, 2020
This is done on a case-by-case basis. In particular, this adds a copy of
the `installation.timestamp` metric as a string and a datetime.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants