Skip to content

Commit c3bea1c

Browse files
MichaelGHSegMichaelGHSeg
authored
Fixing telemetry plugin name (#247)
* Fix plugin names * Add comments, simplify error logic * Adding destination plugin name update to mediator * adding caller to error metrics
1 parent 527b4bc commit c3bea1c

File tree

7 files changed

+62
-33
lines changed

7 files changed

+62
-33
lines changed

‎core/src/main/java/com/segment/analytics/kotlin/core/Analytics.kt

+1
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ open class Analytics protected constructor(
9898
Telemetry.INVOKE_ERROR_METRIC, t.stackTraceToString()) {
9999
it["error"] = t.toString()
100100
it["message"] = "Exception in Analytics Scope"
101+
it["caller"] = t.stackTrace[0].toString()
101102
}
102103
}
103104
override val analyticsScope = CoroutineScope(SupervisorJob() + exceptionHandler)

‎core/src/main/java/com/segment/analytics/kotlin/core/HTTPClient.kt

+1
Original file line numberDiff line numberDiff line change
@@ -39,6 +39,7 @@ class HTTPClient(
3939
it["error"] = e.toString()
4040
it["writekey"] = writeKey
4141
it["message"] = "Malformed url"
42+
it["caller"] = e.stackTrace[0].toString()
4243
}
4344
throw error
4445
}

‎core/src/main/java/com/segment/analytics/kotlin/core/Settings.kt

+1
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,7 @@ internal fun Analytics.fetchSettings(
124124
it["error"] = ex.toString()
125125
it["writekey"] = writeKey
126126
it["message"] = "Error retrieving settings"
127+
it["caller"] = ex.stackTrace[0].toString()
127128
}
128129
configuration.defaultSettings
129130
}

‎core/src/main/java/com/segment/analytics/kotlin/core/Telemetry.kt

