Skip to content

Commit

Permalink
Align http.status to span data convention (#2786)
Browse files Browse the repository at this point in the history
* added SpanDataConvention interface to hold all strings aligned with span data convention
* added `http.response.status_code` to http request spans
  • Loading branch information
stefanosiano committed Jun 16, 2023
1 parent 98fb9c7 commit 496bdfd
Show file tree
Hide file tree
Showing 30 changed files with 171 additions and 47 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,10 @@ val apolloClient = ApolloClient.Builder()
- [changelog](https://github.com/getsentry/sentry-native/blob/master/CHANGELOG.md#063)
- [diff](https://github.com/getsentry/sentry-native/compare/0.6.2...0.6.3)

### Fixes

- Align http.status with [span data conventions](https://develop.sentry.dev/sdk/performance/span-data-conventions/) ([#2786](https://github.com/getsentry/sentry-java/pull/2786))

## 6.22.0

### Features
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import io.sentry.Breadcrumb
import io.sentry.Hint
import io.sentry.IHub
import io.sentry.ISpan
import io.sentry.SpanDataConvention
import io.sentry.SpanStatus
import io.sentry.TypeCheckHint
import io.sentry.android.okhttp.SentryOkHttpEventListener.Companion.CONNECTION_EVENT
Expand Down Expand Up @@ -48,7 +49,7 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val request: Req
callRootSpan?.setData("url", url)
callRootSpan?.setData("host", host)
callRootSpan?.setData("path", encodedPath)
callRootSpan?.setData("http.method", method)
callRootSpan?.setData(SpanDataConvention.HTTP_METHOD_KEY, method)
}

/**
Expand All @@ -60,7 +61,7 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val request: Req
breadcrumb.setData(PROTOCOL_KEY, response.protocol.name)
breadcrumb.setData("status_code", response.code)
callRootSpan?.setData(PROTOCOL_KEY, response.protocol.name)
callRootSpan?.setData("http.status_code", response.code)
callRootSpan?.setData(SpanDataConvention.HTTP_STATUS_CODE_KEY, response.code)
callRootSpan?.status = SpanStatus.fromHttpStatusCode(response.code)
}

Expand All @@ -81,7 +82,7 @@ internal class SentryOkHttpEvent(private val hub: IHub, private val request: Req
fun setResponseBodySize(byteCount: Long) {
if (byteCount > -1) {
breadcrumb.setData("response_content_length", byteCount)
callRootSpan?.setData("http.response_content_length", byteCount)
callRootSpan?.setData(SpanDataConvention.HTTP_RESPONSE_CONTENT_LENGTH_KEY, byteCount)
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package io.sentry.android.okhttp

import io.sentry.HubAdapter
import io.sentry.IHub
import io.sentry.SpanDataConvention
import io.sentry.SpanStatus
import okhttp3.Call
import okhttp3.Connection
Expand Down Expand Up @@ -311,7 +312,7 @@ class SentryOkHttpEventListener(
val okHttpEvent: SentryOkHttpEvent = eventMap[call] ?: return
okHttpEvent.setResponse(response)
okHttpEvent.finishSpan(RESPONSE_HEADERS_EVENT) {
it.setData("status_code", response.code)
it.setData(SpanDataConvention.HTTP_STATUS_CODE_KEY, response.code)
it.status = SpanStatus.fromHttpStatusCode(response.code)
}
}
Expand All @@ -334,7 +335,7 @@ class SentryOkHttpEventListener(
okHttpEvent.setResponseBodySize(byteCount)
okHttpEvent.finishSpan(RESPONSE_BODY_EVENT) {
if (byteCount > 0) {
it.setData("http.response_content_length", byteCount)
it.setData(SpanDataConvention.HTTP_RESPONSE_CONTENT_LENGTH_KEY, byteCount)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@ import io.sentry.IntegrationName
import io.sentry.SentryEvent
import io.sentry.SentryIntegrationPackageStorage
import io.sentry.SentryOptions.DEFAULT_PROPAGATION_TARGETS
import io.sentry.SpanDataConvention
import io.sentry.SpanStatus
import io.sentry.TypeCheckHint.OKHTTP_REQUEST
import io.sentry.TypeCheckHint.OKHTTP_RESPONSE
Expand Down Expand Up @@ -101,6 +102,7 @@ class SentryOkHttpInterceptor(
request = requestBuilder.build()
response = chain.proceed(request)
code = response.code
span?.setData(SpanDataConvention.HTTP_STATUS_CODE_KEY, code)
span?.status = SpanStatus.fromHttpStatusCode(code)

// OkHttp errors (4xx, 5xx) don't throw, so it's safe to call within this block.
Expand Down Expand Up @@ -134,7 +136,7 @@ class SentryOkHttpInterceptor(
val hint = Hint().also { it.set(OKHTTP_REQUEST, request) }
response?.let {
it.body?.contentLength().ifHasValidLength { responseBodySize ->
breadcrumb.setData("http.response_content_length", responseBodySize)
breadcrumb.setData(SpanDataConvention.HTTP_RESPONSE_CONTENT_LENGTH_KEY, responseBodySize)
}

hint[OKHTTP_RESPONSE] = it
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import io.sentry.IHub
import io.sentry.SentryOptions
import io.sentry.SentryTraceHeader
import io.sentry.SentryTracer
import io.sentry.SpanDataConvention
import io.sentry.SpanStatus
import io.sentry.TransactionContext
import okhttp3.Call
Expand Down Expand Up @@ -158,7 +159,7 @@ class SentryOkHttpEventListenerTest {
assertNotNull(span.data["proxies"])
assertNotNull(span.data["domain_name"])
assertNotNull(span.data["dns_addresses"])
assertEquals(201, span.data["status_code"])
assertEquals(201, span.data[SpanDataConvention.HTTP_STATUS_CODE_KEY])
}
1 -> {
assertEquals("http.client.proxy_select", span.operation)
Expand All @@ -180,7 +181,7 @@ class SentryOkHttpEventListenerTest {
}
6 -> {
assertEquals("http.client.response_headers", span.operation)
assertEquals(201, span.data["status_code"])
assertEquals(201, span.data[SpanDataConvention.HTTP_STATUS_CODE_KEY])
}
7 -> {
assertEquals("http.client.response_body", span.operation)
Expand Down Expand Up @@ -223,8 +224,14 @@ class SentryOkHttpEventListenerTest {
response.close()
val requestBodySpan = fixture.sentryTracer.children.firstOrNull { it.operation == "http.client.response_body" }
assertNotNull(requestBodySpan)
assertEquals(responseBytes.size.toLong(), requestBodySpan.data["http.response_content_length"])
assertEquals(responseBytes.size.toLong(), callSpan?.getData("http.response_content_length"))
assertEquals(
responseBytes.size.toLong(),
requestBodySpan.data[SpanDataConvention.HTTP_RESPONSE_CONTENT_LENGTH_KEY]
)
assertEquals(
responseBytes.size.toLong(),
callSpan?.getData(SpanDataConvention.HTTP_RESPONSE_CONTENT_LENGTH_KEY)
)
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@ import io.sentry.ISpan
import io.sentry.SentryOptions
import io.sentry.SentryTracer
import io.sentry.Span
import io.sentry.SpanDataConvention
import io.sentry.SpanOptions
import io.sentry.TracesSamplingDecision
import io.sentry.TransactionContext
Expand Down Expand Up @@ -106,7 +107,7 @@ class SentryOkHttpEventTest {
assertEquals(fixture.mockRequest.url.toString(), callSpan.getData("url"))
assertEquals(fixture.mockRequest.url.host, callSpan.getData("host"))
assertEquals(fixture.mockRequest.url.encodedPath, callSpan.getData("path"))
assertEquals(fixture.mockRequest.method, callSpan.getData("http.method"))
assertEquals(fixture.mockRequest.method, callSpan.getData(SpanDataConvention.HTTP_METHOD_KEY))
}

@Test
Expand Down Expand Up @@ -247,7 +248,7 @@ class SentryOkHttpEventTest {
sut.setResponse(fixture.response)

assertEquals(fixture.response.protocol.name, sut.callRootSpan?.getData("protocol"))
assertEquals(fixture.response.code, sut.callRootSpan?.getData("http.status_code"))
assertEquals(fixture.response.code, sut.callRootSpan?.getData(SpanDataConvention.HTTP_STATUS_CODE_KEY))
sut.finishEvent()

verify(fixture.hub).addBreadcrumb(
Expand Down Expand Up @@ -321,7 +322,7 @@ class SentryOkHttpEventTest {
fun `setResponseBodySize set ResponseBodySize in the breadcrumb and in the root span`() {
val sut = fixture.getSut()
sut.setResponseBodySize(10)
assertEquals(10L, sut.callRootSpan?.getData("http.response_content_length"))
assertEquals(10L, sut.callRootSpan?.getData(SpanDataConvention.HTTP_RESPONSE_CONTENT_LENGTH_KEY))
sut.finishEvent()
verify(fixture.hub).addBreadcrumb(
check<Breadcrumb> {
Expand All @@ -335,7 +336,7 @@ class SentryOkHttpEventTest {
fun `setResponseBodySize is ignored if ResponseBodySize is negative`() {
val sut = fixture.getSut()
sut.setResponseBodySize(-1)
assertNull(sut.callRootSpan?.getData("http.response_content_length"))
assertNull(sut.callRootSpan?.getData(SpanDataConvention.HTTP_RESPONSE_CONTENT_LENGTH_KEY))
sut.finishEvent()
verify(fixture.hub).addBreadcrumb(
check<Breadcrumb> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import io.sentry.IHub
import io.sentry.SentryOptions
import io.sentry.SentryTraceHeader
import io.sentry.SentryTracer
import io.sentry.SpanDataConvention
import io.sentry.SpanStatus
import io.sentry.TransactionContext
import io.sentry.TypeCheckHint
Expand Down Expand Up @@ -226,6 +227,7 @@ class SentryOkHttpInterceptorTest {
val httpClientSpan = fixture.sentryTracer.children.first()
assertEquals("http.client", httpClientSpan.operation)
assertEquals("GET ${request.url}", httpClientSpan.description)
assertEquals(201, httpClientSpan.data[SpanDataConvention.HTTP_STATUS_CODE_KEY])
assertEquals(SpanStatus.OK, httpClientSpan.status)
assertTrue(httpClientSpan.isFinished)
}
Expand All @@ -235,6 +237,7 @@ class SentryOkHttpInterceptorTest {
val sut = fixture.getSut(httpStatusCode = 400)
sut.newCall(getRequest()).execute()
val httpClientSpan = fixture.sentryTracer.children.first()
assertEquals(400, httpClientSpan.data[SpanDataConvention.HTTP_STATUS_CODE_KEY])
assertEquals(SpanStatus.INVALID_ARGUMENT, httpClientSpan.status)
}

Expand All @@ -243,6 +246,7 @@ class SentryOkHttpInterceptorTest {
val sut = fixture.getSut(httpStatusCode = 502)
sut.newCall(getRequest()).execute()
val httpClientSpan = fixture.sentryTracer.children.first()
assertEquals(502, httpClientSpan.data[SpanDataConvention.HTTP_STATUS_CODE_KEY])
assertNull(httpClientSpan.status)
}

Expand All @@ -253,7 +257,7 @@ class SentryOkHttpInterceptorTest {
verify(fixture.hub).addBreadcrumb(
check<Breadcrumb> {
assertEquals("http", it.type)
assertEquals(13L, it.data["http.response_content_length"])
assertEquals(13L, it.data[SpanDataConvention.HTTP_RESPONSE_CONTENT_LENGTH_KEY])
assertEquals(12L, it.data["http.request_content_length"])
},
anyOrNull()
Expand Down Expand Up @@ -296,6 +300,7 @@ class SentryOkHttpInterceptorTest {
// ignore
}
val httpClientSpan = fixture.sentryTracer.children.first()
assertNull(httpClientSpan.data[SpanDataConvention.HTTP_STATUS_CODE_KEY])
assertEquals(SpanStatus.INTERNAL_ERROR, httpClientSpan.status)
assertTrue(httpClientSpan.throwable is IOException)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import io.sentry.HubAdapter
import io.sentry.IHub
import io.sentry.SentryIntegrationPackageStorage
import io.sentry.SentryStackTraceFactory
import io.sentry.SpanDataConvention
import io.sentry.SpanStatus

internal class SQLiteSpanManager(
Expand Down Expand Up @@ -38,9 +39,9 @@ internal class SQLiteSpanManager(
} finally {
span?.apply {
val isMainThread: Boolean = hub.options.mainThreadChecker.isMainThread
setData("blocked_main_thread", isMainThread)
setData(SpanDataConvention.BLOCKED_MAIN_THREAD_KEY, isMainThread)
if (isMainThread) {
setData("call_stack", stackTraceFactory.inAppCallStack)
setData(SpanDataConvention.CALL_STACK_KEY, stackTraceFactory.inAppCallStack)
}
finish()
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import io.sentry.IHub
import io.sentry.SentryIntegrationPackageStorage
import io.sentry.SentryOptions
import io.sentry.SentryTracer
import io.sentry.SpanDataConvention
import io.sentry.SpanStatus
import io.sentry.TransactionContext
import io.sentry.util.thread.IMainThreadChecker
Expand Down Expand Up @@ -100,8 +101,8 @@ class SQLiteSpanManagerTest {
sut.performSql("sql") {}
val span = fixture.sentryTracer.children.first()

assertFalse(span.getData("blocked_main_thread") as Boolean)
assertNull(span.getData("call_stack"))
assertFalse(span.getData(SpanDataConvention.BLOCKED_MAIN_THREAD_KEY) as Boolean)
assertNull(span.getData(SpanDataConvention.CALL_STACK_KEY))
}

@Test
Expand All @@ -114,7 +115,7 @@ class SQLiteSpanManagerTest {
sut.performSql("sql") {}
val span = fixture.sentryTracer.children.first()

assertTrue(span.getData("blocked_main_thread") as Boolean)
assertNotNull(span.getData("call_stack"))
assertTrue(span.getData(SpanDataConvention.BLOCKED_MAIN_THREAD_KEY) as Boolean)
assertNotNull(span.getData(SpanDataConvention.CALL_STACK_KEY))
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import io.sentry.SentryEvent
import io.sentry.SentryIntegrationPackageStorage
import io.sentry.SentryLevel
import io.sentry.SentryOptions.DEFAULT_PROPAGATION_TARGETS
import io.sentry.SpanDataConvention
import io.sentry.SpanStatus
import io.sentry.TypeCheckHint.APOLLO_REQUEST
import io.sentry.TypeCheckHint.APOLLO_RESPONSE
Expand Down Expand Up @@ -107,6 +108,7 @@ class SentryApollo3HttpInterceptor @JvmOverloads constructor(
try {
httpResponse = chain.proceed(modifiedRequest)
statusCode = httpResponse.statusCode
span?.setData(SpanDataConvention.HTTP_STATUS_CODE_KEY, statusCode)
span?.status = SpanStatus.fromHttpStatusCode(statusCode)

captureEvent(modifiedRequest, httpResponse, operationName, operationType)
Expand All @@ -117,6 +119,7 @@ class SentryApollo3HttpInterceptor @JvmOverloads constructor(
when (e) {
is ApolloHttpException -> {
statusCode = e.statusCode
span?.setData(SpanDataConvention.HTTP_STATUS_CODE_KEY, statusCode)
span?.status =
SpanStatus.fromHttpStatusCode(statusCode, SpanStatus.INTERNAL_ERROR)
}
Expand Down Expand Up @@ -209,10 +212,10 @@ class SentryApollo3HttpInterceptor @JvmOverloads constructor(

if (span != null) {
statusCode?.let {
span.setData("http.response.status_code", statusCode)
span.setData(SpanDataConvention.HTTP_STATUS_CODE_KEY, statusCode)
}
responseContentLength?.let {
span.setData("http.response_content_length", it)
span.setData(SpanDataConvention.HTTP_RESPONSE_CONTENT_LENGTH_KEY, it)
}
if (beforeSpan != null) {
try {
Expand Down

0 comments on commit 496bdfd

Please sign in to comment.