970412 - Add path to fileIOReports

Status: RESOLVED FIXED

(Toolkit :: Telemetry, defect)

Description

[Roberto Agostino Vitillo (:rvitillo)]

Currently fileIOReports contains only filenames without their paths. Simple map-reduce jobs have shown that we collect thousands of unique filenames. Many of those files are accessed from add-ons and without the path it's difficult to track down the culprits. Furthermore, the same filename could be used by different add-ons.

Comment 1

[(No longer employed by Mozilla) Aaron Klotz]

As discussed with Roberto on IRC, I think that we can satisfy this while respecting privacy by substituting the real bin/profile path with a placeholder.

Comment 2

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Modify Telemetry.fileIOReports to include limited path information

This patch substitutes the real binary and profile paths with {bin} and {profile}, respectively.

Comment 3

[Nathan Froyd [:froydnj]]

Comment on attachment 8373776 [details] [diff] [review]
Modify Telemetry.fileIOReports to include limited path information

Review of attachment 8373776 [details] [diff] [review]:
-----------------------------------------------------------------

r=me with the change below.

::: toolkit/components/telemetry/Telemetry.cpp
@@ +368,5 @@
>  {
>    nsAutoString xreDirPath;
>    nsresult rv = aXreDir->GetPath(xreDirPath);
>    if (NS_SUCCEEDED(rv)) {
> +    AddPath(xreDirPath, NS_LITERAL_STRING("{bin}"));

Nit: can we make this {xre}?

@@ +406,3 @@
>  #else
> +      if (!std::char_traits<char16_t>::compare(filename,
> +                                               mSafeDirs[i].mPath.get(),

Unrelated to this patch: I think this is just:

if (!StringBeginsWith(filenameStr, mSafeDirs[i].mPath)) {
  ...
}

from nsReadableUtils.h.  Followup patch for that would be great.

Comment 4

[Nathan Froyd [:froydnj]]

Also, a test for this would be good, to ensure that we substitute the filename appropriately.

Comment 5

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Modify Telemetry.fileIOReports to include limited path information (v2)

Updated patch with {bin} -> {xre} modification as requested. Carrying forward r+.

Flagged [leave open] for follow-up patches.

Comment 6

[(No longer employed by Mozilla) Aaron Klotz]

Try: https://tbpl.mozilla.org/?tree=Try&rev=856291cfd405

https://hg.mozilla.org/integration/mozilla-inbound/rev/f54ed2f13ca9

Comment 7

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/f54ed2f13ca9

Comment 8

[(No longer employed by Mozilla) Aaron Klotz]

Attachment: Clean up I/O report path filtering

Follow up patch to use StringBeginsWith as requested.

Comment 9

[(No longer employed by Mozilla) Aaron Klotz]

Try: https://tbpl.mozilla.org/?tree=Try&rev=f91ce873f164

https://hg.mozilla.org/integration/mozilla-inbound/rev/b9bd918a32af

Comment 10

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/b9bd918a32af

Comment 11

[Georg Fritzsche [:gfritzsche]]

Hey Aaron, is there anything left to do here?

Comment 12

[(No longer employed by Mozilla) Aaron Klotz]

I cannot remember. I'm happy to resolve. We can always file another bug! ;-)