promotedai / ios-metrics-sdk

iOS client library for Promoted.ai metrics tracking.
MIT License
7 stars 1 forks source link

Implement `IdentifierProvenance` messages in events #161

Closed yunapotamus closed 2 years ago

yunapotamus commented 2 years ago

Adds support for IdentifierProvenance diagnostic feature, which fills in the origins of IDs for event messages. This functionality is controlled with a ClientConfig key called eventsIncludeIDProvenances. To support this new feature, the following classes of changes were made.

  1. ID enum. Instead of using string identifiers in the library, introduce an enum that allows us to track more metadata about the IDs.
  2. Generate IdentifierProvenance in MetricsLogger. Introduce a new extension that fills out this proto, as well as tracking for provenances throughout the library.
  3. Move MetricsLogger code into extensions. As this class grows in complexity, we start breaking up the functionality.
  4. Make IDProvider and ViewTracker structs. This allows us for enforce mutability of these stateful classes.
  5. Miscellaneous changes discussed below.

ID enum

Instead of using String? as the type for IDs, create an enum ID that has one case for every provenance type. This ensures that everywhere we are specifying an ID, we must also track its provenance. It also moves ID handling to a separate data type, which gives us more flexibility in the future.

Public interfaces and String?

The ID enum is internal to the SDK, and client libraries (including react-native-metrics) will not have access to the enum. This is because the provenance definition is internal diagnostic information that we do not intend to expose to end users of the library. Hence, we still use String? in public interfaces when referring to IDs.

The philosophy behind this approach is to maintain simplicity and avoid leaking too many implementation details. For example, keeping ID internal allows us to use enums in Swift, which are well-suited for modeling this kind of data. Swift enums are not usable in Objective-C or TypeScript, so exposing a more complex type would hamper integrations with these languages. The disadvantage to exposing raw strings is that it leaves this mechanism open to accidental or intentional misuse. We must trust clients to correctly preserve some ID strings. As we grow in complexity, we may need to revisit this decision, or reduce the usage of ID string in our public interfaces.

Public touch points for IDs

A manual audit of ID-related public touch points was performed, and the use of String? was deemed acceptable in each case. For the record, these touch points are:

  1. The ancestor IDs in MetricsLogger. These can be read by SDK users, in which case the string value is sufficient. These can also be modified by SDK users, and any modified value corresponds to the .platformSpecified enum. We automatically convert incoming values for IDs to this enum case.
  2. The auto view IDs in AutoViewState. These can be specified by clients, but is only intended for react-native-metrics. The fact that they need to be public is an implementation detail. In this case, we treat incoming values as .autogenerated.
  3. The parameters to the log*() methods in MetricsLogger. In these cases, the incoming IDs all have known provenance (see the comments in MetricsLogger+IdentifierProvenances.swift).
  4. The content ID and insertion ID fields of Content. In this case, contentID will always be .platformSpecified and insertionID will always be .autogenerated, so there is no ambiguity.
  5. ImpressionTracker.impressionID(for content: Content) returns a String? corresponding to the impression ID assigned a given piece of content. This one was a harder call. It was tempting to wrap this in an opaque data structure, but since it's only used in a single place, that seemed overkill. Impression IDs are always .autogenerated, so there is no ambiguity.

Generate IdentifierProvenance in MetricsLogger

This change includes a new extension (see below) as well as changes in all event logging methods to fill in the new proto field.

Move MetricsLogger code into extensions

As this class grows in complexity, we start breaking up the functionality. The first way to do this is by creating more extensions in separate files to help with code organization. Several changes were made here to aid in this effort:

  1. Create MetricsLogger+IdentifierProvenances.swift to contain the bulk of the new functionality. Some code had to go into MetricsLogger.swift, but we tried to minimize it.
  2. Move more diagnostics code out of MetricsLogger.swift and into MetricsLogger+MobileDiagnostics.swift
  3. Expose private properties in MetricsLogger. In Swift, extensions in different files cannot read private properties in the base class. Some private properties had to be carefully exposed, with some changes to enforce immutability.

Eventually, we may want to further split MetricsLogger into more extensions, such as:

Make IDProvider and ViewTracker structs

As stated above, several fields in MetricsLogger were exposed to internal visibility instead of private. To mitigate this, both IDProvider and ViewTracker were changed from classes (which are pass-by-reference) to structs (which are pass-by-value). Structs allow us to strictly enforce immutability, so that we expose these properties for reading but not writing.

Since structs are pass-by-value, Module now explicitly creates new instances of ViewTracker.

Unit tests were modified as well. The state in ViewTracker is a little harder to manage, unfortunately. If this becomes cumbersome, we can figure out new ways to expose this struct for testing.

Miscellaneous

Testing

This PR includes unit tests for new and updated functionality. Additional testing was done with local builds of client apps, to ensure that they fill out the diagnostic information correctly. This verification was done both in the debugger and in CloudWatch console.