Add http fields to `span.data` #1497

denrase · 2023-05-30T09:51:14Z

📜 Description

Set http.response.status_code to span.data
Set http.response_content_lengthto span.data

💡 Motivation and Context

Closes #1473

💚 How did you test it?

Updated test expectations

📝 Checklist

I reviewed submitted code
I added tests to verify changes
No new PII added or SDK only sends newly added PII if sendDefaultPii is enabled
I updated the docs if needed
All tests passing
No breaking changes

github-actions · 2023-05-30T09:55:48Z

Android Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	312.81 ms	377.45 ms	64.64 ms
Size	6.16 MiB	7.13 MiB	1000.83 KiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`8cb6557`	321.20 ms	370.46 ms	49.26 ms
`e893df5`	310.60 ms	380.58 ms	69.98 ms
`af2d175`	279.08 ms	312.37 ms	33.29 ms
`a1a1545`	295.21 ms	351.16 ms	55.95 ms
`ef31c7f`	311.39 ms	359.33 ms	47.94 ms
`8a7f528`	290.27 ms	341.80 ms	51.53 ms
`559d28f`	302.35 ms	339.53 ms	37.18 ms
`6d7a391`	331.94 ms	367.04 ms	35.10 ms
`2d3b03d`	309.53 ms	353.40 ms	43.87 ms
`fcd1ee4`	298.96 ms	376.04 ms	77.09 ms

App size

Revision	Plain	With Sentry	Diff
`8cb6557`	6.06 MiB	7.03 MiB	997.01 KiB
`e893df5`	6.06 MiB	7.09 MiB	1.03 MiB
`af2d175`	5.94 MiB	6.92 MiB	1001.83 KiB
`a1a1545`	5.94 MiB	6.96 MiB	1.02 MiB
`ef31c7f`	6.06 MiB	7.09 MiB	1.03 MiB
`8a7f528`	6.06 MiB	7.03 MiB	989.36 KiB
`559d28f`	5.94 MiB	6.92 MiB	1001.70 KiB
`6d7a391`	5.94 MiB	6.95 MiB	1.01 MiB
`2d3b03d`	6.06 MiB	7.09 MiB	1.03 MiB
`fcd1ee4`	6.16 MiB	7.13 MiB	1000.85 KiB

Previous results on branch: enha/span-data-http-status-code

Startup times

Revision	Plain	With Sentry	Diff
`7d62110`	303.92 ms	359.98 ms	56.06 ms
`bf8d7c8`	298.63 ms	361.86 ms	63.23 ms
`a66adfa`	304.40 ms	385.78 ms	81.38 ms

App size

Revision	Plain	With Sentry	Diff
`7d62110`	6.16 MiB	7.13 MiB	1000.80 KiB
`bf8d7c8`	6.16 MiB	7.13 MiB	1000.85 KiB
`a66adfa`	6.16 MiB	7.13 MiB	1000.85 KiB

codecov · 2023-05-30T09:57:22Z

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.14 🎉

Comparison is base (8932ece) 90.73% compared to head (9098d61) 90.88%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1497      +/-   ##
==========================================
+ Coverage   90.73%   90.88%   +0.14%     
==========================================
  Files          61       61              
  Lines        2030     2030              
==========================================
+ Hits         1842     1845       +3     
+ Misses        188      185       -3

Impacted Files	Coverage Δ
dio/lib/src/breadcrumb_client_adapter.dart	`95.00% <100.00%> (+12.39%)`	⬆️
dio/lib/src/tracing_client_adapter.dart	`89.28% <100.00%> (+1.28%)`	⬆️

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

github-actions · 2023-05-30T10:17:50Z

iOS Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	1251.82 ms	1257.61 ms	5.80 ms
Size	8.29 MiB	9.36 MiB	1.07 MiB

Baseline results on branch: main

Startup times

Revision	Plain	With Sentry	Diff
`69670c9`	1269.18 ms	1288.69 ms	19.51 ms
`f96ca24`	1264.39 ms	1287.98 ms	23.59 ms
`3a69405`	1292.84 ms	1303.96 ms	11.12 ms
`dd1f7d2`	1263.16 ms	1275.15 ms	11.99 ms
`8cb6557`	1265.14 ms	1266.08 ms	0.94 ms
`5aab4c5`	1247.08 ms	1271.17 ms	24.09 ms
`457a85b`	1275.17 ms	1285.51 ms	10.34 ms
`b49bf00`	1248.00 ms	1260.35 ms	12.35 ms
`613760b`	1263.10 ms	1277.27 ms	14.16 ms
`e2d89fc`	1251.80 ms	1261.92 ms	10.11 ms

