consider status codes below 400 to be OK

adinauer · adinauer · commit db775cffd586 · 2024-11-11T09:12:40.000+01:00
diff --git a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTracingFilterTest.kt b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/tracing/SentryTracingFilterTest.kt
@@ -126,7 +126,7 @@ class SentryTracingFilterTest {
 
     @Test
     fun `does not set span status for response status that dont match predefined span statuses`() {
-        val filter = fixture.getSut(status = 302)
+        val filter = fixture.getSut(status = 507)
 
         filter.doFilter(fixture.request, fixture.response, fixture.chain)
 
diff --git a/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebFluxTracingFilterTest.kt b/sentry-spring-jakarta/src/test/kotlin/io/sentry/spring/jakarta/webflux/SentryWebFluxTracingFilterTest.kt
@@ -146,7 +146,7 @@ class SentryWebFluxTracingFilterTest {
 
     @Test
     fun `does not set span status for response status that dont match predefined span statuses`() {
-        val filter = fixture.getSut(status = HttpStatus.FOUND)
+        val filter = fixture.getSut(status = HttpStatus.INSUFFICIENT_STORAGE)
 
         withMockScopes {
             filter.filter(fixture.exchange, fixture.chain).block()
diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/tracing/SentryTracingFilterTest.kt
@@ -126,7 +126,7 @@ class SentryTracingFilterTest {
 
     @Test
     fun `does not set span status for response status that dont match predefined span statuses`() {
-        val filter = fixture.getSut(status = 302)
+        val filter = fixture.getSut(status = 507)
 
         filter.doFilter(fixture.request, fixture.response, fixture.chain)
 
diff --git a/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebFluxTracingFilterTest.kt b/sentry-spring/src/test/kotlin/io/sentry/spring/webflux/SentryWebFluxTracingFilterTest.kt
@@ -147,7 +147,7 @@ class SentryWebFluxTracingFilterTest {
 
     @Test
     fun `does not set span status for response status that dont match predefined span statuses`() {
-        val filter = fixture.getSut(status = HttpStatus.FOUND)
+        val filter = fixture.getSut(status = HttpStatus.INSUFFICIENT_STORAGE)
 
         withMockScopes {
             filter.filter(fixture.exchange, fixture.chain).block()
diff --git a/sentry/src/main/java/io/sentry/SpanStatus.java b/sentry/src/main/java/io/sentry/SpanStatus.java
@@ -7,7 +7,7 @@
 
 public enum SpanStatus implements JsonSerializable {
   /** Not an error, returned on success. */
-  OK(200, 299),
+  OK(0, 399),
   /** The operation was cancelled, typically by the caller. */
   CANCELLED(499),
   /**
diff --git a/sentry/src/test/java/io/sentry/SpanStatusTest.kt b/sentry/src/test/java/io/sentry/SpanStatusTest.kt
@@ -21,14 +21,19 @@ class SpanStatusTest {
         assertEquals(SpanStatus.INTERNAL_ERROR, SpanStatus.fromHttpStatusCode(500))
     }
 
+    @Test
+    fun `code 3xx is now also considered OK`() {
+        assertEquals(SpanStatus.OK, SpanStatus.fromHttpStatusCode(304))
+    }
+
     @Test
     fun `returns null when no SpanStatus matches specific code`() {
-        assertNull(SpanStatus.fromHttpStatusCode(302))
+        assertNull(SpanStatus.fromHttpStatusCode(599))
     }
 
     @Test
     fun `returns default value when no SpanStatus matches specific code`() {
-        assertEquals(SpanStatus.UNKNOWN_ERROR, SpanStatus.fromHttpStatusCode(302, SpanStatus.UNKNOWN_ERROR))
+        assertEquals(SpanStatus.UNKNOWN_ERROR, SpanStatus.fromHttpStatusCode(599, SpanStatus.UNKNOWN_ERROR))
     }
 
     @Test

Original file line number	Diff line number	Diff line change
`@@ -21,14 +21,19 @@ class SpanStatusTest {`
`21`	`21`	`assertEquals(SpanStatus.INTERNAL_ERROR, SpanStatus.fromHttpStatusCode(500))`
`22`	`22`	`}`
`23`	`23`
	`24`	`+ @Test`
	`25`	+ fun `code 3xx is now also considered OK`() {
	`26`	`+ assertEquals(SpanStatus.OK, SpanStatus.fromHttpStatusCode(304))`
	`27`	`+ }`
	`28`	`+`
`24`	`29`	`@Test`
`25`	`30`	fun `returns null when no SpanStatus matches specific code`() {
`26`		`- assertNull(SpanStatus.fromHttpStatusCode(302))`
	`31`	`+ assertNull(SpanStatus.fromHttpStatusCode(599))`
`27`	`32`	`}`
`28`	`33`
`29`	`34`	`@Test`
`30`	`35`	fun `returns default value when no SpanStatus matches specific code`() {
`31`		`- assertEquals(SpanStatus.UNKNOWN_ERROR, SpanStatus.fromHttpStatusCode(302, SpanStatus.UNKNOWN_ERROR))`
	`36`	`+ assertEquals(SpanStatus.UNKNOWN_ERROR, SpanStatus.fromHttpStatusCode(599, SpanStatus.UNKNOWN_ERROR))`
`32`	`37`	`}`
`33`	`38`
`34`	`39`	`@Test`