Skip to content
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

Pixel retrying #3358

Merged
merged 47 commits into from
Oct 18, 2024
Merged
Show file tree
Hide file tree
Changes from 45 commits
Commits
Show all changes
47 commits
Select commit Hold shift + click to select a range
11b42cb
Add pixel persistence.
samsymons Sep 11, 2024
a7ebe56
Add the PersistentPixel implementation and tests.
samsymons Sep 12, 2024
13c2837
Update tests.
samsymons Sep 12, 2024
6f3bc75
Merge branch 'main' into sam/persistent-pixels
samsymons Sep 12, 2024
ad1847d
Add timestamp and retry parameters.
samsymons Sep 13, 2024
9785e68
Undo unused changes.
samsymons Sep 13, 2024
8be2119
Fix indentation.
samsymons Sep 13, 2024
ff4ca96
More cleanup.
samsymons Sep 13, 2024
0b4c412
Test the pixel limit.
samsymons Sep 13, 2024
ff84e47
Handle incoming pixels when the queue is being processed.
samsymons Sep 13, 2024
33b2850
Tidy up tests.
samsymons Sep 13, 2024
d2db6aa
Clear out pending pixels when queue processing is complete.
samsymons Sep 13, 2024
7808ed3
Add dispatchPrecondition.
samsymons Sep 13, 2024
e201976
Add PersistentPixel logs.
samsymons Sep 13, 2024
7b0f864
Migrate VPN controller pixels to PersistentPixel.
samsymons Sep 13, 2024
b2a5a55
Update PersistentPixel tests.
samsymons Sep 13, 2024
63ddd33
Fix broken test.
samsymons Sep 13, 2024
cff48e2
Test the timestamp check.
samsymons Sep 13, 2024
847f502
Update PersistentPixel.
samsymons Sep 13, 2024
3a890de
Surface persistence errors as their own types.
samsymons Sep 13, 2024
e61f9bb
Relocate mock.
samsymons Sep 13, 2024
175ae48
Add PersistentPixel to the PacketTunnelProvider.
samsymons Sep 13, 2024
9866e5a
Merge branch 'main' into sam/persistent-pixels
samsymons Sep 16, 2024
874b312
Filter out pixels older than 28 days.
samsymons Sep 16, 2024
d27681b
Add additional logs to help debugging.
samsymons Sep 16, 2024
c772efa
Merge branch 'main' into sam/persistent-pixels
samsymons Sep 28, 2024
aacc4dc
Merge branch 'main' into sam/persistent-pixels
samsymons Oct 9, 2024
f4e380f
Merge branch 'main' into sam/persistent-pixels
samsymons Oct 10, 2024
ad82419
Update pixels to be removed by UUID, and remove `failedPixelsPendingS…
samsymons Oct 11, 2024
725520a
Replace lock and boolean flag with a single queue.
samsymons Oct 11, 2024
e2a8771
Merge branch 'main' into sam/persistent-pixels
samsymons Oct 13, 2024
bd24899
Resolve build failures after merge conflicts.
samsymons Oct 13, 2024
d18829c
Fix up main test failures. Still a flaky test failure to fix.
samsymons Oct 13, 2024
d936a99
Wait for `sendQueuedPixels` to complete its work before proceeding.
samsymons Oct 13, 2024
b6f4e96
Fix SwiftLint.
samsymons Oct 13, 2024
154506c
Update logger usage.
samsymons Oct 13, 2024
e3c8665
Merge branch 'main' into sam/persistent-pixels
samsymons Oct 14, 2024
5911875
Merge branch 'main' into sam/persistent-pixels
samsymons Oct 16, 2024
62bb409
Only compute `date28DaysAgo` once.
samsymons Oct 16, 2024
5cae23c
Add a test for the DailyPixel `alreadyFired` case.
samsymons Oct 16, 2024
756f62e
Change PersistentPixel errors array to tuple.
samsymons Oct 16, 2024
7c775ef
Merge branch 'main' into sam/persistent-pixels
samsymons Oct 17, 2024
2b87ddc
Update DailyPixel to accept suffixes as a parameter.
samsymons Oct 18, 2024
26058a3
Remove unused stub entry.
samsymons Oct 18, 2024
d28a6ea
Cache pixel metadata for read/write calls.
samsymons Oct 18, 2024
a7323d2
Merge branch 'main' into sam/persistent-pixels
samsymons Oct 18, 2024
7a52ab0
Remove unused variable.
samsymons Oct 18, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
9 changes: 6 additions & 3 deletions Core/DailyPixel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,11 @@ public final class DailyPixel {

}

private enum Constant {
public enum Constant {

static let dailyPixelStorageIdentifier = "com.duckduckgo.daily.pixel.storage"
public static let dailyPixelSuffixes = (dailySuffix: "_daily", countSuffix: "_count")
public static let legacyDailyPixelSuffixes = (dailySuffix: "_d", countSuffix: "_c")

}

Expand Down Expand Up @@ -80,6 +82,7 @@ public final class DailyPixel {
/// This means a pixel will get sent twice the first time it is called per-day, and subsequent calls that day will only send the `_c` variant.
/// This is useful in situations where pixels receive spikes in volume, as the daily pixel can be used to determine how many users are actually affected.
public static func fireDailyAndCount(pixel: Pixel.Event,
pixelNameSuffixes: (dailySuffix: String, countSuffix: String) = Constant.dailyPixelSuffixes,
error: Swift.Error? = nil,
withAdditionalParameters params: [String: String] = [:],
includedParameters: [Pixel.QueryParameters] = [.appVersion],
Expand All @@ -91,7 +94,7 @@ public final class DailyPixel {

if !hasBeenFiredToday(forKey: key, dailyPixelStore: dailyPixelStore) {
pixelFiring.fire(
pixelNamed: pixel.name + "_d",
pixelNamed: pixel.name + pixelNameSuffixes.dailySuffix,
withAdditionalParameters: params,
includedParameters: includedParameters,
onComplete: onDailyComplete
Expand All @@ -105,7 +108,7 @@ public final class DailyPixel {
newParams.appendErrorPixelParams(error: error)
}
pixelFiring.fire(
pixelNamed: pixel.name + "_c",
pixelNamed: pixel.name + pixelNameSuffixes.countSuffix,
withAdditionalParameters: newParams,
includedParameters: includedParameters,
onComplete: onCountComplete
Expand Down
13 changes: 12 additions & 1 deletion Core/DailyPixelFiring.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,11 +18,22 @@
//

import Foundation
import Persistence

public protocol DailyPixelFiring {
static func fireDaily(_ pixel: Pixel.Event,
withAdditionalParameters params: [String: String])


static func fireDailyAndCount(pixel: Pixel.Event,
pixelNameSuffixes: (dailySuffix: String, countSuffix: String),
error: Swift.Error?,
withAdditionalParameters params: [String: String],
includedParameters: [Pixel.QueryParameters],
pixelFiring: PixelFiring.Type,
dailyPixelStore: KeyValueStoring,
onDailyComplete: @escaping (Swift.Error?) -> Void,
onCountComplete: @escaping (Swift.Error?) -> Void)

static func fireDaily(_ pixel: Pixel.Event)
}

Expand Down
292 changes: 292 additions & 0 deletions Core/PersistentPixel.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,292 @@
//
// PersistentPixel.swift
// DuckDuckGo
//
// Copyright © 2024 DuckDuckGo. All rights reserved.
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
//

import Foundation
import os.log
import Networking
import Persistence

public protocol PersistentPixelFiring {
func fire(pixel: Pixel.Event,
error: Swift.Error?,
includedParameters: [Pixel.QueryParameters],
withAdditionalParameters params: [String: String],
onComplete: @escaping (Error?) -> Void)

func fireDailyAndCount(pixel: Pixel.Event,
pixelNameSuffixes: (dailySuffix: String, countSuffix: String),
error: Swift.Error?,
withAdditionalParameters params: [String: String],
includedParameters: [Pixel.QueryParameters],
completion: @escaping ((dailyPixelStorageError: Error?, countPixelStorageError: Error?)) -> Void)

func sendQueuedPixels(completion: @escaping (PersistentPixelStorageError?) -> Void)
}

public final class PersistentPixel: PersistentPixelFiring {

enum Constants {
static let lastProcessingDateKey = "com.duckduckgo.ios.persistent-pixel.last-processing-timestamp"

#if DEBUG
static let minimumProcessingInterval: TimeInterval = .minutes(1)
#else
static let minimumProcessingInterval: TimeInterval = .hours(1)
#endif
}

private let pixelFiring: PixelFiring.Type
private let dailyPixelFiring: DailyPixelFiring.Type
private let persistentPixelStorage: PersistentPixelStoring
private let lastProcessingDateStorage: KeyValueStoring
private let calendar: Calendar
private let dateGenerator: () -> Date
private let workQueue = DispatchQueue(label: "Persistent Pixel Retry Queue")

private let dateFormatter: ISO8601DateFormatter = {
let formatter = ISO8601DateFormatter()
formatter.formatOptions = [.withInternetDateTime]
return formatter
}()

public convenience init() {
self.init(pixelFiring: Pixel.self,
dailyPixelFiring: DailyPixel.self,
persistentPixelStorage: DefaultPersistentPixelStorage(),
lastProcessingDateStorage: UserDefaults.standard)
}

init(pixelFiring: PixelFiring.Type,
dailyPixelFiring: DailyPixelFiring.Type,
persistentPixelStorage: PersistentPixelStoring,
lastProcessingDateStorage: KeyValueStoring,
calendar: Calendar = .current,
dateGenerator: @escaping () -> Date = { Date() }) {
self.pixelFiring = pixelFiring
self.dailyPixelFiring = dailyPixelFiring
self.persistentPixelStorage = persistentPixelStorage
self.lastProcessingDateStorage = lastProcessingDateStorage
self.calendar = calendar
self.dateGenerator = dateGenerator
}

// MARK: - Pixel Firing

public func fire(pixel: Pixel.Event,
error: Swift.Error? = nil,
includedParameters: [Pixel.QueryParameters] = [.appVersion],
withAdditionalParameters additionalParameters: [String: String] = [:],
onComplete: @escaping (Error?) -> Void = { _ in }) {
let fireDate = dateGenerator()
let dateString = dateFormatter.string(from: fireDate)
var additionalParameters = additionalParameters
additionalParameters[PixelParameters.originalPixelTimestamp] = dateString

Logger.general.debug("Firing persistent pixel named \(pixel.name)")

pixelFiring.fire(pixel: pixel,
error: error,
includedParameters: includedParameters,
withAdditionalParameters: additionalParameters) { pixelFireError in
if pixelFireError != nil {
do {
if let error {
additionalParameters.appendErrorPixelParams(error: error)
}

try self.persistentPixelStorage.append(pixels: [
PersistentPixelMetadata(eventName: pixel.name,
additionalParameters: additionalParameters,
includedParameters: includedParameters)
])

onComplete(nil)
} catch {
onComplete(error)
}
}
}
}

public func fireDailyAndCount(pixel: Pixel.Event,
pixelNameSuffixes: (dailySuffix: String, countSuffix: String) = DailyPixel.Constant.dailyPixelSuffixes,
error: Swift.Error? = nil,
withAdditionalParameters additionalParameters: [String: String],
includedParameters: [Pixel.QueryParameters] = [.appVersion],
completion: @escaping ((dailyPixelStorageError: Error?, countPixelStorageError: Error?)) -> Void = { _ in }) {
let dispatchGroup = DispatchGroup()

dispatchGroup.enter() // onDailyComplete
dispatchGroup.enter() // onCountComplete

var dailyPixelStorageError: Error?
var countPixelStorageError: Error?

let fireDate = dateGenerator()
let dateString = dateFormatter.string(from: fireDate)
var additionalParameters = additionalParameters
additionalParameters[PixelParameters.originalPixelTimestamp] = dateString

Logger.general.debug("Firing persistent daily/count pixel named \(pixel.name)")

dailyPixelFiring.fireDailyAndCount(
pixel: pixel,
pixelNameSuffixes: pixelNameSuffixes,
error: error,
withAdditionalParameters: additionalParameters,
includedParameters: includedParameters,
pixelFiring: Pixel.self,
dailyPixelStore: DailyPixel.storage,
onDailyComplete: { dailyError in
if let dailyError, (dailyError as? DailyPixel.Error) != .alreadyFired {
do {
if let error { additionalParameters.appendErrorPixelParams(error: error) }
Logger.general.debug("Saving persistent daily pixel named \(pixel.name)")
try self.persistentPixelStorage.append(pixels: [
PersistentPixelMetadata(eventName: pixel.name + pixelNameSuffixes.dailySuffix,
additionalParameters: additionalParameters,
includedParameters: includedParameters)
])
} catch {
dailyPixelStorageError = error
}
}

dispatchGroup.leave()
}, onCountComplete: { countError in
if countError != nil {
do {
if let error { additionalParameters.appendErrorPixelParams(error: error) }
Logger.general.debug("Saving persistent count pixel named \(pixel.name)")
try self.persistentPixelStorage.append(pixels: [
PersistentPixelMetadata(eventName: pixel.name + pixelNameSuffixes.countSuffix,
additionalParameters: additionalParameters,
includedParameters: includedParameters)
])
bwaresiak marked this conversation as resolved.
Show resolved Hide resolved
} catch {
countPixelStorageError = error
}
}

dispatchGroup.leave()
}
)

dispatchGroup.notify(queue: .global()) {
let errors = [dailyPixelStorageError, countPixelStorageError].compactMap { $0 }

Check warning on line 192 in Core/PersistentPixel.swift

View workflow job for this annotation

GitHub Actions / Make Release Build

initialization of immutable value 'errors' was never used; consider replacing with assignment to '_' or removing it

Check warning on line 192 in Core/PersistentPixel.swift

View workflow job for this annotation

GitHub Actions / Make Release Build

initialization of immutable value 'errors' was never used; consider replacing with assignment to '_' or removing it

Check warning on line 192 in Core/PersistentPixel.swift

View workflow job for this annotation

GitHub Actions / Unit Tests

initialization of immutable value 'errors' was never used; consider replacing with assignment to '_' or removing it
samsymons marked this conversation as resolved.
Show resolved Hide resolved
completion((dailyPixelStorageError: dailyPixelStorageError, countPixelStorageError: countPixelStorageError))
}
}

// MARK: - Queue Processing

public func sendQueuedPixels(completion: @escaping (PersistentPixelStorageError?) -> Void) {
workQueue.async {
if let lastProcessingDate = self.lastProcessingDateStorage.object(forKey: Constants.lastProcessingDateKey) as? Date {
let threshold = self.dateGenerator().addingTimeInterval(-Constants.minimumProcessingInterval)
if threshold <= lastProcessingDate {
completion(nil)
return
}
}

self.lastProcessingDateStorage.set(self.dateGenerator(), forKey: Constants.lastProcessingDateKey)

do {
let queuedPixels = try self.persistentPixelStorage.storedPixels()

if queuedPixels.isEmpty {
completion(nil)
return
}

Logger.general.debug("Persistent pixel retrying \(queuedPixels.count, privacy: .public) pixels")

self.fire(queuedPixels: queuedPixels) { pixelIDsToRemove in
Logger.general.debug("Persistent pixel retrying done, \(pixelIDsToRemove.count, privacy: .public) pixels successfully sent")

do {
try self.persistentPixelStorage.remove(pixelsWithIDs: pixelIDsToRemove)
completion(nil)
} catch {
completion(PersistentPixelStorageError.writeError(error))
}
}
} catch {
completion(PersistentPixelStorageError.readError(error))
}
}
}

// MARK: - Private

/// Sends queued pixels and calls the completion handler with those that should be removed.
private func fire(queuedPixels: [PersistentPixelMetadata], completion: @escaping (Set<UUID>) -> Void) {
let dispatchGroup = DispatchGroup()

let pixelIDsAccessQueue = DispatchQueue(label: "Failed Pixel Retry Attempt Metadata Queue")
var pixelIDsToRemove: Set<UUID> = []
let currentDate = dateGenerator()
let date28DaysAgo = calendar.date(byAdding: .day, value: -28, to: currentDate)

for pixelMetadata in queuedPixels {
if let sendDateString = pixelMetadata.timestamp, let sendDate = dateFormatter.date(from: sendDateString), let date28DaysAgo {
if sendDate < date28DaysAgo {
pixelIDsAccessQueue.sync {
_ = pixelIDsToRemove.insert(pixelMetadata.id)
}
samsymons marked this conversation as resolved.
Show resolved Hide resolved
continue
}
} else {
// If we don't have a timestamp for some reason, ignore the retry - retries are only useful if they have a timestamp attached.
// It's not expected that this will ever happen, so an assertion failure is used to report it when debugging.
assertionFailure("Did not find a timestamp for pixel \(pixelMetadata.eventName)")
pixelIDsAccessQueue.sync {
_ = pixelIDsToRemove.insert(pixelMetadata.id)
}
continue
}

var pixelParameters = pixelMetadata.additionalParameters
pixelParameters[PixelParameters.retriedPixel] = "1"

dispatchGroup.enter()

pixelFiring.fire(
pixelNamed: pixelMetadata.eventName,
withAdditionalParameters: pixelParameters,
includedParameters: pixelMetadata.includedParameters,
onComplete: { error in
if error == nil {
pixelIDsAccessQueue.sync {
_ = pixelIDsToRemove.insert(pixelMetadata.id)
}
}

dispatchGroup.leave()
}
)
}

dispatchGroup.notify(queue: .global()) {
completion(pixelIDsToRemove)
}
}

}
Loading
Loading