RUM-648 Add request id in okhttp request#2126
Conversation
| ) | ||
| fun startResource( | ||
| key: String, | ||
| key: Any, |
There was a problem hiding this comment.
I wonder why we need to keep Any type here? can we just use the new class ResourceId instead?
| DEPRECATED fun startResource(Any, String, String, Map<String, Any?> = emptyMap()) | ||
| fun startResource(Any, RumResourceMethod, String, Map<String, Any?> = emptyMap()) | ||
| fun stopResource(Any, Int?, Long?, RumResourceKind, Map<String, Any?>) | ||
| fun stopResourceWithError(Any, Int?, String, RumErrorSource, Throwable, Map<String, Any?> = emptyMap()) | ||
| fun stopResourceWithError(Any, Int?, String, RumErrorSource, String, String?, Map<String, Any?> = emptyMap()) |
There was a problem hiding this comment.
just an idea: while the key for resources is something which is used only as a key, and not some information baggage, making it Any in public API may have some consequences: what if we will have a raise in the number of duplicated keys or a possibility of some memory leak (e.g. some network call instance is passed as a key or Request object, which later maybe be transformed to another Request object by the interceptor chain and in the result request != response.request, leading to hanging resources).
Maybe we can instead add methods like startResource(key: ResourceId, ....), etc. to the AdvancedNetworkRumMonitor and use it as a private API for a while? wdyt?
There was a problem hiding this comment.
That makes a lot of sense indeed, I'll do that.
f95e1a4 to
3ba2e49
Compare
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #2126 +/- ##
===========================================
+ Coverage 70.03% 70.12% +0.09%
===========================================
Files 722 725 +3
Lines 26821 26877 +56
Branches 4503 4510 +7
===========================================
+ Hits 18784 18847 +63
+ Misses 6780 6765 -15
- Partials 1257 1265 +8
|
What does this PR do?
This PR adds a better way to track OkHttp requests as resources avoiding key collisions. Our keys used to only take into account the url, now we generate a UUID attached to the request to have a more accurate resource representation.