Skip to content

Commit

Permalink
Avoid checking for reserved keys in internal signals being sent
Browse files Browse the repository at this point in the history
  • Loading branch information
Jeehut committed Nov 8, 2024
1 parent 06bd6c3 commit 57a4ec0
Show file tree
Hide file tree
Showing 6 changed files with 74 additions and 51 deletions.
2 changes: 1 addition & 1 deletion Sources/TelemetryDeck/Presets/TelemetryDeck+Errors.swift
Original file line number Diff line number Diff line change
Expand Up @@ -28,7 +28,7 @@ public extension TelemetryDeck {
errorParameters["TelemetryDeck.Error.message"] = message
}

self.signal(
self.internalSignal(
"TelemetryDeck.Error.occurred",
parameters: errorParameters.merging(parameters) { $1 },
floatValue: floatValue,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ extension TelemetryDeck {
public static func navigationPathChanged(from source: String, to destination: String, customUserID: String? = nil) {
NavigationStatus.shared.previousNavigationPath = destination

self.signal(
self.internalSignal(
"TelemetryDeck.Navigation.pathChanged",
parameters: [
"TelemetryDeck.Navigation.schemaVersion": "1",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,7 @@ extension TelemetryDeck {
}
}

self.signal(
self.internalSignal(
"TelemetryDeck.Purchase.completed",
parameters: purchaseParameters.merging(parameters) { $1 },
floatValue: priceValueInUSD,
Expand Down
43 changes: 0 additions & 43 deletions Sources/TelemetryDeck/Signals/SignalManager.swift
Original file line number Diff line number Diff line change
Expand Up @@ -18,22 +18,6 @@ protocol SignalManageable {
final class SignalManager: SignalManageable, @unchecked Sendable {
static let minimumSecondsToPassBetweenRequests: Double = 10

static let reservedKeysLowercased: Set<String> = Set(
[
"type", "clientUser", "appID", "sessionID", "floatValue",
"newSessionBegan", "platform", "systemVersion", "majorSystemVersion", "majorMinorSystemVersion", "appVersion", "buildNumber",
"isSimulator", "isDebug", "isTestFlight", "isAppStore", "modelName", "architecture", "operatingSystem", "targetEnvironment",
"locale", "region", "appLanguage", "preferredLanguage", "telemetryClientVersion",
].map { $0.lowercased() }
)

static let internalSignalNames: Set<String> = [
"TelemetryDeck.Error.occurred",
"TelemetryDeck.Navigation.pathChanged",
"TelemetryDeck.Purchase.completed",
"TelemetryDeck.Session.started",
]

private var signalCache: SignalCache<SignalPostBody>
let configuration: TelemetryManagerConfiguration

Expand Down Expand Up @@ -91,33 +75,6 @@ final class SignalManager: SignalManageable, @unchecked Sendable {
customUserID: String?,
configuration: TelemetryManagerConfiguration
) {
// warn users about reserved keys to avoid unexpected behavior
if signalName.lowercased().hasPrefix("telemetrydeck."), !Self.internalSignalNames.contains(signalName) {
configuration.logHandler?.log(
.error,
message: "Sending signal with reserved prefix 'TelemetryDeck.' will cause unexpected behavior. Please use another prefix instead."
)
} else if Self.reservedKeysLowercased.contains(signalName.lowercased()) {
configuration.logHandler?.log(
.error,
message: "Sending signal with reserved name '\(signalName)' will cause unexpected behavior. Please use another name instead."
)
}

for parameterKey in parameters.keys {
if parameterKey.lowercased().hasPrefix("telemetrydeck.") {
configuration.logHandler?.log(
.error,
message: "Sending parameter with reserved key prefix 'TelemetryDeck.' will cause unexpected behavior. Please use another prefix instead."
)
} else if Self.reservedKeysLowercased.contains(parameterKey.lowercased()) {
configuration.logHandler?.log(
.error,
message: "Sending parameter with reserved key '\(parameterKey)' will cause unexpected behavior. Please use another key instead."
)
}
}

// enqueue signal to sending cache
DispatchQueue.main.async {
let defaultUserIdentifier = self.defaultUserIdentifier
Expand Down
2 changes: 1 addition & 1 deletion Sources/TelemetryDeck/TelemetryClient.swift
Original file line number Diff line number Diff line change
Expand Up @@ -70,7 +70,7 @@ public struct TelemetryManagerConfiguration: Sendable {
public var sessionID = UUID() {
didSet {
if sendNewSessionBeganSignal {
TelemetryDeck.signal("TelemetryDeck.Session.started")
TelemetryDeck.internalSignal("TelemetryDeck.Session.started")
}
}
}
Expand Down
74 changes: 70 additions & 4 deletions Sources/TelemetryDeck/TelemetryDeck.swift
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,15 @@ public enum TelemetryDeck {
/// This alias makes it easier to migrate the configuration type into the TelemetryDeck namespace in future versions when deprecated code is fully removed.
public typealias Config = TelemetryManagerConfiguration

static let reservedKeysLowercased: Set<String> = Set(
[
"type", "clientUser", "appID", "sessionID", "floatValue",
"newSessionBegan", "platform", "systemVersion", "majorSystemVersion", "majorMinorSystemVersion", "appVersion", "buildNumber",
"isSimulator", "isDebug", "isTestFlight", "isAppStore", "modelName", "architecture", "operatingSystem", "targetEnvironment",
"locale", "region", "appLanguage", "preferredLanguage", "telemetryClientVersion",
].map { $0.lowercased() }
)

/// Initializes TelemetryDeck with a customizable configuration.
///
/// - Parameter configuration: An instance of `Configuration` which includes all the settings required to configure TelemetryDeck.
Expand Down Expand Up @@ -37,12 +46,69 @@ public enum TelemetryDeck {
guard !configuration.swiftUIPreviewMode, !configuration.analyticsDisabled else { return }

let combinedSignalName = (configuration.defaultSignalPrefix ?? "") + signalName
let combinedParameters = configuration.defaultParameters()
.merging(parameters) { $1 }
.mapKeys { (configuration.defaultParameterPrefix ?? "") + $0 }
let prefixedParameters = parameters.mapKeys { (configuration.defaultParameterPrefix ?? "") + $0 }

// warn users about reserved keys to avoid unexpected behavior
if combinedSignalName.lowercased().hasPrefix("telemetrydeck.") {
configuration.logHandler?.log(
.error,
message: "Sending signal with reserved prefix 'TelemetryDeck.' will cause unexpected behavior. Please use another prefix instead."
)
} else if Self.reservedKeysLowercased.contains(combinedSignalName.lowercased()) {
configuration.logHandler?.log(
.error,
message: "Sending signal with reserved name '\(combinedSignalName)' will cause unexpected behavior. Please use another name instead."
)
}

// only check parameters (not default ones)
for parameterKey in prefixedParameters.keys {
if parameterKey.lowercased().hasPrefix("telemetrydeck.") {
configuration.logHandler?.log(
.error,
message: "Sending parameter with reserved key prefix 'TelemetryDeck.' will cause unexpected behavior. Please use another prefix instead."
)
} else if Self.reservedKeysLowercased.contains(parameterKey.lowercased()) {
configuration.logHandler?.log(
.error,
message: "Sending parameter with reserved key '\(parameterKey)' will cause unexpected behavior. Please use another key instead."
)
}
}

self.internalSignal(combinedSignalName, parameters: prefixedParameters, floatValue: floatValue, customUserID: customUserID)
}

/// A signal being sent without enriching the signal name with a prefix. Also, any reserved signal name check are skipped. Only for internal use.
static func internalSignal(
_ signalName: String,
parameters: [String: String] = [:],
floatValue: Double? = nil,
customUserID: String? = nil
) {
let manager = TelemetryManager.shared
let configuration = manager.configuration

let prefixedDefaultParameters = configuration.defaultParameters().mapKeys { (configuration.defaultParameterPrefix ?? "") + $0 }
let combinedParameters = prefixedDefaultParameters.merging(parameters) { $1 }

// check only default parameters
for parameterKey in prefixedDefaultParameters.keys {
if parameterKey.lowercased().hasPrefix("telemetrydeck.") {
configuration.logHandler?.log(
.error,
message: "Sending parameter with reserved key prefix 'TelemetryDeck.' will cause unexpected behavior. Please use another prefix instead."
)
} else if Self.reservedKeysLowercased.contains(parameterKey.lowercased()) {
configuration.logHandler?.log(
.error,
message: "Sending parameter with reserved key '\(parameterKey)' will cause unexpected behavior. Please use another key instead."
)
}
}

manager.signalManager.processSignal(
combinedSignalName,
signalName,
parameters: combinedParameters,
floatValue: floatValue,
customUserID: customUserID,
Expand Down

0 comments on commit 57a4ec0

Please sign in to comment.