[Diagnostics] Add play_store_version and play_services_version to all google events by tonidero · Pull Request #2269 · RevenueCat/purchases-android

tonidero · 2025-03-18T16:16:34Z

Description

This adds play_store_version and play_services_version to all google events.

tonidero · 2025-03-18T16:17:45Z

Originally, I was planning to also add the billing_client_version as was in the doc, but I don't think we can get the version at runtime that I could figure out... We could hardcode it somewhere, but then, it basically means we already have the billing client version from the sdk version and we can obtain this info in the backend/DW.

Just mentioning that we could quite easily create a BuildConfig field set to libs.versions.billing.get(). That doesn't require any more hardcoding than we currently do (i.e. specifying the version in libs.versions.toml). But you're right that we can also infer this info from the SDK version.

Hmm that's a good point... But still seems like duplicated info... If we could figure out if the version didn't match at runtime (like for example if there was a bump because a dev was using a newer version of the billing client separately). But I think this way we can just have this information in the DW and even join if we need a view with this information.

Yea you're right. Btw the billing library leaves a billing.properties file in the final APK with the version that's being used. We might be able to read that? 👀

Something we could look into.

emerge-tools · 2025-03-18T16:27:22Z

📸 Snapshot Test

14 modified, 378 unchanged

Name	Added	Removed	Modified	Renamed	Unchanged	Errored	Approval
TestPurchasesUIAndroidCompatibility com.revenuecat.testpurchasesuiandroidcompatibility	0	0	14	0	250	0	⏳ Needs approval
TestPurchasesUIAndroidCompatibility Paparazzi com.revenuecat.testpurchasesuiandroidcompatibility.paparazzi	0	0	0	0	128	0	N/A

🛸 Powered by Emerge Tools

codecov · 2025-03-18T18:01:21Z

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 80.31%. Comparing base (fb33fa4) to head (53fa52f).
Report is 1 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2269      +/-   ##
==========================================
+ Coverage   80.30%   80.31%   +0.01%     
==========================================
  Files         278      278              
  Lines        9483     9489       +6     
  Branches     1336     1337       +1     
==========================================
+ Hits         7615     7621       +6     
  Misses       1308     1308              
  Partials      560      560

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:

❄ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

ajpallares

Looks good! Just a small nit that you can totally ignore 😄

JayShortway

This is gonna be very valuable! Just the concern on VisibleForTesting.

JayShortway · 2025-03-19T09:40:22Z

Is VisibleForTesting needed? I don't see trackEvent being used in tests. I would prefer to test a class' observable behavior, instead of opening up private methods for tests.

Unfortunately, it is currently used in some tests... For example:

purchases-android/purchases/src/test/java/com/revenuecat/purchases/common/diagnostics/DiagnosticsTrackerFunctionalTest.kt

Line 70 in fb33fa4

diagnosticsTracker.trackEvent(event)

.

We could totally refactor this to avoid making this internal, but didn't want to add further refactors in an unrelated PR. I just added this annotation because I noticed this was only used externally in tests and seemed easy to confuse with the other new trackEvent method.

Can do this change in a follow-up PR though.

Oh right, I see. Maybe the other trackEvent method, taking a DiagnosticsEntryName, has more "right" to be internal than this one, as this one requires the caller to know the appSessionID.

Not something we need to address in this PR indeed.

… events

JayShortway · 2025-03-19T09:58:19Z

Oh right, I see. Maybe the other trackEvent method, taking a DiagnosticsEntryName, has more "right" to be internal than this one, as this one requires the caller to know the appSessionID.

Not something we need to address in this PR indeed.

JayShortway · 2025-03-19T10:12:32Z

Yea you're right. Btw the billing library leaves a billing.properties file in the final APK with the version that's being used. We might be able to read that? 👀

Something we could look into.

@joshdholtz

**This is an automatic release.** ## RevenueCatUI SDK ### 🐞 Bugfixes * Fix landscape mode in Paywalls v1 Template 3 (#2265) via Josh Holtz (@joshdholtz) ### Customer Center #### ✨ New Features * feat: Introduce `CustomerCenterView` for hybrid usage (#2170) via Cesar de la Vega (@vegaro) * Add `onManagementOptionSelected` to `CustomerCenterListener` (#2270) via Cesar de la Vega (@vegaro) ### Paywallv2 #### 🐞 Bugfixes * [Paywalls V2] Ignores missing font alias if it's blank. (#2271) via JayShortway (@JayShortway) * [Paywalls V2] Fixes badges not being overriden (#2266) via JayShortway (@JayShortway) ### 🔄 Other Changes * [Diagnostics] Add play_store_version and play_services_version to all google events (#2269) via Toni Rico (@tonidero) * [Diagnostics] Add `id` and `app_session_id` to events (#2268) via Toni Rico (@tonidero) * Uploads Paparazzi screenshots to Emerge. (#2232) via JayShortway (@JayShortway) Co-authored-by: revenuecat-ops <ops@revenuecat.com>

tonidero added the pr:other label Mar 18, 2025

tonidero commented Mar 18, 2025

View reviewed changes

tonidero marked this pull request as ready for review March 18, 2025 16:18

tonidero requested a review from a team March 18, 2025 16:18

ajpallares approved these changes Mar 19, 2025

View reviewed changes

Comment thread purchases/src/main/kotlin/com/revenuecat/purchases/common/diagnostics/DiagnosticsTracker.kt Outdated

JayShortway reviewed Mar 19, 2025

View reviewed changes

Base automatically changed from diagnostics-add-event-id-app-session-id to main March 19, 2025 09:43

tonidero added 2 commits March 19, 2025 10:48

[Diagnostics] Add play_store_version and play_services_version to all…

d08a4f8

… events

fix tests

3682cd4

tonidero force-pushed the diagnostics-add-play-store-services-versions branch from 7044c5d to 3682cd4 Compare March 19, 2025 09:48

Cleanup

53fa52f

JayShortway approved these changes Mar 19, 2025

View reviewed changes

tonidero merged commit b4e564c into main Mar 19, 2025

tonidero deleted the diagnostics-add-play-store-services-versions branch March 19, 2025 10:31

This was referenced Mar 19, 2025

[AUTOMATIC] Release/8.15.0 #2272

Closed

[AUTOMATIC] Release/8.15.0 #2273

Merged

Uh oh!

Conversation

tonidero commented Mar 18, 2025

Description

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

emerge-tools Bot commented Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

📸 Snapshot Test

14 modified, 378 unchanged

Uh oh!

codecov Bot commented Mar 18, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

ajpallares left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

JayShortway left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

emerge-tools Bot commented Mar 18, 2025 •

edited

Loading

codecov Bot commented Mar 18, 2025 •

edited

Loading