From cc47b9a1f2086bcb2e2035e212bc62a1ba9dc105 Mon Sep 17 00:00:00 2001 From: Michael Grosse Huelsewiesche Date: Fri, 22 Nov 2024 13:28:01 -0500 Subject: [PATCH] Simplifying flush on error logic and improving destination plugin names (#374) --- Sources/Segment/Errors.swift | 1 + Sources/Segment/Timeline.swift | 20 ++++++++++---- Sources/Segment/Utilities/Telemetry.swift | 32 +++++++---------------- Tests/Segment-Tests/Telemetry_Tests.swift | 1 + 4 files changed, 27 insertions(+), 27 deletions(-) diff --git a/Sources/Segment/Errors.swift b/Sources/Segment/Errors.swift index b0ec057..8abf95f 100644 --- a/Sources/Segment/Errors.swift +++ b/Sources/Segment/Errors.swift @@ -92,6 +92,7 @@ extension Analytics { Telemetry.shared.error(metric: Telemetry.INVOKE_ERROR_METRIC, log: Thread.callStackSymbols.joined(separator: "\n")) { (_ it: inout [String: String]) in it["error"] = "\(translatedError)" + it["caller"] = Thread.callStackSymbols[3] } } } diff --git a/Sources/Segment/Timeline.swift b/Sources/Segment/Timeline.swift index 2f8febe..3ab7a2b 100644 --- a/Sources/Segment/Timeline.swift +++ b/Sources/Segment/Timeline.swift @@ -68,7 +68,11 @@ internal class Mediator { Telemetry.shared.increment(metric: Telemetry.INTEGRATION_METRIC) { (_ it: inout [String: String]) in it["message"] = "added" - it["plugin"] = "\(plugin.type)-\(String(describing: plugin))" + if let plugin = plugin as? DestinationPlugin, !plugin.key.isEmpty { + it["plugin"] = "\(plugin.type)-\(plugin.key)" + } else { + it["plugin"] = "\(plugin.type)-\(String(describing: plugin))" + } } } @@ -77,8 +81,11 @@ internal class Mediator { Telemetry.shared.increment(metric: Telemetry.INTEGRATION_METRIC) { (_ it: inout [String: String]) in it["message"] = "removed" - it["plugin"] = "\(plugin.type)-\(String(describing: plugin))" - } + if let plugin = plugin as? DestinationPlugin, !plugin.key.isEmpty { + it["plugin"] = "\(plugin.type)-\(plugin.key)" + } else { + it["plugin"] = "\(plugin.type)-\(String(describing: plugin))" + } } return plugin === storedPlugin } } @@ -99,8 +106,11 @@ internal class Mediator { Telemetry.shared.increment(metric: Telemetry.INTEGRATION_METRIC) { (_ it: inout [String: String]) in it["message"] = "event-\(r.type ?? "unknown")" - it["plugin"] = "\(plugin.type)-\(String(describing: plugin))" - } + if let plugin = plugin as? DestinationPlugin, !plugin.key.isEmpty { + it["plugin"] = "\(plugin.type)-\(plugin.key)" + } else { + it["plugin"] = "\(plugin.type)-\(String(describing: plugin))" + } } } } diff --git a/Sources/Segment/Utilities/Telemetry.swift b/Sources/Segment/Utilities/Telemetry.swift index 55e4f87..53eb0a8 100644 --- a/Sources/Segment/Utilities/Telemetry.swift +++ b/Sources/Segment/Utilities/Telemetry.swift @@ -89,6 +89,7 @@ public class Telemetry: Subscriber { private var seenErrors = [String: Int]() internal var started = false private var rateLimitEndTime: TimeInterval = 0 + internal var flushFirstError = true private var telemetryQueue = DispatchQueue(label: "telemetryQueue") private var updateQueue = DispatchQueue(label: "updateQueue") private var telemetryTimer: QueueTimer? @@ -98,15 +99,10 @@ public class Telemetry: Subscriber { guard enable, !started, sampleRate > 0.0 && sampleRate <= 1.0 else { return } started = true + // Queue contents were sampled at the default 100% + // the values on flush will be adjusted in the send function if Double.random(in: 0...1) > sampleRate { resetQueue() - } else { - telemetryQueue.async { - self.queue = self.queue.map { var metric = $0 - metric.value = Int(Double(metric.value) / self.sampleRate) - return metric - } - } } self.telemetryTimer = QueueTimer(interval: .seconds(self.flushTimer), queue: .main) { [weak self] in @@ -133,13 +129,12 @@ public class Telemetry: Subscriber { /// - buildTags: A closure to build the tags dictionary. func increment(metric: String, buildTags: (inout [String: String]) -> Void) { guard enable, sampleRate > 0.0 && sampleRate <= 1.0, metric.hasPrefix(Telemetry.METRICS_BASE_TAG), queueHasSpace() else { return } + if Double.random(in: 0...1) > sampleRate { return } var tags = [String: String]() buildTags(&tags) guard !tags.isEmpty else { return } - if Double.random(in: 0...1) > sampleRate { return } - addRemoteMetric(metric: metric, tags: tags) } @@ -150,6 +145,7 @@ public class Telemetry: Subscriber { /// - buildTags: A closure to build the tags dictionary. func error(metric: String, log: String, buildTags: (inout [String: String]) -> Void) { guard enable, sampleRate > 0.0 && sampleRate <= 1.0, metric.hasPrefix(Telemetry.METRICS_BASE_TAG), queueHasSpace() else { return } + if Double.random(in: 0...1) > sampleRate { return } var tags = [String: String]() buildTags(&tags) @@ -165,19 +161,11 @@ public class Telemetry: Subscriber { logData = String(log.prefix(errorLogSizeMax)) } - if let errorKey = tags["error"] { - if let count = seenErrors[errorKey] { - seenErrors[errorKey] = count + 1 - if Double.random(in: 0...1) > sampleRate { return } - addRemoteMetric(metric: metric, tags: filteredTags, value: Int(Double(count) * sampleRate), log: logData) - seenErrors[errorKey] = 0 - } else { - addRemoteMetric(metric: metric, tags: filteredTags, log: logData) - flush() - seenErrors[errorKey] = 0 - } - } else { - addRemoteMetric(metric: metric, tags: filteredTags, log: logData) + addRemoteMetric(metric: metric, tags: filteredTags, log: logData) + + if (flushFirstError) { + flushFirstError = false + flush() } } diff --git a/Tests/Segment-Tests/Telemetry_Tests.swift b/Tests/Segment-Tests/Telemetry_Tests.swift index a182b33..9097fbd 100644 --- a/Tests/Segment-Tests/Telemetry_Tests.swift +++ b/Tests/Segment-Tests/Telemetry_Tests.swift @@ -116,6 +116,7 @@ class TelemetryTests: XCTestCase { func testHTTPException() { mockTelemetryHTTPClient(shouldThrow: true) + Telemetry.shared.flushFirstError = true Telemetry.shared.enable = true Telemetry.shared.start() Telemetry.shared.error(metric: Telemetry.INVOKE_METRIC, log: "log") { $0["error"] = "test" }