Skip to content

Commit

Permalink
Initial prep for sending network requests as spans (#745)
Browse files Browse the repository at this point in the history
## Goal

This is the initial PR for sending network requests as spans. 

- Removed recordAndDeduplicateNetworkRequest. 

This was used in HttpURLConnectionDelegate to avoid logging network requests with the same call ID. It is still required, but we will address it differently in the following PRs. For now, we will use the same method as the rest of the requests.

- logNetworkError and logNetworkCall did essentially the same. We will move that logic to the logging service so we don't bloat EmbraceImpl

- getNetworkCallsSnapshot won't be used anymore, as we will record spans directly when they get to the logging service. We won't need to expose them when the session ends.

- Stop looking for network logs in PerformanceInfo. Removed "nr" from session-end.json. We won't be sending network requests there anymore.

- Updated tests to use the new APIs
  • Loading branch information
priettt committed Apr 18, 2024
1 parent 9312e15 commit 2c3bd85
Show file tree
Hide file tree
Showing 16 changed files with 40 additions and 206 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -19,12 +19,6 @@
},
"d": "__EMBRACE_TEST_IGNORE__",
"p": {
"nr": {
"v2": {
"c": {},
"r": []
}
},
"ds": {
"as": "__EMBRACE_TEST_IGNORE__",
"fs": "__EMBRACE_TEST_IGNORE__"
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -89,8 +89,7 @@ internal class EmbraceInternalInterfaceTest {
networkCaptureData = null
)

recordAndDeduplicateNetworkRequest(
callId = "",
recordNetworkRequest(
embraceNetworkRequest = EmbraceNetworkRequest.fromCompletedRequest(
"",
HttpMethod.GET,
Expand Down Expand Up @@ -193,8 +192,7 @@ internal class EmbraceInternalInterfaceTest {
networkCaptureData = null
)

embrace.internalInterface.recordAndDeduplicateNetworkRequest(
callId = "",
embrace.internalInterface.recordNetworkRequest(
embraceNetworkRequest = EmbraceNetworkRequest.fromCompletedRequest(
URL,
HttpMethod.POST,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -213,8 +213,7 @@ internal class NetworkRequestApiTest {
harness.overriddenClock.tick(5)

val callId = UUID.randomUUID().toString()
embrace.internalInterface.recordAndDeduplicateNetworkRequest(
callId,
embrace.internalInterface.recordNetworkRequest(
EmbraceNetworkRequest.fromCompletedRequest(
"$URL/bad",
HttpMethod.GET,
Expand All @@ -225,8 +224,7 @@ internal class NetworkRequestApiTest {
200
)
)
embrace.internalInterface.recordAndDeduplicateNetworkRequest(
callId,
embrace.internalInterface.recordNetworkRequest(
EmbraceNetworkRequest.fromCompletedRequest(
URL,
HttpMethod.GET,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@

import java.util.HashMap;
import java.util.Map;
import java.util.UUID;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.regex.Pattern;

Expand Down Expand Up @@ -45,7 +44,6 @@
import io.embrace.android.embracesdk.internal.Systrace;
import io.embrace.android.embracesdk.internal.clock.Clock;
import io.embrace.android.embracesdk.internal.crash.LastRunCrashVerifier;
import io.embrace.android.embracesdk.internal.network.http.NetworkCaptureData;
import io.embrace.android.embracesdk.internal.spans.EmbraceTracer;
import io.embrace.android.embracesdk.internal.utils.ThrowableUtilsKt;
import io.embrace.android.embracesdk.logging.InternalEmbraceLogger;
Expand Down Expand Up @@ -717,74 +715,14 @@ String generateW3cTraceparent() {
}

void recordNetworkRequest(@NonNull EmbraceNetworkRequest request) {
if (embraceInternalInterface != null && checkSdkStartedAndLogPublicApiUsage("record_network_request")) {
embraceInternalInterface.recordAndDeduplicateNetworkRequest(UUID.randomUUID().toString(), request);
}
}

void recordAndDeduplicateNetworkRequest(@NonNull String callId, @NonNull EmbraceNetworkRequest request) {
if (checkSdkStartedAndLogPublicApiUsage("record_network_request")) {
logNetworkRequestImpl(
callId,
request.getNetworkCaptureData(),
request.getUrl(),
request.getHttpMethod(),
request.getStartTime(),
request.getResponseCode(),
request.getEndTime(),
request.getErrorType(),
request.getErrorMessage(),
request.getTraceId(),
request.getW3cTraceparent(),
request.getBytesOut(),
request.getBytesIn()
);
logNetworkRequest(request);
}
}

private void logNetworkRequestImpl(@NonNull String callId,
@Nullable NetworkCaptureData networkCaptureData,
String url,
String httpMethod,
Long startTime,
Integer responseCode,
Long endTime,
String errorType,
String errorMessage,
String traceId,
@Nullable String w3cTraceparent,
Long bytesOut,
Long bytesIn) {
if (configService.getNetworkBehavior().isUrlEnabled(url)) {
if (errorType != null &&
errorMessage != null &&
!errorType.isEmpty() &&
!errorMessage.isEmpty()) {
networkLoggingService.logNetworkError(
callId,
url,
httpMethod,
startTime,
endTime != null ? endTime : 0,
errorType,
errorMessage,
traceId,
w3cTraceparent,
networkCaptureData);
} else {
networkLoggingService.logNetworkCall(
callId,
url,
httpMethod,
responseCode != null ? responseCode : 0,
startTime,
endTime != null ? endTime : 0,
bytesOut,
bytesIn,
traceId,
w3cTraceparent,
networkCaptureData);
}
private void logNetworkRequest(@NonNull EmbraceNetworkRequest request) {
if (configService.getNetworkBehavior().isUrlEnabled(request.getUrl())) {
networkLoggingService.logNetworkRequest(request);
onActivityReported();
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -167,11 +167,8 @@ internal class EmbraceInternalInterfaceImpl(
)
}

override fun recordAndDeduplicateNetworkRequest(
callId: String,
embraceNetworkRequest: EmbraceNetworkRequest
) {
embraceImpl.recordAndDeduplicateNetworkRequest(callId, embraceNetworkRequest)
override fun recordNetworkRequest(embraceNetworkRequest: EmbraceNetworkRequest) {
embraceImpl.recordNetworkRequest(embraceNetworkRequest)
}

override fun shouldCaptureNetworkBody(url: String, method: String): Boolean = embraceImpl.shouldCaptureNetworkCall(url, method)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ internal class UninitializedSdkInternalInterfaceImpl(
) {
}

override fun recordAndDeduplicateNetworkRequest(callId: String, embraceNetworkRequest: EmbraceNetworkRequest) {}
override fun recordNetworkRequest(embraceNetworkRequest: EmbraceNetworkRequest) {}

override fun logComposeTap(point: Pair<Float, Float>, elementName: String) {}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ internal class EmbracePerformanceInfoService(
val info = getPerformanceInfo(sessionStart, sessionLastKnownTime, coldStart)

return info.copy(
networkRequests = captureDataSafely(logger) { NetworkRequests(networkLoggingService.getNetworkCallsSnapshot()) },
googleAnrTimestamps = captureDataSafely(logger) {
googleAnrTimestampRepository.getGoogleAnrTimestamps(
sessionStart,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,14 +94,11 @@ public interface EmbraceInternalInterface : InternalTracingApi {
)

/**
* Record a network request and overwrite any previously recorded request with the same callId
* Record a network request.
*
* @param callId the ID with which the request will be identified internally. The session will only contain one recorded
* request with a given ID - last writer wins.
* @param embraceNetworkRequest the request to be recorded
*/
public fun recordAndDeduplicateNetworkRequest(
callId: String,
public fun recordNetworkRequest(
embraceNetworkRequest: EmbraceNetworkRequest
)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -573,8 +573,7 @@ synchronized void internalLogNetworkCall(long startTime, long endTime, boolean o
long contentLength = Math.max(0, responseSize.get());

if (inputStreamAccessException == null && lastConnectionAccessException == null && responseCode.get() != 0) {
embrace.getInternalInterface().recordAndDeduplicateNetworkRequest(
callId,
embrace.recordNetworkRequest(
EmbraceNetworkRequest.fromCompletedRequest(
url,
HttpMethod.fromString(getRequestMethod()),
Expand Down Expand Up @@ -604,8 +603,7 @@ synchronized void internalLogNetworkCall(long startTime, long endTime, boolean o
String errorType = exceptionClass != null ? exceptionClass : "UnknownState";
String errorMessage = exceptionMessage != null ? exceptionMessage : "HTTP response state unknown";

embrace.getInternalInterface().recordAndDeduplicateNetworkRequest(
callId,
embrace.recordNetworkRequest(
EmbraceNetworkRequest.fromIncompleteRequest(
url,
HttpMethod.fromString(getRequestMethod()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import io.embrace.android.embracesdk.config.ConfigService
import io.embrace.android.embracesdk.internal.CacheableValue
import io.embrace.android.embracesdk.internal.network.http.NetworkCaptureData
import io.embrace.android.embracesdk.logging.InternalEmbraceLogger
import io.embrace.android.embracesdk.network.EmbraceNetworkRequest
import io.embrace.android.embracesdk.network.logging.EmbraceNetworkCaptureService.Companion.NETWORK_ERROR_CODE
import io.embrace.android.embracesdk.payload.NetworkCallV2
import io.embrace.android.embracesdk.payload.NetworkSessionV2
Expand Down Expand Up @@ -52,7 +53,7 @@ internal class EmbraceNetworkLoggingService(

private var domainSuffixCallLimits = configService.networkBehavior.getNetworkCallLimitsPerDomainSuffix()

override fun getNetworkCallsSnapshot(): NetworkSessionV2 {
fun getNetworkCallsSnapshot(): NetworkSessionV2 {
var storedCallsSize: Int? = null
var cachedCallsSize: Int? = null

Expand Down Expand Up @@ -82,7 +83,7 @@ internal class EmbraceNetworkLoggingService(
}
}

override fun logNetworkCall(
fun logNetworkCall(
callId: String,
url: String,
httpMethod: String,
Expand Down Expand Up @@ -125,7 +126,7 @@ internal class EmbraceNetworkLoggingService(
processNetworkCall(callId, networkCall)
}

override fun logNetworkError(
fun logNetworkError(
callId: String,
url: String,
httpMethod: String,
Expand Down Expand Up @@ -255,4 +256,8 @@ internal class EmbraceNetworkLoggingService(
sessionNetworkCalls.clear()
}
}

override fun logNetworkRequest(networkRequest: EmbraceNetworkRequest) {
// TODO("Not yet implemented")
}
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package io.embrace.android.embracesdk.network.logging

import io.embrace.android.embracesdk.internal.network.http.NetworkCaptureData
import io.embrace.android.embracesdk.payload.NetworkSessionV2
import io.embrace.android.embracesdk.network.EmbraceNetworkRequest

/**
* Logs network calls made by the application. The Embrace SDK intercepts the calls and reports
Expand All @@ -10,66 +9,10 @@ import io.embrace.android.embracesdk.payload.NetworkSessionV2
internal interface NetworkLoggingService {

/**
* Get the calls and counts of network calls (which exceed the limit) that haven't been associated with a session or background activity
* Logs a network request.
* This network request is considered unique and finished, meaning that we will not receive additional data for it.
*
* @return the network calls for the given session
* @param networkRequest the network request to log
*/
fun getNetworkCallsSnapshot(): NetworkSessionV2

/**
* Logs a HTTP network call.
*
* @param callId the unique ID of the call used for deduplication purposes
* @param url the URL being called
* @param httpMethod the HTTP method
* @param statusCode the status code from the response
* @param startTime the start time of the request
* @param endTime the end time of the request
* @param bytesSent the number of bytes sent
* @param bytesReceived the number of bytes received
* @param traceId optional trace ID that can be used to trace a particular request
* @param w3cTraceparent optional W3C-compliant traceparent representing the network call that is being recorded
* @param networkCaptureData the additional data captured if network body capture is enabled for the URL
*/
@Suppress("LongParameterList")
fun logNetworkCall(
callId: String,
url: String,
httpMethod: String,
statusCode: Int,
startTime: Long,
endTime: Long,
bytesSent: Long,
bytesReceived: Long,
traceId: String?,
w3cTraceparent: String?,
networkCaptureData: NetworkCaptureData?
)

/**
* Logs an exception which occurred when attempting to make a network call.
*
* @param callId the unique ID of the call used for deduplication purposes
* @param url the URL being called
* @param httpMethod the HTTP method
* @param startTime the start time of the request
* @param endTime the end time of the request
* @param errorType the type of error being thrown
* @param errorMessage the error message
* @param traceId optional trace ID that can be used to trace a particular request
* @param w3cTraceparent optional W3C-compliant traceparent representing the network call that is being recorded
* @param networkCaptureData the additional data captured if network body capture is enabled for the URL
*/
fun logNetworkError(
callId: String,
url: String,
httpMethod: String,
startTime: Long,
endTime: Long,
errorType: String?,
errorMessage: String?,
traceId: String?,
w3cTraceparent: String?,
networkCaptureData: NetworkCaptureData?
)
fun logNetworkRequest(networkRequest: EmbraceNetworkRequest)
}
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,6 @@ internal class EmbraceInternalInterfaceImplTest {
@Test
fun testRecordAndDeduplicateNetworkRequest() {
val url = "https://embrace.io"
val callId = "testID"
val captor = slot<EmbraceNetworkRequest>()
val networkRequest: EmbraceNetworkRequest = EmbraceNetworkRequest.fromCompletedRequest(
url,
Expand All @@ -202,9 +201,9 @@ internal class EmbraceInternalInterfaceImplTest {
200
)

internalImpl.recordAndDeduplicateNetworkRequest(callId, networkRequest)
internalImpl.recordNetworkRequest(networkRequest)
verify(exactly = 1) {
embraceImpl.recordAndDeduplicateNetworkRequest(callId, capture(captor))
embraceImpl.recordNetworkRequest(capture(captor))
}

assertEquals(url, captor.captured.url)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,6 @@ internal class EmbracePerformanceInfoServiceTest {
}

private fun assertBasicSessionPerfInfoIncluded(info: PerformanceInfo) {
assertValueCopied(NetworkRequests(networkLoggingService.data), info.networkRequests)
assertValueCopied(
googleAnrTimestampRepository.getGoogleAnrTimestamps(0, SESSION_END_TIME_MS),
info.googleAnrTimestamps
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ internal class UninitializedSdkInternalInterfaceImplTest {
2509L,
200
)
impl.recordAndDeduplicateNetworkRequest("testID", request)
impl.recordNetworkRequest(request)
impl.recordIncompleteNetworkRequest(
"https://google.com",
"get",
Expand Down

0 comments on commit 2c3bd85

Please sign in to comment.