App size

Revision	Plain	With Sentry	Diff
`69670c9`	8.10 MiB	9.08 MiB	1004.49 KiB
`f96ca24`	8.10 MiB	9.08 MiB	1004.36 KiB
`3a69405`	8.15 MiB	9.15 MiB	1018.56 KiB
`dd1f7d2`	8.10 MiB	9.16 MiB	1.07 MiB
`8cb6557`	8.10 MiB	9.18 MiB	1.08 MiB
`5aab4c5`	8.10 MiB	9.16 MiB	1.07 MiB
`457a85b`	8.09 MiB	9.07 MiB	1000.88 KiB
`b49bf00`	8.10 MiB	9.08 MiB	1004.36 KiB
`613760b`	8.15 MiB	9.13 MiB	1000.46 KiB
`e2d89fc`	8.10 MiB	9.16 MiB	1.06 MiB

Previous results on branch: enha/span-data-http-status-code

Startup times

Revision	Plain	With Sentry	Diff
`a66adfa`	1232.84 ms	1247.12 ms	14.29 ms
`bf8d7c8`	1250.12 ms	1258.76 ms	8.63 ms

App size

Revision	Plain	With Sentry	Diff
`a66adfa`	8.29 MiB	9.36 MiB	1.07 MiB
`bf8d7c8`	8.29 MiB	9.36 MiB	1.07 MiB

dart/lib/src/protocol/breadcrumb.dart

marandaneto · 2023-05-30T11:05:10Z

@denrase missing for dio TracingClientAdapter

marandaneto · 2023-05-30T11:07:28Z

@denrase check if there's any missing from this list as well.
Probably http.response_content_length, http.decoded_response_body_length and http.response_transfer_size, if possible.

denrase · 2023-05-30T12:48:30Z

Breadcrumbs sentry-java

Breadcrumbs set the following data values throughout the codebase:

http.request_content_length
http.response_content_length
request_content_length
response_content_length
request_body_size
response_body_size

Those are not documented as far as I could tell:

https://develop.sentry.dev/sdk/event-payloads/breadcrumbs/

On sentry-dart we only set request_body_size and response_body_size for breadcrumbs constructed using the http factory constuctor. Are others also relevant for us? See the sentry-java code occurences for (later) reference.

Code (sentry-java)

SentryOkHttpInterceptor

request.body?.contentLength().ifHasValidLength {
    breadcrumb.setData("http.request_content_length", it)
}

https://github.com/getsentry/sentry-java/blob/b2152fd7774dc39bd2b22b0943387e7209f4eb69/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt#L130

it.body?.contentLength().ifHasValidLength { responseBodySize ->
    breadcrumb.setData("http.response_content_length", responseBodySize)
}

https://github.com/getsentry/sentry-java/blob/b2152fd7774dc39bd2b22b0943387e7209f4eb69/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpInterceptor.kt#L136

SentryOkHttpEvent

fun setRequestBodySize(byteCount: Long) {
    if (byteCount > -1) {
        breadcrumb.setData("request_content_length", byteCount)
        callRootSpan?.setData("http.request_content_length", byteCount)
    }
}

https://github.com/getsentry/sentry-java/blob/b2152fd7774dc39bd2b22b0943387e7209f4eb69/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt#L76

fun setResponseBodySize(byteCount: Long) {
    if (byteCount > -1) {
        breadcrumb.setData("response_content_length", byteCount)
        callRootSpan?.setData("http.response_content_length", byteCount)
    }
}

https://github.com/getsentry/sentry-java/blob/b2152fd7774dc39bd2b22b0943387e7209f4eb69/sentry-android-okhttp/src/main/java/io/sentry/android/okhttp/SentryOkHttpEvent.kt#L83

SentryApollo3HttpInterceptor

request.body?.contentLength.ifHasValidLength { contentLength ->
	breadcrumb.setData("request_body_size", contentLength)
}

httpResponse.headersContentLength().ifHasValidLength { contentLength ->
    breadcrumb.setData("response_body_size", contentLength)
}

SentryApolloInterceptor

httpRequest.body()?.contentLength().ifHasValidLength { contentLength ->
    breadcrumb.setData("request_body_size", contentLength)
}