+20-26
Original file line numberDiff line numberDiff line change
@@ -75,7 +75,8 @@ object Telemetry: Subscriber {
7575

7676
var host: String = Constants.DEFAULT_API_HOST
7777
// 1.0 is 100%, will get set by Segment setting before start()
78-
var sampleRate: Double = 0.1
78+
// Values are adjusted by the sampleRate on send
79+
var sampleRate: Double = 1.0
7980
var flushTimer: Int = 30 * 1000 // 30s
8081
var httpClient: HTTPClient = HTTPClient("", MetricsRequestFactory())
8182
var sendWriteKeyOnError: Boolean = true
@@ -96,6 +97,7 @@ object Telemetry: Subscriber {
9697
private val seenErrors = mutableMapOf<String, Int>()
9798
private var started = false
9899
private var rateLimitEndTime: Long = 0
100+
private var flushFirstError = true
99101
private val exceptionHandler = CoroutineExceptionHandler { _, t ->
100102
errorHandler?.let {
101103
it( Exception(
@@ -116,7 +118,7 @@ object Telemetry: Subscriber {
116118
if (!enable || started || sampleRate == 0.0) return
117119
started = true
118120

119-
// Assume sampleRate is now set and everything in the queue hasn't had it applied
121+
// Everything queued was sampled at default 100%, downsample adjustment and send will adjust values
120122
if (Math.random() > sampleRate) {
121123
resetQueue()
122124
}
@@ -187,9 +189,13 @@ object Telemetry: Subscriber {
187189
if (!metric.startsWith(METRICS_BASE_TAG)) return
188190
if (tags.isEmpty()) return
189191
if (queue.size >= maxQueueSize) return
192+
if (Math.random() > sampleRate) return
190193

191-
var filteredTags = tags.toMap()
192-
if (!sendWriteKeyOnError) filteredTags = tags.filterKeys { it.lowercase() != "writekey" }
194+
var filteredTags = if(sendWriteKeyOnError) {
195+
tags.toMap()
196+
} else {
197+
tags.filterKeys { it.lowercase() != "writekey" }
198+
}
193199
var logData: String? = null
194200
if (sendErrorLogData) {
195201
logData = if (log.length > errorLogSizeMax) {
@@ -199,23 +205,11 @@ object Telemetry: Subscriber {
199205
}
200206
}
201207

202-
val errorKey = tags["error"]
203-
if (errorKey != null) {
204-
if (seenErrors.containsKey(errorKey)) {
205-
seenErrors[errorKey] = seenErrors[errorKey]!! + 1
206-
if (Math.random() > sampleRate) return
207-
// Adjust how many we've seen after the first since we know for sure.
208-
addRemoteMetric(metric, filteredTags, log=logData,
209-
value = (seenErrors[errorKey]!! * sampleRate).toInt())
210-
seenErrors[errorKey] = 0
211-
} else {
212-
addRemoteMetric(metric, filteredTags, log=logData)
213-
flush()
214-
seenErrors[errorKey] = 0 // Zero because it's already been sent.
215-
}
216-
}
217-
else {
218-
addRemoteMetric(metric, filteredTags, log=logData)
208+
addRemoteMetric(metric, filteredTags, log=logData)
209+
210+
if(flushFirstError) {
211+
flushFirstError = false
212+
flush()
219213
}
220214
}
221215

@@ -339,12 +333,12 @@ object Telemetry: Subscriber {
339333
system.settings?.let { settings ->
340334
settings.metrics["sampleRate"]?.jsonPrimitive?.double?.let {
341335
sampleRate = it
336+
// We don't want to start telemetry until two conditions are met:
337+
// Telemetry.enable is set to true
338+
// Settings from the server have adjusted the sampleRate
339+
// start is called in both places
340+
start()
342341
}
343-
// We don't want to start telemetry until two conditions are met:
344-
// Telemetry.enable is set to true
345-
// Settings from the server have adjusted the sampleRate
346-
// start is called in both places
347-
start()
348342
}
349343
}
350344

‎core/src/main/java/com/segment/analytics/kotlin/core/platform/Mediator.kt

+18-4
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,11 @@ internal class Mediator(internal var plugins: CopyOnWriteArrayList<Plugin> = Cop
3737
try {
3838
Telemetry.increment(Telemetry.INTEGRATION_METRIC) {
3939
it["message"] = "event-${event.type}"
40-
"plugin" to "${plugin.type}-${plugin.javaClass}"
40+
if (plugin is DestinationPlugin && plugin.key != "") {
41+
it["plugin"] = "${plugin.type}-${plugin.key}"
42+
} else {
43+
it["plugin"] = "${plugin.type}-${plugin.javaClass}"
44+
}
4145
}
4246
when (plugin) {
4347
is DestinationPlugin -> {
@@ -52,9 +56,14 @@ internal class Mediator(internal var plugins: CopyOnWriteArrayList<Plugin> = Cop
5256
reportErrorWithMetrics(null, t,"Caught Exception in plugin",
5357
Telemetry.INTEGRATION_ERROR_METRIC, t.stackTraceToString()) {
5458
it["error"] = t.toString()
55-
it["plugin"] = "${plugin.type}-${plugin.javaClass}"
59+
if (plugin is DestinationPlugin && plugin.key != "") {
60+
it["plugin"] = "${plugin.type}-${plugin.key}"
61+
} else {
62+
it["plugin"] = "${plugin.type}-${plugin.javaClass}"
63+
}
5664
it["writekey"] = plugin.analytics.configuration.writeKey
57-
it["message"] ="Exception executing plugin"
65+
it["message"] = "Exception executing plugin"
66+
it["caller"] = t.stackTrace[0].toString()
5867
}
5968
}
6069
}
@@ -72,9 +81,14 @@ internal class Mediator(internal var plugins: CopyOnWriteArrayList<Plugin> = Cop
7281
"Caught Exception applying closure to plugin: $plugin",
7382
Telemetry.INTEGRATION_ERROR_METRIC, t.stackTraceToString()) {
7483
it["error"] = t.toString()
75-
it["plugin"] = "${plugin.type}-${plugin.javaClass}"
84+
if (plugin is DestinationPlugin && plugin.key != "") {
85+
it["plugin"] = "${plugin.type}-${plugin.key}"
86+
} else {
87+
it["plugin"] = "${plugin.type}-${plugin.javaClass}"
88+
}
7689
it["writekey"] = plugin.analytics.configuration.writeKey
7790
it["message"] = "Exception executing plugin"
91+
it["caller"] = t.stackTrace[0].toString()
7892
}
7993
}
8094
}

‎core/src/main/java/com/segment/analytics/kotlin/core/platform/Timeline.kt

+15-3
Original file line numberDiff line numberDiff line change
@@ -73,14 +73,22 @@ internal class Timeline {
7373
"Caught Exception while setting up plugin $plugin",
7474
Telemetry.INTEGRATION_ERROR_METRIC, t.stackTraceToString()) {
7575
it["error"] = t.toString()
76-
it["plugin"] = "${plugin.type}-${plugin.javaClass}"
76+
if (plugin is DestinationPlugin && plugin.key != "") {
77+
it["plugin"] = "${plugin.type}-${plugin.key}"
78+
} else {
79+
it["plugin"] = "${plugin.type}-${plugin.javaClass}"
80+
}
7781
it["writekey"] = analytics.configuration.writeKey
7882
it["message"] = "Exception executing plugin"
7983
}
8084
}
8185
Telemetry.increment(Telemetry.INTEGRATION_METRIC) {
8286
it["message"] = "added"
83-
it["plugin"] = "${plugin.type.toString()}-${plugin.javaClass.toString()}"
87+
if (plugin is DestinationPlugin && plugin.key != "") {
88+
it["plugin"] = "${plugin.type}-${plugin.key}"
89+
} else {
90+
it["plugin"] = "${plugin.type}-${plugin.javaClass}"
91+
}
8492
}
8593
plugins[plugin.type]?.add(plugin)
8694
with(analytics) {
@@ -109,7 +117,11 @@ internal class Timeline {
109117
list.remove(plugin)
110118
Telemetry.increment(Telemetry.INTEGRATION_METRIC) {
111119
it["message"] = "removed"
112-
it["plugin"] = "${plugin.type.toString()}-${plugin.javaClass.toString()}"
120+
if (plugin is DestinationPlugin && plugin.key != "") {
121+
it["plugin"] = "${plugin.type}-${plugin.key}"
122+
} else {
123+
it["plugin"] = "${plugin.type}-${plugin.javaClass}"
124+
}
113125
}
114126
}
115127
}

‎core/src/test/kotlin/com/segment/analytics/kotlin/core/TelemetryTest.kt

+6
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,11 @@ import java.net.HttpURLConnection
99
import java.util.concurrent.ConcurrentLinkedQueue
1010

1111
class TelemetryTest {
12+
fun TelemetryResetFlushFirstError() {
13+
val field: Field = Telemetry::class.java.getDeclaredField("flushFirstError")
14+
field.isAccessible = true
15+
field.set(true, true)
16+
}
1217
fun TelemetryQueueSize(): Int {
1318
val queueField: Field = Telemetry::class.java.getDeclaredField("queue")
1419
queueField.isAccessible = true
@@ -163,6 +168,7 @@ class TelemetryTest {
163168
@Test
164169
fun `Test HTTP Exception`() {
165170
mockTelemetryHTTPClient(shouldThrow = true)
171+
TelemetryResetFlushFirstError()
166172
Telemetry.enable = true
167173
Telemetry.start()
168174
Telemetry.error(Telemetry.INVOKE_METRIC,"log") { it["error"] = "test" }

0 commit comments

Comments
 (0)