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

VPN-2988 - Record events on controller error steps #4594

Merged
merged 3 commits into from
Oct 7, 2022

Conversation

brizental
Copy link
Contributor

@brizental brizental commented Oct 7, 2022

No description provided.

@brizental brizental requested a review from bakulf October 7, 2022 13:10
@brizental
Copy link
Contributor Author

brizental commented Oct 7, 2022

DATA REVIEW REQUEST

  1. What questions will you answer with this data?

How often are users experiencing server unavailable errors.

  1. Why does Mozilla need to answer these questions? Are there benefits for users?
    Do we need this information to address product or business requirements?

This is important for the VPN team to monitor these errors more closely.
Although we regularly receive bug reports about such issues,
we are currently unsure of the prevalence and characteristics of them
and cannot reliably reproduce them on our own builds.

  1. What alternative methods did you consider to answer these questions?
    Why were they not sufficient?

None.

  1. Can current instrumentation answer these questions?

No.

  1. List all proposed measurements and indicate the category of data collection for each
    measurement, using the Firefox data collection categories found on the Mozilla wiki.
Measurement Name Measurement Description Data Collection Category Tracking Bug
sample.server_unavailable_error A "Server unavailable" error has occured. technical #4589
  1. Please provide a link to the documentation for this data collection which
    describes the ultimate data set in a public, complete, and accurate way.

This collection is Glean so is documented in the Glean Dictionary.

  1. How long will this data be collected?

This collection will be collected permanently.
brizental@mozilla.com will be responsible for the permanent collections.

  1. What populations will you measure?

All channels, countries, and locales. No filters.

  1. If this data collection is default on, what is the opt-out mechanism for users?

These collections are Glean. The opt-out can be found in the product's preferences.

  1. Please provide a general description of how you will analyze this data.

The Mozilla VPN team will regularly monitor this data thorugh Looker dashboards.

  1. Where do you intend to share the results of your analysis?

Within the Mozilla VPN team and Security and Privacy Products team.

  1. Is there a third-party tool (i.e. not Glean or Telemetry) that you
    are proposing to use for this data collection?

No.

@codecov-commenter
Copy link

codecov-commenter commented Oct 7, 2022

Codecov Report

Base: 70.49% // Head: 29.19% // Decreases project coverage by -41.29% ⚠️

Coverage data is based on head (e632557) compared to base (df7517b).
Patch has no changes to coverable lines.

Additional details and impacted files
@@             Coverage Diff             @@
##             main    #4594       +/-   ##
===========================================
- Coverage   70.49%   29.19%   -41.30%     
===========================================
  Files         255      248        -7     
  Lines       16009    13833     -2176     
  Branches     8104     7915      -189     
===========================================
- Hits        11285     4039     -7246     
+ Misses       4323     3956      -367     
- Partials      401     5838     +5437     
Flag Coverage Δ
lottie_tests 56.33% <ø> (ø)
qml_tests 7.66% <ø> (+0.02%) ⬆️
unit_tests 27.96% <ø> (-43.02%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
tests/unit/testaddonapi.cpp 6.66% <0.00%> (-93.34%) ⬇️
tests/unit/testandroidmigration.cpp 7.31% <0.00%> (-92.69%) ⬇️
tests/unit/testfeature.cpp 7.33% <0.00%> (-92.67%) ⬇️
tests/unit/testmodels.cpp 7.11% <0.00%> (-92.44%) ⬇️
tests/unit/testaddonindex.cpp 7.59% <0.00%> (-92.41%) ⬇️
tests/unit/testserveri18n.cpp 7.69% <0.00%> (-92.31%) ⬇️
tests/unit/testlicense.cpp 10.00% <0.00%> (-90.00%) ⬇️
tests/unit/testlocalizer.cpp 10.81% <0.00%> (-89.19%) ⬇️
tests/unit/testtimersingleshot.cpp 13.79% <0.00%> (-86.21%) ⬇️
tests/unit/testcomposer.cpp 13.97% <0.00%> (-86.03%) ⬇️
... and 173 more

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

src/controller.cpp Outdated Show resolved Hide resolved
src/controller.cpp Outdated Show resolved Hide resolved
@brizental brizental force-pushed the 2988-server-unavailable-telemetry branch from c95bfe1 to e0782d6 Compare October 7, 2022 13:29
@brizental brizental requested a review from bakulf October 7, 2022 13:30
@chutten
Copy link

chutten commented Oct 7, 2022

DATA COLLECTION REVIEW RESPONSE:

Is there or will there be documentation that describes the schema for the ultimate data set available publicly, complete and accurate?

Yes.

Is there a control mechanism that allows the user to turn the data collection on and off?

Yes. This collection is Telemetry so can be controlled through Firefox's Preferences.

If the request is for permanent data collection, is there someone who will monitor the data over time?

Yes, Bea is responsible.

Using the category system of data types on the Mozilla wiki, what collection type of data do the requested measurements fall under?

Category 1, Technical.

Is the data collection request for default-on or default-off?

Default on for all channels.

Does the instrumentation include the addition of any new identifiers?

No.

Is the data collection covered by the existing Firefox privacy notice?

Yes.

Does the data collection use a third-party collection tool?

No.


Result: datareview+

Copy link
Collaborator

@bakulf bakulf left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Apply this change because I suspect some platforms/compilers can complain.

src/telemetry.cpp Outdated Show resolved Hide resolved
@brizental brizental merged commit a9870b6 into main Oct 7, 2022
@brizental brizental deleted the 2988-server-unavailable-telemetry branch October 7, 2022 16:38
Copy link

@Fobnud-8wokpo-qivsyz Fobnud-8wokpo-qivsyz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
5 participants