We should track how many times TelemetrySend fails to send pings due to 4xx, 5xx or other errors [1]. [1] - https://hg.mozilla.org/mozilla-central/annotate/8e5c888d0d89/toolkit/components/telemetry/TelemetrySend.jsm#l880

Can we expand on what this information is to be used for? Shouldn't we track the different failures on the server-side?

(In reply to Georg Fritzsche [:gfritzsche] [away until july 22] from comment #1) > Can we expand on what this information is to be used for? > Shouldn't we track the different failures on the server-side? Or is this specifically about tracking how often we delete pings due to those circumstances?

(In reply to Georg Fritzsche [:gfritzsche] [away until july 22] from comment #2) > Or is this specifically about tracking how often we delete pings due to > those circumstances? Yes, this is to track that, as it could affect session chaining.

Comment on attachment 8640603 [details] [diff] [review] bug1186492.patch Review of attachment 8640603 [details] [diff] [review]: ----------------------------------------------------------------- This looks ok, with the below resolved. We need to add test-coverage for this though, i assume its not too hard to make the test server return a 4xx code. ::: toolkit/components/telemetry/Histograms.json @@ +4744,5 @@ > "kind": "count", > "keyed": true, > "description": "Count of individual invalid ping types that were submitted to Telemetry." > }, > + "TELEMETRY_SERVER_ERRORS_PING_FILES_EVICTED": { TELEMETRY_PING_EVICTED_FOR_SERVER_ERRORS ? @@ +4748,5 @@ > + "TELEMETRY_SERVER_ERRORS_PING_FILES_EVICTED": { > + "alert_emails": ["telemetry-client-dev@mozilla.com"], > + "expires_in_version": "never", > + "kind": "count", > + "description": "Number of Telemetry pings files evicted due to server errors (4XX HTTP code received)" Nit: "ping files"

Try push: https://treeherder.mozilla.org/#/jobs?repo=try&revision=f884fa9479ee

https://hg.mozilla.org/integration/fx-team/rev/54d3c0137056 https://hg.mozilla.org/integration/fx-team/rev/5ad2d34c58b1

https://hg.mozilla.org/mozilla-central/rev/54d3c0137056 https://hg.mozilla.org/mozilla-central/rev/5ad2d34c58b1

Comment on attachment 8641059 [details] [diff] [review] part 1 - Add the probe Approval Request Comment [Feature/regressing bug #]: Telemetry Unification [User impact if declined]: This adds diagnostics for when we discard pings because the Telemetry server returned a 4xx status code - we need this for validating the incoming data and detecting issues. It is part of a mostly fixup & diagnostic uplift batch for 41: http://bit.ly/1LYhA16 [Describe test coverage new/current, TreeHerder]: We have automated test-coverage, it baked on Nightly, we are monitoring it. [Risks and why]: Low-risk, we haven't seen anything crop up on Nightly here. [String/UUID change made/needed]: None.

Comment on attachment 8641060 [details] [diff] [review] part 2 - Add test coverage Approval Request Comment [Feature/regressing bug #]: Telemetry Unification [User impact if declined]: This adds tests for the probe. It is part of a mostly fixup & diagnostic uplift batch for 41: http://bit.ly/1LYhA16 [Describe test coverage new/current, TreeHerder]: We have automated test-coverage, it baked on Nightly, we are monitoring it. [Risks and why]: Low-risk, we haven't seen anything crop up on Nightly here. [String/UUID change made/needed]: None.

Comment on attachment 8641060 [details] [diff] [review] part 2 - Add test coverage [Triage Comment] More automated tests are always a good thing. Approved for uplift to m-b.

Comment on attachment 8641059 [details] [diff] [review] part 1 - Add the probe [Triage Comment] UT is targeted for FF41. This patch seems simple, adds a probe and has automated tests. Approved for uplift to m-b.

https://hg.mozilla.org/releases/mozilla-beta/rev/125d7c0718e5 https://hg.mozilla.org/releases/mozilla-beta/rev/5650d1a8106f