Automattic / Automattic-Tracks-iOS

Client library for tracking user events for later analysis
GNU General Public License v2.0
40 stars 12 forks source link

Update `releaseName` default value #267

Closed spencertransier closed 8 months ago

spencertransier commented 8 months ago

This PR updates the default releaseName attribute from sending only the CFBundleVersionKey to now include the CFBundleIdentifierKey, CFBundleShortVersionString, and CFBundleVersionKey. Sentry releases are global across all projects in an organization, so only relying on the CFBundleVersionKey was causing conflicts between apps that had the same release number. This will now correctly separate the releases.

The releaseName format that we're sending now is the default format that Sentry uses for the releaseName. I decided that it would be good to explicitly define that format in this library so that we can ensure future consistency for release names across all the apps.

I had trouble using the TracksDemo app to test updating the release version in Sentry, so I created a test branch for WooCommerce iOS that points to this dev branch: test/sentry-release-name (The only change there is updating the podfile to point here)

Using that test branch, I was able to verify that the releases are now being sent up to Sentry with the updated attributes. You can see that the 16.2 (16.2.0.0) and 16.1 (16.1.0.1) releases show the full version information now:

Screenshot 2023-11-13 at 4 43 21 PM
mokagio commented 8 months ago

@spencertransier I saw CI failed because the change in the source code now requires changes to the tests too, which makes me wonder whether it might require changes in the clients as well.

Depending on the amount of work required, and given that the release tracking inconsistency is a blocker for a project in WooCommerce (internal ref pdnsEh-1mT-p2) feel free to ship a version with your override since you already proven it works. We can iterate on this with more clam later. 👍

spencertransier commented 8 months ago

@mokagio When I was chatting with @jkmassel about this the other day, he suggested making releaseName an optional string so that clients could pass in their own release name if needed. Looking through our iOS apps, though, DOiOS/Mac is the only one not relying on the default behavior that's current the Tracks library. And even DO is just passing the same format as the Sentry default so that the DO releases would be configured correctly (what WCiOS is trying to accomplish). Seeing that all the apps want this format anyway, I don't see a need to provide a way to customize the releaseName in this library.

Considering that, what do you think about the changes I just pushed to remove the releaseName from the protocol entirely and only rely on Sentry's default? It would be a breaking change and we would need to bump the library to 3.0.0. DOiOS/Mac is the only A8C app that would actually need to be updated (and it would just be removing their custom releaseName at that). Then the other A8C apps that update to 3.0.0 would get the improved releaseName's for free without needing to making any changes other than updating the library.

I tried out the latest PR change in the WCiOS test branch and it worked great. The new version showed up in Sentry with the expected format.