Skip to content

Commit 24a7e74

Browse files
bidetofevilbidetofevil
authored
Add truncation logic go span names and attribute keys/values (#2402)
## Goal Previously, we dropped any attribute and spans whose names are too long. Now, we implementation truncation logic so that it will always succeed. There may be clashes as a result, but it should be apparent in the data as the names/keys/values will end with three dots.
1 parent 8576bb9 commit 24a7e74

File tree

15 files changed

+184
-102
lines changed

15 files changed

+184
-102
lines changed

‎embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/spans/CurrentSessionSpanImplTests.kt‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@ import io.embrace.android.embracesdk.fakes.injection.FakeInitModule
1414
import io.embrace.android.embracesdk.internal.arch.destination.SpanAttributeData
1515
import io.embrace.android.embracesdk.internal.arch.schema.SchemaType
1616
import io.embrace.android.embracesdk.internal.clock.nanosToMillis
17-
import io.embrace.android.embracesdk.internal.config.instrumented.InstrumentedConfigImpl
17+
import io.embrace.android.embracesdk.internal.config.instrumented.schema.OtelLimitsConfig
1818
import io.embrace.android.embracesdk.internal.otel.attrs.asPair
1919
import io.embrace.android.embracesdk.internal.otel.config.getMaxTotalAttributeCount
2020
import io.embrace.android.embracesdk.internal.otel.payload.toEmbracePayload
@@ -49,6 +49,7 @@ internal class CurrentSessionSpanImplTests {
4949

5050
private lateinit var spanRepository: SpanRepository
5151
private lateinit var spanSink: SpanSink
52+
private lateinit var otelLimitsConfig: OtelLimitsConfig
5253
private lateinit var telemetryService: TelemetryService
5354
private lateinit var openTelemetryClock: Clock
5455
private lateinit var currentSessionSpan: CurrentSessionSpanImpl
@@ -67,6 +68,7 @@ internal class CurrentSessionSpanImplTests {
6768
tracer = initModule.openTelemetryModule.otelSdkWrapper.sdkTracer
6869
spanService = initModule.openTelemetryModule.spanService
6970
spanService.initializeService(clock.now())
71+
otelLimitsConfig = initModule.instrumentedConfig.otelLimits
7072
}
7173

7274
@Test
@@ -528,7 +530,7 @@ internal class CurrentSessionSpanImplTests {
528530

529531
@Test
530532
fun `validate maximum events on session span`() {
531-
val limit = InstrumentedConfigImpl.otelLimits.getMaxSystemEventCount()
533+
val limit = otelLimitsConfig.getMaxSystemEventCount()
532534
repeat(limit + 1) {
533535
currentSessionSpan.addSessionEvent(SchemaType.Breadcrumb("test-event"), 1000L + it)
534536
}
@@ -555,7 +557,7 @@ internal class CurrentSessionSpanImplTests {
555557

556558
@Test
557559
fun `validate maximum attributes on session span`() {
558-
val limit = InstrumentedConfigImpl.otelLimits.getMaxTotalAttributeCount()
560+
val limit = otelLimitsConfig.getMaxTotalAttributeCount()
559561
repeat(limit + 1) {
560562
currentSessionSpan.addSessionAttribute(SpanAttributeData("attribute-$it", "value"))
561563
}

‎embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/spans/EmbraceTracerTest.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -101,8 +101,8 @@ internal class EmbraceTracerTest {
101101
}
102102

103103
@Test
104-
fun `cannot start a span if it was not created`() {
105-
assertNull(embraceTracer.startSpan(name = TOO_LONG_SPAN_NAME))
104+
fun `long span names truncated`() {
105+
assertNotNull(embraceTracer.startSpan(name = TOO_LONG_SPAN_NAME))
106106
}
107107

108108
@Test

embrace-android-core/src/main/kotlin/io/embrace/android/embracesdk/internal/utils/PropertyUtils.kt renamed to embrace-android-infra/src/main/kotlin/io/embrace/android/embracesdk/internal/utils/PropertyUtils.kt

File renamed without changes.

embrace-android-core/src/test/java/io/embrace/android/embracesdk/internal/utils/PropertyUtilsTest.kt renamed to embrace-android-infra/src/test/kotlin/io/embrace/android/embracesdk/internal/utils/PropertyUtilsTest.kt

File renamed without changes.

‎embrace-android-otel/src/main/kotlin/io/embrace/android/embracesdk/internal/otel/sdk/DataValidator.kt‎

Lines changed: 68 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ package io.embrace.android.embracesdk.internal.otel.sdk
22

33
import io.embrace.android.embracesdk.internal.config.instrumented.OtelLimitsConfigImpl
44
import io.embrace.android.embracesdk.internal.config.instrumented.schema.OtelLimitsConfig
5+
import io.embrace.android.embracesdk.internal.utils.PropertyUtils
56
import io.embrace.android.embracesdk.spans.EmbraceSpanEvent
67

78
/**
@@ -11,45 +12,87 @@ class DataValidator(
1112
val otelLimitsConfig: OtelLimitsConfig = OtelLimitsConfigImpl,
1213
private val bypassValidation: (() -> Boolean) = { false },
1314
) {
14-
fun isNameValid(str: String, internal: Boolean): Boolean {
15-
return if (internal) {
16-
str.isNotBlank() && str.length <= otelLimitsConfig.getMaxInternalNameLength()
17-
} else if (!bypassValidation()) {
18-
str.isNotBlank() && str.length <= otelLimitsConfig.getMaxNameLength()
15+
fun truncateSpanName(name: String, internal: Boolean): String {
16+
val maxLength = if (internal) {
17+
otelLimitsConfig.getMaxInternalNameLength()
1918
} else {
20-
true
19+
otelLimitsConfig.getMaxNameLength()
2120
}
21+
return PropertyUtils.truncate(name, maxLength)
2222
}
2323

24-
fun isEventCountValid(events: List<EmbraceSpanEvent>, internal: Boolean): Boolean {
24+
fun truncateEvents(events: List<EmbraceSpanEvent>, internal: Boolean): List<EmbraceSpanEvent> {
2525
return if (internal) {
26-
events.size <= otelLimitsConfig.getMaxSystemEventCount()
26+
events.take(otelLimitsConfig.getMaxSystemEventCount())
2727
} else if (!bypassValidation()) {
28-
events.size <= otelLimitsConfig.getMaxCustomEventCount()
28+
events.take(otelLimitsConfig.getMaxCustomEventCount())
2929
} else {
30-
true
30+
events
3131
}
3232
}
3333

34-
fun isAttributeCountValid(attributes: Map<String, String>, internal: Boolean): Boolean {
35-
return if (internal) {
36-
attributes.size <= otelLimitsConfig.getMaxSystemAttributeCount()
37-
} else if (!bypassValidation()) {
38-
attributes.size <= otelLimitsConfig.getMaxCustomAttributeCount()
34+
fun truncateAttributes(attributes: Map<String, String>, internal: Boolean): Map<String, String> {
35+
val maxAttributeCount = if (internal) {
36+
otelLimitsConfig.getMaxSystemAttributeCount()
37+
} else {
38+
otelLimitsConfig.getMaxCustomAttributeCount()
39+
}
40+
val maxKeyLength = if (internal) {
41+
otelLimitsConfig.getMaxInternalAttributeKeyLength()
42+
} else {
43+
otelLimitsConfig.getMaxCustomAttributeKeyLength()
44+
}
45+
val maxValueLength = if (internal) {
46+
otelLimitsConfig.getMaxInternalAttributeValueLength()
47+
} else {
48+
otelLimitsConfig.getMaxCustomAttributeValueLength()
49+
}
50+
51+
return if (internal || !bypassValidation()) {
52+
attributes.truncate(
53+
maxCount = maxAttributeCount,
54+
maxKeyLength = maxKeyLength,
55+
maxValueLength = maxValueLength
56+
)
3957
} else {
40-
true
58+
attributes
4159
}
4260
}
4361

44-
fun isAttributeValid(key: String, value: String, internal: Boolean): Boolean {
45-
with(otelLimitsConfig) {
46-
return if (internal) {
47-
key.length <= getMaxInternalAttributeKeyLength() && value.length <= getMaxInternalAttributeValueLength()
48-
} else if (!bypassValidation()) {
49-
key.length <= getMaxCustomAttributeKeyLength() && value.length <= getMaxCustomAttributeValueLength()
50-
} else {
51-
true
52-
}
62+
fun truncateAttribute(key: String, value: String, internal: Boolean): Pair<String, String> {
63+
val maxKeyLength = if (internal) {
64+
otelLimitsConfig.getMaxInternalAttributeKeyLength()
65+
} else {
66+
otelLimitsConfig.getMaxCustomAttributeKeyLength()
67+
}
68+
val maxValueLength = if (internal) {
69+
otelLimitsConfig.getMaxInternalAttributeValueLength()
70+
} else {
71+
otelLimitsConfig.getMaxCustomAttributeValueLength()
5372
}
73+
74+
val truncatedValue = if (key.isValidLongValueAttribute()) {
75+
value
76+
} else {
77+
PropertyUtils.truncate(value, maxValueLength)
78+
}
79+
80+
return Pair(PropertyUtils.truncate(key, maxKeyLength), truncatedValue)
5481
}
82+
83+
private fun Map<String, String>.truncate(
84+
maxCount: Int,
85+
maxKeyLength: Int,
86+
maxValueLength: Int,
87+
): Map<String, String> =
88+
mutableMapOf<String, String>().apply {
89+
this@truncate.entries.take(maxCount).forEach {
90+
val truncatedValue = if (it.key.isValidLongValueAttribute()) {
91+
it.value
92+
} else {
93+
PropertyUtils.truncate(it.value, maxValueLength)
94+
}
95+
this[PropertyUtils.truncate(it.key, maxKeyLength)] = truncatedValue
96+
}
97+
}
5598
}

‎embrace-android-otel/src/main/kotlin/io/embrace/android/embracesdk/internal/otel/sdk/OtelExt.kt‎

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,16 +24,14 @@ import io.embrace.opentelemetry.kotlin.tracing.model.ReadableSpanEvent
2424
import io.embrace.opentelemetry.kotlin.tracing.model.Span
2525

2626
/**
27-
* Populate an [AttributesBuilder] with String key-value pairs from a [Map]
27+
* Populate an AttributesBuilder with String key-value pairs from a [Map]
2828
*/
2929
fun OtelJavaAttributesBuilder.fromMap(
3030
attributes: Map<String, String>,
3131
internal: Boolean,
32-
limitsValidator: DataValidator,
32+
dataValidator: DataValidator,
3333
): OtelJavaAttributesBuilder {
34-
attributes.filter {
35-
limitsValidator.isAttributeValid(it.key, it.value, internal) || it.key.isValidLongValueAttribute()
36-
}.forEach {
34+
dataValidator.truncateAttributes(attributes, internal).forEach {
3735
put(it.key, it.value)
3836
}
3937
return this

‎embrace-android-otel/src/main/kotlin/io/embrace/android/embracesdk/internal/otel/spans/EmbraceSpanFactoryImpl.kt‎

Lines changed: 11 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -291,12 +291,15 @@ private class EmbraceSpanImpl(
291291
override fun getStartTimeMs(): Long? = spanStartTimeMs
292292

293293
override fun addAttribute(key: String, value: String): Boolean {
294-
if (customAttributes.size < dataValidator.otelLimitsConfig.getMaxCustomAttributeCount() &&
295-
dataValidator.isAttributeValid(key, value, otelSpanCreator.spanStartArgs.internal)
296-
) {
294+
if (customAttributes.size < dataValidator.otelLimitsConfig.getMaxCustomAttributeCount() && key.isNotBlank()) {
297295
synchronized(customAttributes) {
298296
if (customAttributes.size < dataValidator.otelLimitsConfig.getMaxCustomAttributeCount() && isRecording) {
299-
customAttributes[key] = value
297+
val attribute = dataValidator.truncateAttribute(
298+
key = key,
299+
value = value,
300+
internal = otelSpanCreator.spanStartArgs.internal
301+
)
302+
customAttributes[attribute.first] = attribute.second
300303
spanRepository.notifySpanUpdate()
301304
return true
302305
}
@@ -307,11 +310,12 @@ private class EmbraceSpanImpl(
307310
}
308311

309312
override fun updateName(newName: String): Boolean {
310-
if (dataValidator.isNameValid(newName, otelSpanCreator.spanStartArgs.internal)) {
313+
if (newName.isNotBlank()) {
311314
synchronized(startedSpan) {
312315
if (!spanStarted() || isRecording) {
313-
updatedName = newName
314-
startedSpan.get()?.name = newName
316+
val validatedName = dataValidator.truncateSpanName(newName, otelSpanCreator.spanStartArgs.internal)
317+
updatedName = validatedName
318+
startedSpan.get()?.name = validatedName
315319
spanRepository.notifySpanUpdate()
316320
return true
317321
}

‎embrace-android-otel/src/main/kotlin/io/embrace/android/embracesdk/internal/otel/spans/OtelSpanStartArgs.kt‎

Lines changed: 12 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -24,11 +24,10 @@ class OtelSpanStartArgs(
2424
) {
2525
var parentContext: OtelJavaContext = OtelJavaContext.root()
2626

27-
val spanName: String = if (internal) {
28-
name.toEmbraceObjectName()
29-
} else {
30-
name
31-
}
27+
var spanName: String = name.appendEmbracePrefix(internal)
28+
set(value) {
29+
field = value.appendEmbracePrefix(internal)
30+
}
3231

3332
var startTimeMs: Long? = null
3433
var spanKind: OtelJavaSpanKind? = null
@@ -59,4 +58,12 @@ class OtelSpanStartArgs(
5958
null
6059
}
6160
}
61+
62+
private fun String.appendEmbracePrefix(internal: Boolean): String {
63+
return if (internal) {
64+
toEmbraceObjectName()
65+
} else {
66+
this
67+
}
68+
}
6269
}

‎embrace-android-otel/src/main/kotlin/io/embrace/android/embracesdk/internal/otel/spans/SpanServiceImpl.kt‎

Lines changed: 14 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -40,9 +40,9 @@ class SpanServiceImpl(
4040
autoTerminationMode: AutoTerminationMode,
4141
): EmbraceSdkSpan? {
4242
EmbTrace.trace("span-create") {
43-
return if (dataValidator.isNameValid(name, internal) && canStartNewSpan(parent, internal)) {
43+
return if (name.isNotBlank() && canStartNewSpan(parent, internal)) {
4444
embraceSpanFactory.create(
45-
name = name,
45+
name = dataValidator.truncateSpanName(name, internal),
4646
type = type,
4747
internal = internal,
4848
private = private,
@@ -57,9 +57,11 @@ class SpanServiceImpl(
5757

5858
override fun createSpan(otelSpanCreator: OtelSpanCreator): EmbraceSdkSpan? {
5959
EmbTrace.trace("span-create") {
60-
val otelSpanStartArgs = otelSpanCreator.spanStartArgs
60+
val otelSpanStartArgs = otelSpanCreator.spanStartArgs.apply {
61+
spanName = dataValidator.truncateSpanName(spanName, internal)
62+
}
6163
return if (
62-
dataValidator.isNameValid(otelSpanStartArgs.spanName, otelSpanStartArgs.internal) &&
64+
otelSpanStartArgs.spanName.isNotBlank() &&
6365
canStartNewSpan(
6466
otelSpanStartArgs.parentContext.getEmbraceSpan(),
6567
otelSpanStartArgs.internal
@@ -133,19 +135,23 @@ class SpanServiceImpl(
133135
return false
134136
}
135137

136-
if (inputsValid(name, internal, events, attributes) && canStartNewSpan(parent, internal)) {
138+
val validName = dataValidator.truncateSpanName(name, internal)
139+
val validEvents = dataValidator.truncateEvents(events, internal)
140+
val validAttributes = dataValidator.truncateAttributes(attributes, internal)
141+
142+
if (canStartNewSpan(parent, internal)) {
137143
val newSpan = embraceSpanFactory.create(
138-
name = name,
144+
name = validName,
139145
type = type,
140146
internal = internal,
141147
private = private,
142148
parent = parent,
143149
)
144150
if (newSpan.start(startTimeMs)) {
145-
attributes.forEach {
151+
validAttributes.forEach {
146152
newSpan.addAttribute(it.key, it.value)
147153
}
148-
events.forEach {
154+
validEvents.forEach {
149155
newSpan.addEvent(it.name, it.timestampNanos.nanosToMillis(), it.attributes)
150156
}
151157
return newSpan.stop(errorCode, endTimeMs)
@@ -157,13 +163,4 @@ class SpanServiceImpl(
157163
}
158164

159165
override fun getSpan(spanId: String): EmbraceSpan? = spanRepository.getSpan(spanId = spanId)
160-
161-
private fun inputsValid(
162-
name: String,
163-
internal: Boolean,
164-
events: List<EmbraceSpanEvent>,
165-
attributes: Map<String, String>,
166-
): Boolean = dataValidator.isNameValid(name, internal) &&
167-
dataValidator.isEventCountValid(events, internal) &&
168-
dataValidator.isAttributeCountValid(attributes, internal)
169166
}

‎embrace-android-otel/src/test/kotlin/io/embrace/android/embracesdk/internal/otel/spans/EmbraceSpanImplTest.kt‎

Lines changed: 23 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,11 @@ import io.embrace.android.embracesdk.fixtures.TOO_LONG_ATTRIBUTE_VALUE
1717
import io.embrace.android.embracesdk.fixtures.TOO_LONG_ATTRIBUTE_VALUE_FOR_INTERNAL_SPAN
1818
import io.embrace.android.embracesdk.fixtures.TOO_LONG_EVENT_NAME
1919
import io.embrace.android.embracesdk.fixtures.TOO_LONG_SPAN_NAME
20+
import io.embrace.android.embracesdk.fixtures.TRUNCATED_TOO_LONG_ATTRIBUTE_KEY
21+
import io.embrace.android.embracesdk.fixtures.TRUNCATED_TOO_LONG_ATTRIBUTE_KEY_FOR_INTERNAL_SPAN
22+
import io.embrace.android.embracesdk.fixtures.TRUNCATED_TOO_LONG_ATTRIBUTE_VALUE
23+
import io.embrace.android.embracesdk.fixtures.TRUNCATED_TOO_LONG_ATTRIBUTE_VALUE_FOR_INTERNAL_SPAN
24+
import io.embrace.android.embracesdk.fixtures.TRUNCATED_TOO_LONG_SPAN_NAME
2025
import io.embrace.android.embracesdk.fixtures.fakeContextKey
2126
import io.embrace.android.embracesdk.fixtures.maxSizeEventAttributes
2227
import io.embrace.android.embracesdk.fixtures.tooBigEventAttributes
@@ -231,8 +236,9 @@ internal class EmbraceSpanImplTest {
231236
@Test
232237
fun `span name update`() {
233238
with(embraceSpan) {
239+
assertTrue(embraceSpan.updateName(TOO_LONG_SPAN_NAME))
240+
assertEquals(TRUNCATED_TOO_LONG_SPAN_NAME, embraceSpan.name())
234241
assertTrue(embraceSpan.updateName("new-name"))
235-
assertFalse(embraceSpan.updateName(TOO_LONG_SPAN_NAME))
236242
assertFalse(embraceSpan.updateName(""))
237243
assertFalse(embraceSpan.updateName(" "))
238244
assertTrue(start())
@@ -371,16 +377,22 @@ internal class EmbraceSpanImplTest {
371377
fun `check custom attribute limits`() {
372378
with(embraceSpan) {
373379
assertTrue(start())
374-
assertFalse(addAttribute(key = TOO_LONG_ATTRIBUTE_KEY, value = "value"))
375-
assertFalse(addAttribute(key = "key", value = TOO_LONG_ATTRIBUTE_VALUE))
380+
assertTrue(addAttribute(key = TOO_LONG_ATTRIBUTE_KEY, value = "long-key-value"))
381+
assertTrue(addAttribute(key = "long-value-key", value = TOO_LONG_ATTRIBUTE_VALUE))
376382
assertTrue(addAttribute(key = MAX_LENGTH_ATTRIBUTE_KEY, value = "value"))
377383
assertTrue(addAttribute(key = "key", value = MAX_LENGTH_ATTRIBUTE_VALUE))
378384
assertTrue(addAttribute(key = "Key", value = MAX_LENGTH_ATTRIBUTE_VALUE))
379-
repeat(dataValidator.otelLimitsConfig.getMaxCustomAttributeCount() - 3) {
385+
repeat(dataValidator.otelLimitsConfig.getMaxCustomAttributeCount() - 5) {
380386
assertTrue(addAttribute(key = "key$it", value = "value"))
381387
}
382388
assertFalse(addAttribute(key = "failedKey", value = "value"))
383389
assertEquals(dataValidator.otelLimitsConfig.getMaxCustomAttributeCount() + 1, attributes().size)
390+
val combinedAttributes = attributes()
391+
with(combinedAttributes) {
392+
assertEquals(TRUNCATED_TOO_LONG_ATTRIBUTE_VALUE, combinedAttributes["long-value-key"])
393+
assertEquals("long-key-value", combinedAttributes[TRUNCATED_TOO_LONG_ATTRIBUTE_KEY])
394+
}
395+
384396
assertTrue(updateNotified)
385397
}
386398
}
@@ -390,11 +402,16 @@ internal class EmbraceSpanImplTest {
390402
embraceSpan = createInternalEmbraceSdkSpan()
391403
with(embraceSpan) {
392404
assertTrue(start())
393-
assertFalse(addAttribute(key = TOO_LONG_ATTRIBUTE_KEY_FOR_INTERNAL_SPAN, value = "value"))
394-
assertFalse(addAttribute(key = "key", value = TOO_LONG_ATTRIBUTE_VALUE_FOR_INTERNAL_SPAN))
405+
assertTrue(addAttribute(key = TOO_LONG_ATTRIBUTE_KEY_FOR_INTERNAL_SPAN, value = "long-key-value"))
406+
assertTrue(addAttribute(key = "long-value-key", value = TOO_LONG_ATTRIBUTE_VALUE_FOR_INTERNAL_SPAN))
395407
assertTrue(addAttribute(key = MAX_LENGTH_ATTRIBUTE_KEY_FOR_INTERNAL_SPAN, value = "value"))
396408
assertTrue(addAttribute(key = "key", value = MAX_LENGTH_ATTRIBUTE_VALUE_FOR_INTERNAL_SPAN))
397409
assertTrue(addAttribute(key = "Key", value = MAX_LENGTH_ATTRIBUTE_VALUE_FOR_INTERNAL_SPAN))
410+
val combinedAttributes = attributes()
411+
with(combinedAttributes) {
412+
assertEquals("long-key-value", combinedAttributes[TRUNCATED_TOO_LONG_ATTRIBUTE_KEY_FOR_INTERNAL_SPAN])
413+
assertEquals(TRUNCATED_TOO_LONG_ATTRIBUTE_VALUE_FOR_INTERNAL_SPAN, combinedAttributes["long-value-key"])
414+
}
398415
}
399416
}
400417

0 commit comments

Comments
 (0)