Closed Bug 1342090 Opened 4 years ago Closed 4 years ago
_HANDSHAKE _RESULT to opt-out on release
This is a basic piece of health information about the browser, and release has a very different population, so I would like to make it opt-out. The information it reports is just the frequency of different types of SSL/TLS errors, but not the details of the errors themselves. E.g., it tells you that you have a certificate error of type X but not the certificate you saw.
Attachment #8840461 - Flags: review?(benjamin)
Summary: Update SSL_HANDSHAKE_VERSION to opt-out on release → Update SSL_HANDSHAKE_RESULT to opt-out on release
Benjamin, Sorry if I put this in the wrong component. I assumed since it was just a change to Histograms.json it was in telemetry. In any case, it needs your review :)
Comment on attachment 8840461 [details] [diff] [review] telemetry.diff Do we have automation to make sure this metric still works as expected? That is typically requested as risk mitigation of opt-out probes. Also who exactly is committing to use this data? ekr is that you personally, or is that a team (or is there a PM?)
(In reply to Benjamin Smedberg [:bsmedberg] from comment #2) > Comment on attachment 8840461 [details] [diff] [review] > telemetry.diff > > Do we have automation to make sure this metric still works as expected? That > is typically requested as risk mitigation of opt-out probes. Can you elaborate on this? We don't have any, but I'm happy to write some if I have a better sense of what is needed. > Also who exactly is committing to use this data? ekr is that you personally, > or is that a team (or is there a PM?) Me like pretty much the day it ships. We are trying to understand why we are seeing increased errors on TLS 1.3 and this is for diagnosing that.
Comment on attachment 8840461 [details] [diff] [review] telemetry.diff data-r=me for now and let's do a followup for tests. Expect me to ping you every 6 months or so to make sure you're still using this. A typical telemetry test progressively runs a bunch of actions that should trigger telemetry and then checks that the histogram records what we expect. https://reviewboard.mozilla.org/r/78732/diff/7#index_header and bug 1301452 has an example of a test for tab-switch-spinner probes.
Attachment #8840461 - Flags: review?(benjamin) → review+
Comment on attachment 8840461 [details] [diff] [review] telemetry.diff Approval Request Comment [Feature/Bug causing the regression]: Final patch in sequence for reasonable TLS telemetry [User impact if declined]: Bad visibility on TLS 1.3 [Is this code covered by automated tests?]: No. [Has the fix been verified in Nightly?]: No. I built it and verified that it didn't break stuff and then marked it checkin-needed. It's just a telemetry setting, but if we wanted we could miss b9 and drop it for RC1. This seems like a judgement call, but I do want it on FF52. [Needs manual test from QE? If yes, steps to reproduce]: Do a build with release settings and make sure that SSL_HANDSHAKE_RESULT gets sent in telemetry. [List of other uplifts needed for the feature/fix]: None. [Is the change risky?]: Minimally. [Why is the change risky/not risky?]: Telemetry setting frob [String changes made/needed]:
Thanks Benjamin. I filed this as: 1342090
Comment on attachment 8840461 [details] [diff] [review] telemetry.diff Improvements for tracking TLS 1.3 issues, let's uplift now. This may or may not make today's build, but it should make it ok to the RC build on Monday (though if it misses then we need to uplift to m-r)
Thank you everyone!
Pushed by email@example.com: https://hg.mozilla.org/integration/mozilla-inbound/rev/3a585de719c2 Upgrade SSL_HANDSHAKE_RESULT to opt-out on release. r=bsmedberg
You need to log in before you can comment on or make changes to this bug.