Change OkHttp sub-spans to span attributes

[stefanosiano]

📜 Description

SentryOkHttpEvent now set data instead of creating child spans
renamed constants to make them nicer in the UI
SentryOkHttpInterceptor now closes the listener call

Before

After

💡 Motivation and Context

Closes #3520

💚 How did you test it?

📝 Checklist

• I reviewed the submitted code.
• I added tests to verify the changes.
• No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
• I updated the docs if needed.
• Review from the native team if needed.
• No breaking change or entry added to the changelog.
• No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

[github-actions]

	Messages
📖	Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Generated by 🚫 dangerJS against 9ea7cd4

[github-actions]

Performance metrics 🚀

	Plain	With Sentry	Diff
Startup time	398.79 ms	475.02 ms	76.23 ms
Size	1.70 MiB	2.29 MiB	599.31 KiB

Baseline results on branch: 8.x.x

Startup times

Revision	Plain	With Sentry	Diff
a0f7731	357.02 ms	433.40 ms	76.38 ms
41e496a	408.90 ms	491.32 ms	82.42 ms
ca82680	490.88 ms	631.67 ms	140.80 ms
2f49b9c	416.54 ms	479.14 ms	62.60 ms
57732e8	435.66 ms	510.50 ms	74.84 ms
a59fca2	560.08 ms	658.80 ms	98.71 ms

App size

Revision	Plain	With Sentry	Diff
a0f7731	1.70 MiB	2.29 MiB	599.77 KiB
41e496a	1.70 MiB	2.29 MiB	599.77 KiB
ca82680	1.70 MiB	2.29 MiB	599.31 KiB
2f49b9c	1.70 MiB	2.29 MiB	599.31 KiB
57732e8	1.70 MiB	2.29 MiB	599.77 KiB
a59fca2	1.70 MiB	2.29 MiB	599.77 KiB

[gggritso]

This is awesome thank you! Really great for how Insights shows this data, too! Would be great to also align to how the JavaScript SDK records the connection timings:

"http.request.connect_start": 1720444446.0359,
"http.request.connection_end": 1720444446.0359,
"http.request.domain_lookup_end": 1720444446.0359,
"http.request.domain_lookup_start": 1720444446.0359,
"http.request.fetch_start": 1720444446.0359,
"http.request.redirect_start": 1720444445.188,
"http.request.request_start": 1720444446.0493,
"http.request.response_start": 1720444446.0962,
"http.request.response_end": 1720444446.0963,
"http.request.secure_connection_start": 1720444446.0359,

That way, these measurements will show up with a nicer UI in the trace view. Unsure if this is standardized somewhere, but would love to all SDKs to report these measurements the same way.

[kahest]

This is awesome thank you! Really great for how Insights shows this data, too! Would be great to also align to how the JavaScript SDK records the connection timings:
[...]
That way, these measurements will show up with a nicer UI in the trace view. Unsure if this is standardized somewhere, but would love to all SDKs to report these measurements the same way.

Thanks for bringing this up, definitely good to align! We landed on the key names you see in this PR because that we we kinda reproduce the nested spans we had before (e.g. http.connection contains child spans http.connection.request_header, http.connection.response_body, etc.). Do you think this makes sense or should we aim to align with JS fully?

[gggritso]

@kahest 🤔 I think this decision is best left to the SDK folks! The important thing to the UI is that the measurements have the same names in every SDK that supports them. If you align to JS that makes the UI work easy (since it's already done). If you decide to not align to JS, you'll need to let us know what the names will be, and we can adjust the UI code, and then update the JS SDK output to match?

For what it's worth, right now the UI specifically treats these two measurements as special: "http.request.request_start" and "http.request.response_start". If they are present, it shows nice little duration lines in the span data section in the trace view:

We don't do anything special with the other measurements, but we might in the future! e.g., there was some talk of adding more metrics to the Requests Insights module to track connection times.

[stefanosiano]

@gggritso from what i understand http.request.request_start and http.request.response_start are not durations, but timestamps, right?

[gggritso]

@stefanosiano that's correct!