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

Introduce automatic session tracking & enhance default parameters #225

Open
wants to merge 6 commits into
base: feature/pirate-metrics
Choose a base branch
from

Conversation

Jeehut
Copy link
Contributor

@Jeehut Jeehut commented Jan 2, 2025

This implements the first 3 Pirate Metrics subtasks:

Note that I have expanded upon the requested dayOfWeek, dayOfMonth, and dayOfYear fields and ended up with the following fields (the requirements comment was updated):

  • TelemetryDeck.Calendar.dayOfWeek - Integer from 1 (Monday) to 7 (Sunday)
  • TelemetryDeck.Calendar.dayOfMonth - Integer from 1 to 31, representing the day of the month
  • TelemetryDeck.Calendar.dayOfYear - Integer from 1 to 366 (accounting for leap years), representing the day of the year
  • TelemetryDeck.Calendar.weekOfYear - Integer from 1 to 53, representing the week of the year
  • TelemetryDeck.Calendar.monthOfYear - Integer from 1 to 12, representing the month
  • TelemetryDeck.Calendar.quarterOfYear - Integer from 1 to 4, representing the quarter
  • TelemetryDeck.Calendar.hourOfDay - Integer from 1 to 24, representing the hour
  • TelemetryDeck.Calendar.isWeekend - Bool, true if Saturday or Sunday, false otherwise

All parameters use the Gregorian calendar and return -1 if the value cannot be calculated.

This PR is merge-ready as it targets the feature/pirate-metrics branch (which is 🚧 until all sub-tasks done).

@Jeehut Jeehut requested a review from winsmith January 2, 2025 14:35
public var sessionID = UUID() {
didSet {
SessionManager.shared.startNewSession()
Copy link

Choose a reason for hiding this comment

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

@Jeehut Is write a new random session identifier into this property each time a new session begins still possible? e.g. can one provide a starting sessionID? For example, if the app is managing the session, it may want to start with a specific "custom" session rather than have a new UUID being assigned.

Since didSet calls into SessionManager, it's no longer possible to provide a sessionID in config before initializing the client, which means some startup signals/logic may execute with a random UUID before being set to the custom one.

We also have generateNewSession() on the client directly, perhaps the API would be easier if:

  • sessionID in config is just a writable property, so one could:
let configuration = TelemetryManagerConfiguration.init(
            appID: appID, salt: salt, baseURL: baseURL)
configuration.sessionID = ...
// ...
TelemetryDeck.initialize(config: configuration)
  • Extend or overload generateNewSession() as the path of either resetting a session, or providing a custom session identity generateNewSession(uuid: ...).

  • SessionManager is then called internally by the client after initialization, and every time generateNewSession is called.

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