httpResponse.body()?.contentLength().ifHasValidLength { contentLength ->
    breadcrumb.setData("response_body_size", contentLength)
}

CHANGELOG.md

dart/lib/src/utils/http_header_utils.dart

marandaneto · 2023-05-30T13:02:11Z

@denrase the issue is only about span.data, not about breadcrumb.data, we can ignore the crumbs for now, they are a different spec.

denrase · 2023-05-30T13:06:44Z

@marandaneto Cool. The we can see this as reference if we do need to change/add something there and we end up needing to do similar things as sentry-java.

denrase added 2 commits May 30, 2023 11:43

Set data http.response.status_code instead of status_code

b3c67ca

add changelog entry

219edbe

denrase added 2 commits May 30, 2023 11:58

Merge branch 'main' into enha/span-data-http-status-code

b33a2c6

update changelog

a66adfa

denrase marked this pull request as ready for review May 30, 2023 09:58

denrase requested review from marandaneto, krystofwoldrich and stefanosiano as code owners May 30, 2023 09:58

marandaneto reviewed May 30, 2023

View reviewed changes

dart/lib/src/protocol/breadcrumb.dart Outdated Show resolved Hide resolved

denrase added 5 commits May 30, 2023 13:26

revert breadcrumb changes

19ca600

remove trailing comma

bf8d7c8

set http.response.status_code in dio tracingclientadapter

d32769e

set http.response_content_length

f2d71d6

format

040acd4

resolve issues

9312dd6

marandaneto reviewed May 30, 2023

View reviewed changes

CHANGELOG.md Outdated Show resolved Hide resolved

add tests for helper

7d62110

marandaneto reviewed May 30, 2023

View reviewed changes

dart/lib/src/utils/http_header_utils.dart Show resolved Hide resolved

denrase changed the title ~~Set http.response.status_code to span.data~~ Add http fields to span.data May 30, 2023

update changelog entry

879d5f6

denrase requested a review from marandaneto May 30, 2023 13:07

marandaneto approved these changes May 30, 2023

View reviewed changes

Merge branch 'main' into enha/span-data-http-status-code

9098d61

marandaneto enabled auto-merge (squash) May 31, 2023 06:57

marandaneto merged commit 5d2b46d into main May 31, 2023
85 of 87 checks passed

marandaneto deleted the enha/span-data-http-status-code branch May 31, 2023 07:14

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add http fields to `span.data` #1497

Add http fields to `span.data` #1497

denrase commented May 30, 2023 •

edited

github-actions bot commented May 30, 2023 •

edited

Baseline results on branch: main

Startup times

App size

Previous results on branch: enha/span-data-http-status-code

Startup times

App size

codecov bot commented May 30, 2023 •

edited

github-actions bot commented May 30, 2023 •

edited

Baseline results on branch: main

Startup times

App size

Previous results on branch: enha/span-data-http-status-code

Startup times

App size

marandaneto commented May 30, 2023

marandaneto commented May 30, 2023

denrase commented May 30, 2023

marandaneto commented May 30, 2023

denrase commented May 30, 2023

Add http fields to span.data #1497

Add http fields to span.data #1497

Conversation

denrase commented May 30, 2023 • edited

📜 Description

💡 Motivation and Context

💚 How did you test it?

📝 Checklist

github-actions bot commented May 30, 2023 • edited

Android Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: enha/span-data-http-status-code

Startup times

App size

codecov bot commented May 30, 2023 • edited

Codecov Report

github-actions bot commented May 30, 2023 • edited

iOS Performance metrics 🚀

Baseline results on branch: main

Startup times

App size

Previous results on branch: enha/span-data-http-status-code

Startup times

App size

marandaneto commented May 30, 2023

marandaneto commented May 30, 2023

denrase commented May 30, 2023

Breadcrumbs sentry-java

Code (sentry-java)

SentryOkHttpInterceptor

SentryOkHttpEvent

SentryApollo3HttpInterceptor

SentryApolloInterceptor

marandaneto commented May 30, 2023

denrase commented May 30, 2023

Add http fields to `span.data` #1497

Add http fields to `span.data` #1497

denrase commented May 30, 2023 •

edited

github-actions bot commented May 30, 2023 •

edited

codecov bot commented May 30, 2023 •

edited

github-actions bot commented May 30, 2023 •

edited