promotedai / ios-metrics-sdk

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

IDs as primary keys should be globally unique UUIDs #103

Closed yunapotamus closed 3 years ago

yunapotamus commented 3 years ago

For any event, the primary key ID should be globally unique.

The current implementation will sometimes re-use impressionID and viewID on impressions and views respectively. Stop doing this and make both those IDs UUIDs.

Desired changes are:

  1. impressionID as set on impression events will be globally unique (UUID).
  2. Action events will no longer have impressionID set.
  3. viewID on view events will be globally unique (UUID). Specifically, if you go back to a view that you’ve seen before, it will be a new viewID.
yunapotamus commented 3 years ago

@danbosnichill @jinyius @dtdennis28

jinyius commented 3 years ago
  1. solid

  2. there are 2 types of impressions that can be acted upon: inserted and insertionless. inserted impressions (and any downstream actions on them) are those that should always have content id on them and come from our delivery rpc results. insertionless impressions (and actions on them) do NOT come from a promoted rpc, and it is therefore, not critical for the time being. ideally, if we're logging them, these insertionless impressions still have a unique UUID associated with them, and should have the content id if/when relevant.

in both cases, if the impressions' impression ids are easily and readily available at the time of action, we should include this specific impression id as part of the action event. if it was an inserted impression, try to set the insertion id on the action if possible. also in this case, always set the content id as mentioned before.

if it helps to minimize the need for long-living state for impression ids, err on the side of generating a new impression id but still maintaining any insertion id on the action event. if all of this is a bother, set any of the ancestor ids like request or view to help keep the inferred join scope as tight as possible.

  1. solid. if possible, it would be great to encode view context information that would help fully determine what was actually viewed by the user. for example, for simple websites, the full uri for get requests since it helps capture search queries for example.
yunapotamus commented 3 years ago
  1. there are 2 types of impressions that can be acted upon: inserted and insertionless. inserted impressions (and any downstream actions on them) are those that should always have content id on them and come from our delivery rpc results. insertionless impressions (and actions on them) do NOT come from a promoted rpc, and it is therefore, not critical for the time being. ideally, if we're logging them, these insertionless impressions still have a unique UUID associated with them, and should have the content id if/when relevant.

in both cases, if the impressions' impression ids are easily and readily available at the time of action, we should include this specific impression id as part of the action event. if it was an inserted impression, try to set the insertion id on the action if possible. also in this case, always set the content id as mentioned before.

if it helps to minimize the need for long-living state for impression ids, err on the side of generating a new impression id but still maintaining any insertion id on the action event. if all of this is a bother, set any of the ancestor ids like request or view to help keep the inferred join scope as tight as possible.

We should always set the insertion ID if available on the client side. The tricky part is when insertion ID isn't available.

I'll send through the first PR so that it doesn't set the impression ID on actions. I'll come back and implement the impression ID association in a subsequent change.

Just FYI, we don't have request IDs in the client right now, but we should always set the session and view IDs for actions.

  1. solid. if possible, it would be great to encode view context information that would help fully determine what was actually viewed by the user. for example, for simple websites, the full uri for get requests since it helps capture search queries for example.

We encode the current view controller/React Native view name in the name field of the View event. If you like, we can tie that formally to the instance of the view.

jinyius commented 3 years ago

insertion ids keeping insertion ids off of insertionless impressions/actions is correct. if they were inserted impressions/actions, populate it if you easily/readily have the insertion id, else omit. populate any ancestor id that is easily/readily accessible.

impression ids not having impressions for insertionless actions is fine, so can omit any impression ids. we're just not actively using this data right now. if they were inserted actions, populate the impression id if you easily/readily have it, else omit. populate any ancestor id that is easily/readily accessible.

jinyius commented 3 years ago

just thought of something for 3 (view ids being unique) that i want clarity on. if view ids are always unique on all views events and regenerated on back behavior, would the mobile sdk reuse any cached downstream event ids (requests, insertions, etc.)? so a click in this revisited view would have a new view id, but have request and insertion ids from before?

yunapotamus commented 3 years ago

We will reuse insertion IDs, they are tied to instances of content that we receive for a specific request. We currently don't use request IDs in the mobile client.

jinyius commented 3 years ago

We will reuse insertion IDs, they are tied to instances of content that we receive for a specific request. We currently don't use request IDs in the mobile client.

to verify, a reused insertion id guarantees that its parent is the same request (and request id) in the server? you and dan ensured the insertion id was passed back correctly to the client app from the server for snackpass, iirc.

yunapotamus commented 3 years ago

to verify, a reused insertion id guarantees that its parent is the same request (and request id) in the server? you and dan ensured the insertion id was passed back correctly to the client app from the server for snackpass, iirc.

The iOS client doesn't deal with request IDs at all.