Skip to content

Commit 8576bb9

Browse files
bidetofevilbidetofevil
authored
Fix filtering in attributes() getter function (#2401)
## Goal Filter the filtering conditions and add a tests for the getters that aren't explicitly used and tested via `snapshot()` already.
1 parent 1556959 commit 8576bb9

File tree

3 files changed

+28
-10
lines changed

3 files changed

+28
-10
lines changed

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,7 @@ class EmbraceSpanFactoryImpl(
5353
private val tracer: Tracer,
5454
private val openTelemetryClock: Clock,
5555
private val spanRepository: SpanRepository,
56-
private val dataValidator: DataValidator = DataValidator(),
56+
private val dataValidator: DataValidator,
5757
private val stopCallback: ((spanId: String) -> Unit)? = null,
5858
private var redactionFunction: ((key: String, value: String) -> String)? = null,
5959
) : EmbraceSpanFactory {
@@ -390,7 +390,7 @@ private class EmbraceSpanImpl(
390390

391391
override fun attributes(): Map<String, Any> {
392392
val raw = getAttributesPayload()
393-
val attrs = raw.filter { it.key == null || it.data == null }
393+
val attrs = raw.filter { it.key != null && it.data != null }
394394
return attrs.associate { Pair(checkNotNull(it.key), checkNotNull(it.data)) }
395395
}
396396

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

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import io.embrace.android.embracesdk.fakes.FakeOtelJavaTracer
66
import io.embrace.android.embracesdk.fakes.FakeOtelKotlinClock
77
import io.embrace.android.embracesdk.internal.otel.schema.EmbType
88
import io.embrace.android.embracesdk.internal.otel.schema.PrivateSpan
9+
import io.embrace.android.embracesdk.internal.otel.sdk.DataValidator
910
import io.embrace.android.embracesdk.internal.otel.sdk.otelSpanCreator
1011
import io.embrace.opentelemetry.kotlin.ExperimentalApi
1112
import io.embrace.opentelemetry.kotlin.k2j.tracing.TracerAdapter
@@ -39,6 +40,7 @@ internal class EmbraceSpanFactoryImplTest {
3940
tracer = tracer,
4041
openTelemetryClock = openTelemetryClock,
4142
spanRepository = spanRepository,
43+
dataValidator = DataValidator()
4244
)
4345
}
4446

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

Lines changed: 24 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -22,10 +22,10 @@ import io.embrace.android.embracesdk.fixtures.maxSizeEventAttributes
2222
import io.embrace.android.embracesdk.fixtures.tooBigEventAttributes
2323
import io.embrace.android.embracesdk.internal.clock.millisToNanos
2424
import io.embrace.android.embracesdk.internal.clock.nanosToMillis
25-
import io.embrace.android.embracesdk.internal.config.instrumented.InstrumentedConfigImpl
2625
import io.embrace.android.embracesdk.internal.otel.schema.EmbType
2726
import io.embrace.android.embracesdk.internal.otel.schema.LinkType
2827
import io.embrace.android.embracesdk.internal.otel.schema.PrivateSpan
28+
import io.embrace.android.embracesdk.internal.otel.sdk.DataValidator
2929
import io.embrace.android.embracesdk.internal.otel.sdk.findAttributeValue
3030
import io.embrace.android.embracesdk.internal.otel.sdk.id.OtelIds
3131
import io.embrace.android.embracesdk.internal.otel.sdk.otelSpanCreator
@@ -38,6 +38,7 @@ import io.embrace.opentelemetry.kotlin.ExperimentalApi
3838
import io.embrace.opentelemetry.kotlin.OpenTelemetryInstance
3939
import io.embrace.opentelemetry.kotlin.k2j.tracing.SpanContextAdapter
4040
import io.embrace.opentelemetry.kotlin.kotlinApi
41+
import io.embrace.opentelemetry.kotlin.tracing.model.SpanKind
4142
import io.opentelemetry.semconv.ExceptionAttributes
4243
import org.junit.Assert.assertEquals
4344
import org.junit.Assert.assertFalse
@@ -53,6 +54,7 @@ internal class EmbraceSpanImplTest {
5354
private lateinit var embraceSpan: EmbraceSdkSpan
5455
private lateinit var spanRepository: SpanRepository
5556
private lateinit var serializer: PlatformSerializer
57+
private lateinit var dataValidator: DataValidator
5658
private lateinit var embraceSpanFactory: EmbraceSpanFactory
5759
private var updateNotified: Boolean = false
5860
private var stoppedSpanId: String? = null
@@ -66,10 +68,12 @@ internal class EmbraceSpanImplTest {
6668
val otelClock = FakeOtelKotlinClock(fakeClock)
6769
spanRepository = SpanRepository().apply { setSpanUpdateNotifier { updateNotified = true } }
6870
serializer = TestPlatformSerializer()
71+
dataValidator = DataValidator()
6972
embraceSpanFactory = EmbraceSpanFactoryImpl(
7073
tracer = tracer,
7174
openTelemetryClock = otelClock,
7275
spanRepository = spanRepository,
76+
dataValidator = dataValidator,
7377
stopCallback = ::stopCallback,
7478
redactionFunction = ::redactionFunction
7579
)
@@ -94,6 +98,7 @@ internal class EmbraceSpanImplTest {
9498
assertFalse(addAttribute("first", "value"))
9599
assertEquals(0, spanRepository.getActiveSpans().size)
96100
assertEquals(0, spanRepository.getCompletedSpans().size)
101+
assertEquals(SpanKind.INTERNAL, spanKind)
97102
assertNull(embraceSpan.snapshot())
98103
}
99104
assertFalse(updateNotified)
@@ -109,6 +114,10 @@ internal class EmbraceSpanImplTest {
109114
assertNotNull(traceId)
110115
assertNotNull(spanId)
111116
assertTrue(isRecording)
117+
assertEquals(EXPECTED_SPAN_NAME, name())
118+
assertEquals(SpanKind.INTERNAL, spanKind)
119+
assertEquals(expectedStartTimeMs, getStartTimeMs())
120+
assertEquals("emb.type", attributes().entries.single().key)
112121
assertSnapshot(expectedStartTimeMs = expectedStartTimeMs, expectedEndTimeMs = null)
113122
assertTrue(addEvent("eventName"))
114123
assertTrue(addAttribute("first", "value"))
@@ -227,11 +236,14 @@ internal class EmbraceSpanImplTest {
227236
assertFalse(embraceSpan.updateName(""))
228237
assertFalse(embraceSpan.updateName(" "))
229238
assertTrue(start())
239+
assertEquals("new-name", embraceSpan.name())
230240
assertEquals("new-name", embraceSpan.snapshot()?.name)
231241
assertTrue(embraceSpan.updateName("new-new-name"))
242+
assertEquals("new-new-name", embraceSpan.name())
232243
assertEquals("new-new-name", embraceSpan.snapshot()?.name)
233244
assertTrue(stop())
234245
assertFalse(embraceSpan.updateName("failed-to-update"))
246+
assertEquals("new-new-name", embraceSpan.name())
235247
assertEquals("new-new-name", embraceSpan.snapshot()?.name)
236248
assertTrue(updateNotified)
237249
}
@@ -260,7 +272,7 @@ internal class EmbraceSpanImplTest {
260272
val sanitizedEvents = checkNotNull(events)
261273
assertEquals(2, sanitizedEvents.size)
262274
with(sanitizedEvents.first()) {
263-
assertEquals(InstrumentedConfigImpl.otelLimits.getExceptionEventName(), name)
275+
assertEquals(dataValidator.otelLimitsConfig.getExceptionEventName(), name)
264276
val attrs = checkNotNull(attributes)
265277
assertEquals(timestampNanos, timestampNanos)
266278
assertEquals(
@@ -274,7 +286,7 @@ internal class EmbraceSpanImplTest {
274286
)
275287
}
276288
with(sanitizedEvents.last()) {
277-
assertEquals(InstrumentedConfigImpl.otelLimits.getExceptionEventName(), name)
289+
assertEquals(dataValidator.otelLimitsConfig.getExceptionEventName(), name)
278290
val attrs = checkNotNull(attributes)
279291
assertEquals(timestampNanos, timestampNanos)
280292
assertEquals(
@@ -313,7 +325,7 @@ internal class EmbraceSpanImplTest {
313325
assertTrue(addEvent(name = MAX_LENGTH_EVENT_NAME, timestampMs = null, attributes = null))
314326
assertTrue(addEvent(name = "yo", timestampMs = null, attributes = maxSizeEventAttributes))
315327
assertTrue(recordException(exception = RuntimeException()))
316-
repeat(InstrumentedConfigImpl.otelLimits.getMaxCustomEventCount() - 5) {
328+
repeat(dataValidator.otelLimitsConfig.getMaxCustomEventCount() - 5) {
317329
assertTrue(addEvent(name = "event $it"))
318330
}
319331
val eventAttributesAMap = mutableMapOf(
@@ -340,14 +352,17 @@ internal class EmbraceSpanImplTest {
340352
fun `check adding and removing system attributes not affected by custom attributes`() {
341353
with(embraceSpan) {
342354
assertTrue(start())
343-
repeat(InstrumentedConfigImpl.otelLimits.getMaxCustomAttributeCount()) {
355+
val maxCustomAttrCount = dataValidator.otelLimitsConfig.getMaxCustomAttributeCount()
356+
repeat(maxCustomAttrCount) {
344357
assertTrue(addAttribute(key = "key$it", value = "value"))
345358
}
346359
assertFalse(addAttribute(key = "failed", value = "value"))
347360
addSystemAttribute("system-attribute", "value")
348361
assertEquals("value", embraceSpan.snapshot()?.attributes?.findAttributeValue("system-attribute"))
362+
assertEquals(maxCustomAttrCount + 2, attributes().size)
349363
removeSystemAttribute("system-attribute")
350364
assertNull("value", embraceSpan.snapshot()?.attributes?.findAttributeValue("system-attribute"))
365+
assertEquals(maxCustomAttrCount + 1, attributes().size)
351366
assertTrue(updateNotified)
352367
}
353368
}
@@ -361,10 +376,11 @@ internal class EmbraceSpanImplTest {
361376
assertTrue(addAttribute(key = MAX_LENGTH_ATTRIBUTE_KEY, value = "value"))
362377
assertTrue(addAttribute(key = "key", value = MAX_LENGTH_ATTRIBUTE_VALUE))
363378
assertTrue(addAttribute(key = "Key", value = MAX_LENGTH_ATTRIBUTE_VALUE))
364-
repeat(InstrumentedConfigImpl.otelLimits.getMaxCustomAttributeCount() - 3) {
379+
repeat(dataValidator.otelLimitsConfig.getMaxCustomAttributeCount() - 3) {
365380
assertTrue(addAttribute(key = "key$it", value = "value"))
366381
}
367382
assertFalse(addAttribute(key = "failedKey", value = "value"))
383+
assertEquals(dataValidator.otelLimitsConfig.getMaxCustomAttributeCount() + 1, attributes().size)
368384
assertTrue(updateNotified)
369385
}
370386
}
@@ -385,7 +401,7 @@ internal class EmbraceSpanImplTest {
385401
@Test
386402
fun `check system link limits`() {
387403
assertTrue(embraceSpan.start())
388-
repeat(InstrumentedConfigImpl.otelLimits.getMaxSystemLinkCount()) {
404+
repeat(dataValidator.otelLimitsConfig.getMaxSystemLinkCount()) {
389405
val spanContext = checkNotNull(FakeEmbraceSdkSpan.stopped().spanContext)
390406
assertTrue(embraceSpan.addSystemLink(SpanContextAdapter(spanContext), LinkType.PreviousSession))
391407
}
@@ -398,7 +414,7 @@ internal class EmbraceSpanImplTest {
398414
@Test
399415
fun `check custom link limits`() {
400416
assertTrue(embraceSpan.start())
401-
repeat(InstrumentedConfigImpl.otelLimits.getMaxCustomLinkCount()) {
417+
repeat(dataValidator.otelLimitsConfig.getMaxCustomLinkCount()) {
402418
assertTrue(embraceSpan.addLink(FakeEmbraceSdkSpan.stopped()))
403419
}
404420

0 commit comments

Comments
 (0)