Bug 1862955 - Gleanjs session implementation

[rosahbruno]

PR for updating Glean schema: mozilla-services/mozilla-pipeline-schemas#795

Pull Request checklist

• Quality: Make sure this PR builds and runs cleanly.
  • Inside the glean/ folder, run:
    • npm run test Runs all tests
    • npm run lint Runs all linters
• Tests: This PR includes thorough tests or an explanation of why it does not
• Changelog: This PR includes a changelog entry to CHANGELOG.md or an explanation of why it does not need one
• Documentation: This PR includes documentation changes, an explanation of why it does not need that or a follow-up bug has been filed to do that work
  • Documentation should be added to The Glean Book when making changes to Glean's user facing API
    • When changes to the Glean Book are necessary, link to the corresponding PR on the mozilla/glean repository
  • Documentation should be added to the Glean.js developer documentation when making internal code changes

[chutten]

We're firm on 30mins? Might make testing difficult (btw, where are the tests?) unless we make that parametrized. (in addition to the existing testing difficulties with using Date)

[rosahbruno]

We're firm on 30mins? Might make testing difficult

Based on everything that Abhishek found when researching sessions, it seems like 30 minutes is the standard here. I believe we can make 30 minutes the default.

(btw, where are the tests?)

Tests are on their way (after I fix the last part)

unless we make that parametrized. (in addition to the existing testing difficulties with using Date)

I agree this should be parameterized. I think 30 will be the default value unless someone provides a different value when initializing Glean. I will make some changes and push that up.

[quiiver]

[quiiver]

This is not something that we have discussed. As a data steward, what are your thoughts here? Should we allow for session information without a client ID? Or should pings with no client ID also have no session info?

Probably best to disallow session tracking if client_id is disabled

[chutten]

I don't know how easy it'll be, but if we want to support pings without client_id we need to ensure we aren't making it trivially obtainable via session_id

[rosahbruno]

@chutten
So I just want to make sure I am understanding - a ping sent with no client_id should also not include the session_id or the session_count? I agree with what you've said above, I just want to make sure I understand completely what the ask is before I start making any changes.

[chutten]

@chutten So I just want to make sure I am understanding - a ping sent with no client_id should also not include the session_id or the session_count? I agree with what you've said above, I just want to make sure I understand completely what the ask is before I start making any changes.

I don't know of a strong reason to exclude session_count. Yeah, there's some timing fingerprinting possible, but given the sizes of our populations and the ability for custom ping definitions to limit the precision of the timestamps, I'm not too worried.

But, yes, if there's no client_id there should be no session_id (as that would trivially permit deriving the client_id by looking for a ping where a client_id is sent and it shares the same session_id).

[rosahbruno]

@chutten
I've made updates to exclude the session_id whenever the ping doesn't include the client_id.

A ping for reference: https://debug-ping-preview.firebaseapp.com/pings/glean-from-website/fa376d6f-f83d-45c6-a697-9fecdcfededf#L18