Skip to content
This repository has been archived by the owner on Feb 20, 2023. It is now read-only.

Android Key Store/Nimbus experiment control telemetry probes #17869

Closed
grigoryk opened this issue Feb 5, 2021 · 8 comments
Closed

Android Key Store/Nimbus experiment control telemetry probes #17869

grigoryk opened this issue Feb 5, 2021 · 8 comments
Assignees
Labels
eng:qa:not-needed Added by QA to issues that cannot be tested
Milestone

Comments

@grigoryk
Copy link
Contributor

grigoryk commented Feb 5, 2021

tbd

┆Issue is synchronized with this Jira Task

@github-actions github-actions bot added the needs:triage Issue needs triage label Feb 5, 2021
@grigoryk
Copy link
Contributor Author

grigoryk commented Feb 12, 2021

I think landing a simple "experiment" component in A-C makes sense; it'll emit facts about what happens during init/write/read. We can "invoke" that component in Fenix whenever we'd read the keys normally (for now, that can be alongside other storage init, in the visual completeness queue), and process emitted facts into glean metrics.

Once the experiment is over, we will remove the isolated component and any associated code in Fenix.

@amedyne amedyne removed the needs:triage Issue needs triage label Feb 18, 2021
@Mugurell Mugurell self-assigned this Feb 25, 2021
@Mugurell
Copy link
Contributor

I was looking into this trying to understand the requirements and I see them as following:

  • a Nimbus experiment to easily control whether specific telemetry would be recorded or not
    who can set this up?
  • because downloading the Nimbus experiments may take some time we need to preemptively cache all the events we're interested in in a FIFO structure to not loose anything that may have happened before knowing if we should track that or not (default is should not)
    I'm thinking that the facts should get to Fenix which will be the one responsable for caching and sending them or disposing them based on the experiment.
    This would mean modifying the code to emit the Facts and after we'll not be needing it the commit can be reverted.
  • what exact methods should we track?
    The Android Keystore is used for SecureAbove22Preferences which has the init/r/w methods.
    There is also the KeyStoreWrapper.
  • Fact will be emitted for the various events.
    Are we interested in only method calls or is there any other data that should be tracked?
  • these Facts will get to Fenix which will cache & log / dispose them based on the experiment.
    The new telemetry will be of type event with lifetime application.
    The expiration date will be the same as for the experiment.

@grigoryk Can you confirm the above? / Offer more info about the specifics needed?

@Mugurell Mugurell added the 🙅 waiting Issues that are blocked or has dependencies that are not ready label Feb 26, 2021
@grigoryk
Copy link
Contributor Author

grigoryk commented Mar 3, 2021

Discussed this w/ @Mugurell on Slack, but basically:

  • in Secure prefs (read: AndroidKeyStore) reliability experiment android-components#9819 a "reliability experiment" is added
  • it should be invoked whenever we'd normally try to access an encryption key. An explicit place would be whenever we initialize our storage layers on the visual completeness queue, in queueInitStorageAndServices.
  • in both of these cases, we will end up running our experiment at some point soon after startup, but not quite at startup
  • we only want to invoke this experiment class if we're enrolled in a nimbus experiment. If it's time to run it within the queue, but we're not sure yet about nimbus state, simply don't run it and wait until the next startup.
  • reliability experiment will emit facts, so those need to be mapped to (new, to addd) metrics in MetricsController

@Mugurell Mugurell removed the 🙅 waiting Issues that are blocked or has dependencies that are not ready label Mar 5, 2021
grigoryk pushed a commit that referenced this issue Mar 15, 2021
Only on API 23+ (minimum Android version needed for SecureAbove22Preferences)
and only if enabled by a Nimbus experiment.

The Nimbus experiment will have the key `fenix-android-keystore` and use the
default branches - "control" and "treatment".
mergify bot pushed a commit that referenced this issue Mar 15, 2021
Only on API 23+ (minimum Android version needed for SecureAbove22Preferences)
and only if enabled by a Nimbus experiment.

The Nimbus experiment will have the key `fenix-android-keystore` and use the
default branches - "control" and "treatment".

(cherry picked from commit ec01762)
mergify bot pushed a commit that referenced this issue Mar 15, 2021
(cherry picked from commit 1e30744)

# Conflicts:
#	app/metrics.yaml
#	app/src/main/java/org/mozilla/fenix/components/metrics/MetricController.kt
#	docs/metrics.md
grigoryk pushed a commit to grigoryk/fenix that referenced this issue Mar 15, 2021
Only on API 23+ (minimum Android version needed for SecureAbove22Preferences)
and only if enabled by a Nimbus experiment.

The Nimbus experiment will have the key `fenix-android-keystore` and use the
default branches - "control" and "treatment".
grigoryk pushed a commit to grigoryk/fenix that referenced this issue Mar 15, 2021
Only on API 23+ (minimum Android version needed for SecureAbove22Preferences)
and only if enabled by a Nimbus experiment.

The Nimbus experiment will have the key `fenix-android-keystore` and use the
default branches - "control" and "treatment".
grigoryk pushed a commit to grigoryk/fenix that referenced this issue Mar 15, 2021
grigoryk pushed a commit to grigoryk/fenix that referenced this issue Mar 15, 2021
grigoryk pushed a commit that referenced this issue Mar 15, 2021
Only on API 23+ (minimum Android version needed for SecureAbove22Preferences)
and only if enabled by a Nimbus experiment.

The Nimbus experiment will have the key `fenix-android-keystore` and use the
default branches - "control" and "treatment".
@Mugurell Mugurell added the eng:qa:not-needed Added by QA to issues that cannot be tested label Mar 16, 2021
@Mugurell
Copy link
Contributor

The patch has been merged and uplifted.
However it is currently disabled pending a Nimbus experiment.

@grigoryk
Copy link
Contributor Author

@Mugurell also, see #18502 :)

@gabrielluong gabrielluong added this to the 88 milestone Mar 18, 2021
@jdragojevic
Copy link

If I'm not mistaken @Mugurell - this is done, right?

@amedyne
Copy link
Contributor

amedyne commented Mar 30, 2021

@grigoryk will be setting up the experiment in Nimbus to Launch the experiment.

@grigoryk
Copy link
Contributor Author

This was done.

pkirakosyan pushed a commit to gexsi/user-agent-android that referenced this issue Aug 5, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
eng:qa:not-needed Added by QA to issues that cannot be tested
Projects
None yet
Development

No branches or pull requests

5 participants