Skip to content

Bug 1271380 - Report search counts in core ping#1843

Merged
thebnich merged 2 commits intomozilla-mobile:masterfrom
thebnich:search-ping
May 26, 2016
Merged

Bug 1271380 - Report search counts in core ping#1843
thebnich merged 2 commits intomozilla-mobile:masterfrom
thebnich:search-ping

Conversation

@thebnich
Copy link
Copy Markdown
Contributor

The first commit creates/send the core ping only if we have an active network connection. The second commit adds search recording to our SearchViewController and reports this data with the core ping.

When creating the core ping, we immediately reset the search data that we're reporting. That means if we create the ping but the ping isn't successful, we'll drop the data. That's why the network connectivity check was added to the PR: it increases the likelihood that the ping will succeed.

Unless we want to take the time to implement a telemetry fault-handling layer that stores and resends unique pings, I think this is the best we can do. The other option would be to reset the count only after receiving an HTTP 200, where we'd instead be sending potentially duplicate pings instead of dropping them. I agree with mcomella's comment that missing data is better than bad data.

@thebnich thebnich force-pushed the search-ping branch 3 times, most recently from fba4515 to 8877c0e Compare May 22, 2016 18:54
Comment thread Client/Application/AppDelegate.swift Outdated
Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd explain why you don't try to send when you don't have connectivity – i.e. because it resets your measurements & increments seq, e.g. like your comment on this PR.

@thebnich thebnich force-pushed the search-ping branch 2 times, most recently from ccd1cd8 to 7be34c1 Compare May 24, 2016 22:55
@st3fan
Copy link
Copy Markdown
Contributor

st3fan commented May 26, 2016

I think this is a great start. We can do buffering of pings later.

@thebnich thebnich merged commit 12e9005 into mozilla-mobile:master May 26, 2016
@thebnich thebnich deleted the search-ping branch May 26, 2016 19:09
isabelrios pushed a commit to isabelrios/firefox-ios that referenced this pull request Feb 19, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants