-
Notifications
You must be signed in to change notification settings - Fork 3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
FXIOS-11121 #24248 [Sponsored tiles] Ensure we send to both places for telemetry #24257
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
code looks good! do we want to add unit tests for these changes?
7ce79d0
to
96d772b
Compare
Client.app: Coverage: 32.32
Generated by 🚫 Danger Swift against 6ffa440 |
a28d455
to
a21fa3c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
thanks for making the updateS @lmarceau!! I added some smaller comments, but nothing blocking!
firefox-ios/firefox-ios-tests/Tests/ClientTests/Mocks/MockGleanWrapper.swift
Outdated
Show resolved
Hide resolved
firefox-ios/firefox-ios-tests/Tests/ClientTests/SponsoredTileTelemetryTests.swift
Show resolved
Hide resolved
...ox-ios-tests/Tests/ClientTests/Frontend/Home/TopSites/UnifiedAdsCallbackTelemetryTests.swift
Show resolved
Hide resolved
9b51cc6
to
6ffa440
Compare
…r telemetry (#24257) * Ensure we send to both places for telemetry * Add tests, remove static from SponsoredTileTelemetry todo so * Fix * Remove savedEvent and replace with SavedEvents * Add event check on array * Adjust for latest main
📜 Tickets
Jira ticket
Github issue
💡 Description
We should use both endpoints to send telemetry events whenever the Unified Ads api feature flag is turned ON. By maintaining the older Glean pings, we ensure the Looker dashboard continue to work while we migrate to used the Unified Ads api. Data will also be able to be matched between the old and the new APIs, ensuring we remain on par.
In other words, with this PR when the feature flag is ON we show the tiles by using the
UnifiedAdsProvider
, but impression and click telemetry are sent with bothSponsoredTileTelemetry
(using the Glean pings system) andUnifiedAdsCallbackTelemetry
.📝 Checklist
You have to check all boxes before merging
@Mergifyio backport release/v120
)