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

Avoid checking for reserved keys in internal signals being sent #203

Merged
merged 1 commit into from
Nov 8, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
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