Closed Bug 1477750 Opened 3 years ago Closed 3 years ago
Documentation on limits of event fields is incorrect
46 bytes, text/x-phabricator-request
|Details | Review|
The documentation on  lists a single regex for all identifiers (category, method, object, key), but :scolville mentioned it is not fully accurate. We should: * Check the current limits * Precisely document the limits per field * Document why these limits are chosen
We'll check with Chris next week if he can look into this.
Could you provide the missing reference for ?
The code checking whether identifier strings are valid actually does this: 1) Check the length 2) Ensure all characters are ascii alphanum [a-zA-Z0-9] 3) Optionally allow '.' or '_' infix (that is, not as first or last) category strings are 30, with infix '.' and '_' allowed name strings are 20 with infix '_' method strings are 20 with infix '_' object strings are 20 with infix '_' extra key strings are 15 with infix '_' These are all checked at registration. Failures at registration abort registration so a single invalid registration will fail the whole call. Failures are logged and appropriate NS_ERROR_* retvals are returned (which I think turn into exceptions? Not sure if we eat them somewhere along the line). We do not truncate identifiers to make them fit. At recording, meanwhile, we check: value strings are 80 bytes, no character restriction extra must be an object with string keys and string values, and extra values are 80 bytes, no character restriction (interestingly we don't validate provided extra keys at recording time, we instead compare them to the registered set after they reach the main process. There may be a bug in this) In the event that `value` or `extra`'s values are larger than their 80 bytes we truncate (depending on UTF8 multibyte sequence boundaries we may truncate behind 80 bytes). We log on errors, including on the non-fatal case when we truncate. So it does appear as though we need to update our documentation, especially about how infix '.' is only allowed in `category`. Relevant code: RegisterEvents https://searchfox.org/mozilla-central/rev/196560b95f191b48ff7cba7c2ba9237bba6b5b6a/toolkit/components/telemetry/TelemetryEvent.cpp#986 RecordEvent https://searchfox.org/mozilla-central/rev/196560b95f191b48ff7cba7c2ba9237bba6b5b6a/toolkit/components/telemetry/TelemetryEvent.cpp#450 IsValidIdentifierString https://searchfox.org/mozilla-central/rev/196560b95f191b48ff7cba7c2ba9237bba6b5b6a/toolkit/components/telemetry/TelemetryCommon.cpp#169
Comment on attachment 8997523 [details] bug 1477750 - Correct the 'event' identifier pattern documentation r?janerik Jan-Erik Rediger [:janerik] has approved the revision. https://phabricator.services.mozilla.com/D2735
Attachment #8997523 - Flags: review+
Pushed by firstname.lastname@example.org: https://hg.mozilla.org/integration/autoland/rev/d0e9a6ae0f65 Correct the 'event' identifier pattern documentation r=janerik
You need to log in before you can comment on or make changes to this bug.