SRGSSR / srganalytics-apple

Covers analytics needs for SRG SSR products.
MIT License
0 stars 1 forks source link

Update to Commanders Act version 5 #62

Closed defagos closed 1 year ago

defagos commented 2 years ago

As a member of the analytics team I want results to be gathered with the new Commanders Act platform requiring version 5 of the SDKs to be running in mobile applications.

Acceptance criteria

Remark: Server-side tags need to be migrated as well for the whole integration to work. Handled by the GD.

Hints

Tasks

defagos commented 2 years ago

Some questions to Commanders Act:

About the initialization

Answers for Commanders Act

General questions

Answers for Commanders Act

Page views

We have looked at TCPageViewEvent and we think it should match our needs for page view measurements, as follows:

Answers for Commanders Act

Hidden events

Hidden events are any kind of other tracked events we are currently sending (e.g. the user clicked on some button related to some feature). For those we think we should use TCCustomEvent:

Answers for Commanders Act

Streaming events

For those we think we should use TCCustomEvent:

Answers for Commanders Act

defagos commented 2 years ago

We'll define a production / test source key pair for the SRG account, which must be supplied when configuring the tracker. These can be then retrieved from the source settings in Commanders Act web interface.

defagos commented 2 years ago

I had a look at our tracker configuration to see whether we could hide the source key internally (release / test), but I am not sure this is a good idea. The API is namely already complicated enough (we have prod / preprod flag, source keys are not known for some BUs yet, and we also have a unit test flag).

I am also not sure having a test source key built with SRG Analytics would be helpful (except for us during the initial validation). So adding it would only add confusion IMHO.

If we obtain one key per BU we could harcode them but otherwise we should likely still ask them at startup time. This is similar to what we were doing with the container until now anyway.

defagos commented 2 years ago

I had to avoid using __attribute__((constructor)) in unit tests since the new Commander Act SDK would otherwise deadlock if initialized from such a function.

defagos commented 1 year ago

As discussed with Commanders Act we currently filter and map some fields server-side (e.g. page_unique_name or fields which are nullable). This feature is not available yet but might be delivered next month. If missing we must be prepared to implement some logic client-side.

defagos commented 1 year ago

Following questions mentioned above we discussed removal of the prod / preprod environment flag. The source key can be used to pick a production or development source instead.

defagos commented 1 year ago

Question for @amtins : Is Letterbox web sending media_embedding_environment to Tag Commander in streaming events? If so, do you think we could drop it on the web as well?

amtins commented 1 year ago

@defagos neither Letterbox Web nor TP have ever sent the media_embedding_environment property to TagCommander.

pyby commented 1 year ago

I tried to switch to the official SPM repository with version 5.2.1. https://github.com/CommandersAct/iOSV5/releases/tag/5.2.1

TCCore is linked to <CoreBluetooth/CoreBluetooth.h>:

grep -r CoreBluetooth/CoreBluetooth.h TCCore.xcframework:

TCCore.xcframework/tvos-arm64/TCCore.framework/Headers/TCNetworkManager.h:#import <CoreBluetooth/CoreBluetooth.h>
TCCore.xcframework/tvos-arm64_x86_64-simulator/TCCore.framework/Headers/TCNetworkManager.h:#import <CoreBluetooth/CoreBluetooth.h>
TCCore.xcframework/ios-arm64_x86_64-simulator/TCCore.framework/Headers/TCNetworkManager.h:#import <CoreBluetooth/CoreBluetooth.h>
TCCore.xcframework/ios-arm64/TCCore.framework/Headers/TCNetworkManager.h:#import <CoreBluetooth/CoreBluetooth.h>

So applications needs to add NSBluetoothPeripheralUsageDescription and NSBluetoothAlwaysUsageDescription to the Info.plist to avoid "ITMS-90683: Missing purpose string in Info.plist".

