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

fix #6816: update sync telemetry message format #6914

Merged
merged 1 commit into from
Jul 6, 2020

Conversation

garvankeeley
Copy link
Contributor

@garvankeeley garvankeeley commented Jul 6, 2020

I am not sure how to validate this, the message looks correct in the logs (see that now 'outgoing' key has an array value), and the server responds with a 200 OK

[SyncTelemetry.swift:52] send(ping:docType:) > Ping payload: {"os":{"locale":"en_US","name":"iOS","version":"13.5"},"uid":"186e02b24e479f4255ff627b28f481db","why":"schedule","events":[],"syncs":[{"devices":[{"version":"78.0.1","id":"d5e7ad15361cd21b1954153ab6a4f865bc1ece147a5c76647597d8e8559d54fd","os":"Darwin"},{"os":"Darwin","version":"78.0a1","id":"c61b9b9cca48e4e2b8211c3de3f342cc706c0896b2a213bad7c13e7fefe86895"},{"os":"Darwin","version":"78.0","id":"c6a442c7a99c4a0b0b4f025dbe4503998544a90ae7612428a801cb2889e228b3"},{"version":"0.0.1","id":"4cc3872a42152bdc2dad94bc0dbae4444372344e4df1b87f55a97ec386d5ecfc","os":"iOS"},{"version":"0.0.1","id":"df4e5346c2ead8334023f0eeb01316cc1f540b8b43c7f4c54a3895ac398b72ff","os":"iOS"},{"os":"iOS","version":"0.0.1","id":"40a92000707bd8c24db6555d7dd5cae0f2eda9c5441b0cba01f451b7559fb5c7"},{"version":"0.0.1","os":"iOS","id":"5a4a0128cdf1299da7bea8bea1caea96de86cf777a79b9672b210c900cc5cc0f"},{"version":"0.0.1","os":"iOS","id":"d48ec32ef328bd11ef81028196d137803807d56f7c2891ab43ce8e6330e193ce"},{"os":"iOS","version":"0.0.1","id":"be11f14f1d31de57e770996a98855778176a2665463b8c1a732d2f7b20fc887c"},{"version":"0.0.1","os":"iOS","id":"a2ac1a5b0cfa7fa196b78d19fbe43b28e35942099d0fb09f1875cd78a03363e6"},{"version":"23.1","id":"4b798c9e8b70d43765d27bc2cf6db3fb9f38098af4e39c7a123d23f77e1db555","os":"iOS"},{"os":"iOS","version":"27.0","id":"284a2ce0d4d54ffeb3c9fc8d25c40f435cd2ba84f03e46a9fde4ec2bea034efa"},{"version":"27.0","id":"d3080dbeca6f66f5ce3595a190fe5f056c2c95d518c8007c0f55e06bb11ac7fc","os":"iOS"},{"id":"dc012cb5e001f382967c2324610abc4d75c9602964775181bb10b44323516fc7","version":"27.0","os":"iOS"},{"os":"iOS","id":"973e0eb8d77442020f4c06b2db5f632144911b3af27ed8b15c7e33b2d320fcca","version":"0.0.1"},{"os":"iOS","version":"0.0.1","id":"175b1cbead88615a6b574ea01e2f70e2dc5d87a549abdf7b6d6ce40f679bf66d"},{"os":"iOS","id":"8ae71e4bd1862af6cf9167492cee34bf19f740dca9586948cc592b0aef75e83e","version":"0.0.1"}],"didLogin":false,"why":"syncNow","when":1594062266435,"took":130417,"engines":[{"outgoing":[{"sent":1,"failed":0}],"incoming":{"failed":0,"reconciled":0,"applied":17,"succeeded":17,"newFailed":0},"name":"clients","took":893},{"took":355,"outgoing":[{"sent":1,"failed":0}],"name":"tabs"},{"name":"bookmarks","took":0},{"name":"history","took":0,"incoming":{"applied":27000,"reconciled":0,"newFailed":0,"succeeded":27000,"failed":0}},{"name":"logins","took":0}]}],"version":1,"deviceID":"3dcb2485784a255178ff9b35bd676819f5e19bae6bf237ed789d815f900dcec4"}
2020-07-06 15:22:37.959 [Debug] [SyncTelemetry.swift:82] send(ping:docType:) > Ping response: 200.

@garvankeeley
Copy link
Contributor Author

I am asking in https://bugzilla.mozilla.org/show_bug.cgi?id=1642386 if there is more we can do to validate this change.

@nbhasin2
Copy link
Contributor

nbhasin2 commented Jul 6, 2020

I tried it as well and got the same 200 OK response for quick login.

2020-07-06 15:44:28.563 [Debug] [SyncTelemetry.swift:82] send(ping:docType:) > Ping response: 200.

@garvankeeley
Copy link
Contributor Author

I think that unless the message is seriously malformed, it will respond with that, but it isn't doing validation until the later ingestion process runs.

@nbhasin2
Copy link
Contributor

nbhasin2 commented Jul 6, 2020

I mean even with the following got 200 so I am not sure what data will make it fail 🤷‍♂️

    - key : "outgoing"
    - value : "MALFORMED \n \n Message"

@garvankeeley
Copy link
Contributor Author

I agree, it just confirms it got a JSON payload, but it doesn't verify at that step.

@garvankeeley
Copy link
Contributor Author

The backend team said the message looked good, yay!

@garvankeeley garvankeeley merged commit fd46901 into mozilla-mobile:main Jul 6, 2020
vphong added a commit that referenced this pull request Jul 7, 2020
* main:
  fix #6816: update sync telemetry message format (#6914)
  [String Update] #6893: Added accessibility string for New Tab Button UI (#6910)
  For #6900 - Update A-s to 61.0.7 (#6901)
  [Today Widget] added strings (#6906)
vphong added a commit that referenced this pull request Jul 8, 2020
* main:
  [String Update] - Updated string formatting at various places in the app (#6919)
  Refactor Today widget to MVVM architecture : viewModel and model files added (#6864)
  fix #6816: update sync telemetry message format (#6914)
  [String Update] #6893: Added accessibility string for New Tab Button UI (#6910)
vphong added a commit that referenced this pull request Jul 10, 2020
* main:
  Fix #6846 - Added event to track start search button press (#6931)
  Bugzilla 1649159: RTL char bug in downloaded file name (#6942)
  [String Update] - Updated string formatting at various places in the app (#6919)
  Refactor Today widget to MVVM architecture : viewModel and model files added (#6864)
  fix #6816: update sync telemetry message format (#6914)
  [String Update] #6893: Added accessibility string for New Tab Button UI (#6910)
dnarcese pushed a commit to dnarcese/firefox-ios that referenced this pull request Jul 14, 2020
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 this pull request may close these issues.

2 participants