Skip to content

Commit

Permalink
Don't enforce span name length limit for spans the SDK logs
Browse files Browse the repository at this point in the history
  • Loading branch information
bidetofevil committed Sep 9, 2024
1 parent e854529 commit b48775a
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 13 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -238,7 +238,7 @@ internal class EmbraceSpanImpl(
}

override fun updateName(newName: String): Boolean {
if (newName.isValidName()) {
if (newName.isValidName(spanBuilder.internal)) {
synchronized(startedSpan) {
if (!spanStarted() || isRecording) {
updatedName = newName
Expand Down Expand Up @@ -333,6 +333,6 @@ internal class EmbraceSpanImpl(
internal fun attributeValid(key: String, value: String) =
key.length <= MAX_CUSTOM_ATTRIBUTE_KEY_LENGTH && value.length <= MAX_CUSTOM_ATTRIBUTE_VALUE_LENGTH

internal fun String.isValidName(): Boolean = isNotBlank() && (length <= MAX_NAME_LENGTH)
internal fun String.isValidName(internal: Boolean): Boolean = isNotBlank() && (internal || length <= MAX_NAME_LENGTH)
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ internal class SpanServiceImpl(
internal: Boolean,
private: Boolean
): PersistableEmbraceSpan? {
return if (inputsValid(name) && currentSessionSpan.canStartNewSpan(parent, internal)) {
return if (inputsValid(name, internal) && currentSessionSpan.canStartNewSpan(parent, internal)) {
embraceSpanFactory.create(
name = name,
type = type,
Expand All @@ -49,7 +49,7 @@ internal class SpanServiceImpl(

override fun createSpan(embraceSpanBuilder: EmbraceSpanBuilder): PersistableEmbraceSpan? {
return if (
inputsValid(embraceSpanBuilder.spanName) &&
inputsValid(embraceSpanBuilder.spanName, embraceSpanBuilder.internal) &&
currentSessionSpan.canStartNewSpan(embraceSpanBuilder.getParentSpan(), embraceSpanBuilder.internal)
) {
embraceSpanFactory.create(embraceSpanBuilder)
Expand Down Expand Up @@ -110,7 +110,7 @@ internal class SpanServiceImpl(
return false
}

if (inputsValid(name, events, attributes) && currentSessionSpan.canStartNewSpan(parent, internal)) {
if (inputsValid(name, internal, events, attributes) && currentSessionSpan.canStartNewSpan(parent, internal)) {
val newSpan = embraceSpanFactory.create(name = name, type = type, internal = internal, private = private, parent = parent)
if (newSpan.start(startTimeMs)) {
attributes.forEach {
Expand All @@ -130,10 +130,11 @@ internal class SpanServiceImpl(

private fun inputsValid(
name: String,
internal: Boolean,
events: List<EmbraceSpanEvent>? = null,
attributes: Map<String, String>? = null
): Boolean {
return name.isValidName() &&
return (name.isValidName(internal)) &&
((events == null) || (events.size <= EmbraceSpanLimits.MAX_CUSTOM_EVENT_COUNT)) &&
((attributes == null) || (attributes.size <= EmbraceSpanLimits.MAX_CUSTOM_ATTRIBUTE_COUNT))
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -473,14 +473,14 @@ internal class SpanServiceImplTest {
}

@Test
fun `check name length limit`() {
assertNull(spansService.createSpan(name = TOO_LONG_SPAN_NAME))
assertFalse(spansService.recordCompletedSpan(name = TOO_LONG_SPAN_NAME, startTimeMs = 100L, endTimeMs = 200L))
assertNotNull(spansService.recordSpan(name = TOO_LONG_SPAN_NAME) { 1 })
fun `check name length limit for non-internal spans`() {
assertNull(spansService.createSpan(name = TOO_LONG_SPAN_NAME, internal = false))
assertFalse(spansService.recordCompletedSpan(name = TOO_LONG_SPAN_NAME, startTimeMs = 100L, endTimeMs = 200L, internal = false))
assertNotNull(spansService.recordSpan(name = TOO_LONG_SPAN_NAME, internal = false) { 1 })
assertEquals(0, spanSink.completedSpans().size)
assertNotNull(spansService.createSpan(name = MAX_LENGTH_SPAN_NAME))
assertNotNull(spansService.recordSpan(name = MAX_LENGTH_SPAN_NAME) { 2 })
assertTrue(spansService.recordCompletedSpan(name = MAX_LENGTH_SPAN_NAME, startTimeMs = 100L, endTimeMs = 200L))
assertNotNull(spansService.createSpan(name = MAX_LENGTH_SPAN_NAME, internal = false))
assertNotNull(spansService.recordSpan(name = MAX_LENGTH_SPAN_NAME, internal = false) { 2 })
assertTrue(spansService.recordCompletedSpan(name = MAX_LENGTH_SPAN_NAME, startTimeMs = 100L, endTimeMs = 200L, internal = false))
assertEquals(2, spanSink.completedSpans().size)
}

Expand Down

0 comments on commit b48775a

Please sign in to comment.