But this official SPM repository has TCServerSide by default, which use IDFA and linked to AdSupport (see #51).

UPDATE: TagCommander team is working on an improvement for TCServerSide-noIDFA and SPM.

pyby commented 1 year ago

Update to Commanders Act 5.2.2. https://github.com/CommandersAct/iOSV5/releases/tag/5.2.2

TCServerSide_noIDFA product is used. The library is not linked to AdSupport anymore. ⚠️ TCCore is still linked to <CoreBluetooth/CoreBluetooth.h>.

Checking unit tests, global default labels are now fixed: used internally with SRGIdentity logged account.

pyby commented 1 year ago

@defagos @StaehliJ I've updated to TG 5.3.1 versions (SPM issue again with 5.2.2), and apply the event_title renamed for hidden event "name", before to the flatten payload and the conflict with the reserved event_name from the SDK.

When change validated, we should change on both platforms (Apple and Android), maybe on the third one too, the web JS. I let you refine in your team. I could be involve if needed. https://github.com/SRGSSR/pillarbox-documentation/issues/26

pyby commented 1 year ago

Meeting with Julius, Mario and @StaehliJ and @waliid to clarify and update specifications:

1. PageView event
- event_name : page_view (TC predefined key)
- SDK: TCPageViewEvent
- In TC server, for MAPP Page View destination filter: event_name is page_view

2. Media events
- event_name : play, pause , stop, pos, uptime, seek, eof, segment
- SDK: TCCustomEvent
- In TC server, for MAPP Media destination filter: media_urn exists

3. Custom events
- event_name : Event name example (ScrollTo, ButtonClicked, favorite_add, …)
- SDK: TCCustomEvent
- In TC server, for MAPP Event destination filter: event_name is not page_view AND media_urn doesn't exit

For Apple SDK, here is the update:

EDIT: filter is now updated and share in the migration guide.

pyby commented 1 year ago

During a mapping review in TagCommander server V2, the content_title replaced by page_type is an issue. I fixed 2 times.

After a meeting with Julius, Mario, 2 MAPP consultants, 1 Commander Act consultant, they all spoke about page_name and page_type. Like the TC example, type and name for a page a common convention.

From documentation in confluence and SRF News iOS app code, and regarding mapping:

SRG current property SDK TCPageView property TC new property mandatory
content_title pageName page_name true
content_page_type pageType page_ype false

For sure, type ≠ title/name We could add the page_type in the public API as pageType (String). Or ignore and set pageType to null.

Example from SRF News (UDP):

Example from SRF News (Webtrekk):

pyby commented 1 year ago

Update after a meeting with Julius and Mario:

Page views

We have looked at TCPageViewEvent and we think it should match our needs for page view measurements, as follows:

  • The page titles are sent under the content_title key with v4 and are technical non-localized page identifiers (e.g. home or player). We therefore think we should send them in page_type with v5. Is it a good idea?
  • We are currently not sending any pretty page title, thus we think we should leave page_name empty. Correct?
  • We are currently sending other values under other keys and we will send them to v5 in the same way. Some of them are currently processed server-side (e.g. navigation levels which are concatenated together to produce a new identifier) and we assume these rules will be migrated to the new platform as is. Correct?

Answers for ADI DG team

pyby commented 1 year ago

Page view title is set to page_name property.

ADI team asked to set page_type as mandatory. Should we add it in the global send page view method?

pyby commented 1 year ago

I tried to switch to the official SPM repository with version 5.2.1. https://github.com/CommandersAct/iOSV5/releases/tag/5.2.1

TCCore is linked to <CoreBluetooth/CoreBluetooth.h>:

grep -r CoreBluetooth/CoreBluetooth.h TCCore.xcframework

[…]

So applications needs to add NSBluetoothPeripheralUsageDescription and NSBluetoothAlwaysUsageDescription to the Info.plist to avoid "ITMS-90683: Missing purpose string in Info.plist".

[…]

UPDATE: TagCommander team is working on an improvement for TCServerSide-noIDFA and SPM.

iOSV5 Commanders Act version for SPM makes dependency to TCore with a different version value.

So not need anymore to add Bluetooth descriptions in info plist.