Record logged network requests as completed spans #748
Conversation
| endTime, | ||
| networkCaptureData, | ||
| errorMessage | ||
| networkRequest.url, //TODO: This used the non-stripped URL, is that correct? |
There was a problem hiding this comment.
Yeah, we want to log the entire URL when capturing the entire request/response in case there are parameters are that important. We aren't aggregating by this so we don't need to reduce the cardinality by tweaking the URL
| } ?: return | ||
| val domain = getDomain( | ||
| stripUrl(networkRequest.url) | ||
| ) ?: return | ||
|
|
||
| synchronized(callsStorageLastUpdate) { |
There was a problem hiding this comment.
we can remove this synchronization and only block if we are counting against the same domain
We can change this in a separate PR though - this locking has always sucked and now we can remove it
| callsStorageLastUpdate.incrementAndGet() | ||
| sessionNetworkCalls[callId] = networkCall | ||
|
|
||
| //TODO: Why do we want to strip the URL? |
There was a problem hiding this comment.
We strip because we want to aggregate all the calls to this base URL without parameters and hash section links (or whenever they are called). I guess the idea is that foo.com/#first and foo.com/#second is close enough that they should be looked at together.
| return null | ||
| } | ||
|
|
||
| fun stripUrl(url: String): String { |
There was a problem hiding this comment.
Can move this. method to the logging service? It's only called there and in the test, and the test could just validate the URL is stripped without using that method to get the expected match?
| @@ -140,7 +140,7 @@ internal class NetworkUtilsTest { | |||
|
|
|||
| @Test | |||
| fun stripUrl() { | |||
priettt
force-pushed
the
request-schema-type
branch
from
9be830b
to
97196f8
Compare
priettt
force-pushed
the
record-network-span
branch
from
5bdad96
to
21e7adf
Compare
Merge activity
|
bidetofevil
force-pushed
the
request-schema-type
branch
from
97196f8
to
c0bc028
Compare
bidetofevil
force-pushed
the
record-network-span
branch
from
21e7adf
to
31d4443
Compare
Record network requests as spans.
Consolidate logNetworkCall and logNetworkError methods
EmbraceNetworkLoggingService had two methods: logNetworkCall and logNetworkError. They were both doing essentially the same, but with a different NetworkCallV2 object: strip the URL, sanitize the TraceId, logging the networkCapturedData, and processing the network call.
We don't need a NetworkCallV2 object anymore, and the EmbraceNetworkRequest is the same for both network call and network error, so we can just use one method.
Questions: