Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Only log network request when getWrappedStream is called #787

Merged
merged 2 commits into from
Apr 20, 2024
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -174,7 +174,8 @@ public void connect() throws IOException {
@Override
public void disconnect() {
// The network call must be logged before we close the transport
internalLogNetworkCall(createdTime);
setStartTime(createdTime);
internalLogNetworkCall();
this.connection.disconnect();
}

Expand Down Expand Up @@ -347,8 +348,7 @@ public long getHeaderFieldLong(@NonNull String name, long defaultValue) {
@Override
@Nullable
public Map<String, List<String>> getHeaderFields() {
final long startTime = embrace.getInternalInterface().getSdkCurrentTime();
cacheAndLogNetworkCall(startTime);
cacheNetworkCallData();
return headerFields.get();
}

Expand All @@ -359,14 +359,13 @@ private <R> R retrieveHeaderField(@Nullable String name,
if (name == null) {
return null;
}
long startTime = embrace.getInternalInterface().getSdkCurrentTime();
if (shouldInterceptHeaderRetrieval(name)) {
// Strip the content encoding and length headers, as we transparently ungzip the content
return defaultValue;
}

R result = action.invoke();
cacheAndLogNetworkCall(startTime);
cacheNetworkCallData();

return result;
}
Expand Down Expand Up @@ -461,19 +460,16 @@ public String getRequestProperty(@NonNull String key) {
@Override
public int getResponseCode() {
identifyTraceId();
long startTime = embrace.getInternalInterface().getSdkCurrentTime();
cacheAndLogNetworkCall(startTime);
cacheNetworkCallData();
return responseCode.get();
}

@Override
@Nullable
public String getResponseMessage() throws IOException {
identifyTraceId();
long startTime = embrace.getInternalInterface().getSdkCurrentTime();
String responseMsg = this.connection.getResponseMessage();
cacheAndLogNetworkCall(startTime);
return responseMsg;
cacheNetworkCallData();
return this.connection.getResponseMessage();
}

@Override
Expand Down Expand Up @@ -532,12 +528,11 @@ public boolean usingProxy() {
* <p>
* If this delegate has already logged the call it represents, this method is a no-op.
*/
synchronized void internalLogNetworkCall(long startTime) {
synchronized void internalLogNetworkCall() {
if (isSDKStarted && !this.didLogNetworkCall) {
// We are proactive with setting this flag so that we don't get nested calls to log the network call by virtue of
// extracting the data we need to log the network call.
this.didLogNetworkCall = true; // TODO: Wouldn't this mean that the network call might not be logged
this.startTime = startTime;
long endTime = embrace.getInternalInterface().getSdkCurrentTime();

String url = EmbraceHttpPathOverride.getURLString(new EmbraceHttpUrlConnectionOverride(this.connection));
Expand Down Expand Up @@ -632,7 +627,7 @@ private CountingInputStreamWithCallback countingInputStream(InputStream inputStr
hasNetworkCaptureRules(),
(responseBody) -> {
cacheNetworkCallData(responseBody);
internalLogNetworkCall(startTime);
internalLogNetworkCall();
return null;
});
}
Expand Down Expand Up @@ -760,8 +755,7 @@ public Principal getPeerPrincipal() throws SSLPeerUnverifiedException {
@Nullable
private InputStream getWrappedInputStream(InputStream connectionInputStream) {
identifyTraceId();
startTime = embrace.getInternalInterface().getSdkCurrentTime();

setStartTime(embrace.getInternalInterface().getSdkCurrentTime());
InputStream in = null;
if (shouldUncompressGzip()) {
try {
Expand All @@ -777,18 +771,11 @@ private InputStream getWrappedInputStream(InputStream connectionInputStream) {
countingInputStream(new BufferedInputStream(connectionInputStream)) : connectionInputStream;
}

cacheAndLogNetworkCall(startTime);
cacheNetworkCallData();

return in;
}

private void cacheAndLogNetworkCall(long startTime) {
if (!enableWrapIoStreams) {
cacheNetworkCallData();
internalLogNetworkCall(startTime);
}
}

private boolean hasNetworkCaptureRules() {
if (!isSDKStarted || this.connection.getURL() == null) {
return false;
Expand All @@ -800,16 +787,20 @@ private boolean hasNetworkCaptureRules() {
}

private void cacheNetworkCallData() {
if (isSDKStarted) {
cacheNetworkCallData(null);
}
cacheNetworkCallData(null);
}

/**
* Cache values from response at the first point of availability so that we won't try to retrieve these values when the response
* is not available.
*/
private void cacheNetworkCallData(@Nullable byte[] responseBody) {
if (!isSDKStarted) {
return;
}

setStartTime(embrace.getInternalInterface().getSdkCurrentTime());

if (headerFields.get() == null) {
synchronized (headerFields) {
if (headerFields.get() == null) {
Expand Down Expand Up @@ -856,6 +847,10 @@ private void cacheNetworkCallData(@Nullable byte[] responseBody) {
}
}

if (!enableWrapIoStreams) {
internalLogNetworkCall();
}

if (shouldCaptureNetworkData()) {
// If we don't have network capture rules, it's unnecessary to save these values
synchronized (networkCaptureData) {
Expand Down Expand Up @@ -905,6 +900,12 @@ private boolean shouldCaptureNetworkData() {
(networkCaptureData.get() == null || networkCaptureData.get().getCapturedResponseBody() == null);
}

private void setStartTime(@NonNull Long startTimeMs) {
if (startTime == null) {
startTime = startTimeMs;
}
}

private void logError(@NonNull Throwable t) {
Embrace.getInstance().getInternalInterface().logInternalError(t);
}
Expand Down