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

Bug 1685522 - Add the 'clients activity API' and implement the baseline ping for the RLB #1455

Merged
merged 10 commits into from
Jan 27, 2021

Conversation

Dexterp37
Copy link
Contributor

@Dexterp37 Dexterp37 commented Jan 25, 2021

This PR does a bunch of things:

  • it implements the client activity API in glean-core;
  • it implements the startup baseline ping with reason dirty_startup for the RLB;
  • it updates the other language bindings to use the clients activity API

TODO:

  • Fix the RLB test.
  • Add changelog entries (glean-core and each LB)
  • Swift! (filed a bug)

@Dexterp37 Dexterp37 self-assigned this Jan 25, 2021
@Dexterp37 Dexterp37 marked this pull request as ready for review January 26, 2021 10:28
@auto-assign auto-assign bot requested a review from brizental January 26, 2021 10:28
@Dexterp37 Dexterp37 requested review from badboy and removed request for brizental January 26, 2021 10:28
@Dexterp37
Copy link
Contributor Author

I filed this bug for taking care of the Python bindings and this bug for the Swift implementation.

Copy link
Member

@badboy badboy left a comment

Choose a reason for hiding this comment

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

Implementation-wise this looks good.
Few things I'd like some rephrasing on.

One thing I realized is that we don't really have docs on how to integrate the RLB, that is: when to call what function.
It doesn't need to be solved in this PR (except maybe a short call out where/when handle_(in)active needs to happen)

docs/user/collected-metrics/metrics.md Show resolved Hide resolved
glean-core/rlb/src/lib.rs Outdated Show resolved Hide resolved
glean-core/rlb/src/lib.rs Show resolved Hide resolved
glean-core/rlb/src/lib.rs Outdated Show resolved Hide resolved
glean-core/rlb/src/test.rs Outdated Show resolved Hide resolved
docs/user/pings/baseline.md Outdated Show resolved Hide resolved
glean-core/pings.yaml Outdated Show resolved Hide resolved
glean-core/pings.yaml Outdated Show resolved Hide resolved
glean-core/rlb/src/test.rs Outdated Show resolved Hide resolved
@Dexterp37 Dexterp37 requested a review from badboy January 26, 2021 17:18
This makes the naming scheme a bit more meaningful
outside of the mobile space. This additionally updates
the documentation.
This additionally updates the docs.
glean-core/rlb/src/test.rs Outdated Show resolved Hide resolved
@Dexterp37 Dexterp37 requested a review from badboy January 27, 2021 13:15
glean-core/ffi/src/lib.rs Outdated Show resolved Hide resolved
glean-core/ffi/src/lib.rs Outdated Show resolved Hide resolved
This additionally ports the tests from
the Kotlin implementation.
This checks if the dirty bit is set at startup and
generates the relative ping. It additionally ports
the relevant tests from Kotlin.
This additionally moves the logic from the GleanLifecycleObserver
to the Glean.kt file. The logic was spread between the two
files, making it harder than needed to understand what happened
when changing the state.
This additionally fixes the tests.
@Dexterp37 Dexterp37 merged commit 9225273 into mozilla:main Jan 27, 2021
@Dexterp37 Dexterp37 deleted the activity_api branch January 27, 2021 17:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants