Skip to content

Commit

Permalink
Use new sdk time direcctly and indirectly for timetamps sent in SDK
Browse files Browse the repository at this point in the history
  • Loading branch information
bidetofevil committed Oct 18, 2023
1 parent a5fae6f commit 5f7c4a1
Show file tree
Hide file tree
Showing 5 changed files with 29 additions and 15 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ public EmbraceOkHttp3ApplicationInterceptor() {

@Override
public Response intercept(Chain chain) throws IOException {
long startTime = System.currentTimeMillis();
long startTime = embrace.getSdkApi().getSdkCurrentTime();
Request request = chain.request();
try {
// we are not interested in response, just proceed
Expand All @@ -65,7 +65,7 @@ public Response intercept(Chain chain) throws IOException {
urlString,
HttpMethod.fromString(request.method()),
startTime,
System.currentTimeMillis(),
embrace.getSdkApi().getSdkCurrentTime(),
causeName(e, UNKNOWN_EXCEPTION),
causeMessage(e, UNKNOWN_MESSAGE),
request.header(embrace.getTraceIdHeader()),
Expand All @@ -87,7 +87,7 @@ public Response intercept(Chain chain) throws IOException {
urlString,
HttpMethod.fromString(request.method()),
startTime,
System.currentTimeMillis(),
embrace.getSdkApi().getSdkCurrentTime(),
errorType != null ? errorType : UNKNOWN_EXCEPTION,
errorMessage != null ? errorMessage : UNKNOWN_MESSAGE,
request.header(embrace.getTraceIdHeader()),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,7 @@ public Response intercept(Chain chain) throws IOException {
return chain.proceed(originalRequest);
}

long preRequestClockOffset = sdkClockOffset();
boolean networkSpanForwardingEnabled = sdkFacade.isNetworkSpanForwardingEnabled();

String traceparent = null;
Expand All @@ -88,6 +89,7 @@ public Response intercept(Chain chain) throws IOException {
originalRequest : originalRequest.newBuilder().header(TRACEPARENT_HEADER_NAME, traceparent).build();

Response networkResponse = chain.proceed(request);
long postResponseClockOffset = sdkClockOffset();
Response.Builder responseBuilder = networkResponse.newBuilder().request(request);

Long contentLength = null;
Expand Down Expand Up @@ -153,12 +155,14 @@ public Response intercept(Chain chain) throws IOException {
networkCaptureData = getNetworkCaptureData(request, response);
}



embrace.recordNetworkRequest(
EmbraceNetworkRequest.fromCompletedRequest(
EmbraceHttpPathOverride.getURLString(new EmbraceOkHttp3PathOverrideRequest(request)),
HttpMethod.fromString(request.method()),
response.sentRequestAtMillis(),
response.receivedResponseAtMillis(),
response.sentRequestAtMillis() + preRequestClockOffset,
response.receivedResponseAtMillis() + postResponseClockOffset,
request.body() != null ? request.body().contentLength() : 0,
contentLength,
response.code(),
Expand Down Expand Up @@ -250,4 +254,12 @@ private byte[] getRequestBody(final Request request) {
}
return null;
}

/**
* Get the difference between the SDK clock time and the time System.currentTimeMillis() returns, which is used by OkHttp for
* determining client-side timestamps.
*/
private long sdkClockOffset() {
return embrace.getSdkApi().getSdkCurrentTime() - System.currentTimeMillis();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -110,6 +110,7 @@ internal class EmbraceOkHttp3InterceptorsTest {
every { mockEmbrace.shouldCaptureNetworkBody(any(), "GET") } answers { false }
every { mockEmbrace.recordNetworkRequest(capture(capturedEmbraceNetworkRequest)) } answers { }
every { mockEmbrace.generateW3cTraceparent() } answers { GENERATED_TRACEPARENT }
every { mockEmbrace.sdkApi.getSdkCurrentTime() } answers { System.currentTimeMillis() }
every { mockSdkFacade.isNetworkSpanForwardingEnabled } answers { isNetworkSpanForwardingEnabled }
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -154,9 +154,9 @@ public EmbraceUrlConnectionOverride(@NonNull T connection, boolean enableWrapIoS
EmbraceUrlConnectionOverride(@NonNull T connection, boolean enableWrapIoStreams,
@NonNull Embrace embrace) {
this.connection = connection;
this.createdTime = System.currentTimeMillis();
this.enableWrapIoStreams = enableWrapIoStreams;
this.embrace = embrace;
this.createdTime = embrace.getSdkApi().getSdkCurrentTime();
}

@Override
Expand Down Expand Up @@ -354,7 +354,7 @@ public long getHeaderFieldLong(@NonNull String name, long defaultValue) {
@Override
@Nullable
public Map<String, List<String>> getHeaderFields() {
final long startTime = System.currentTimeMillis();
final long startTime = embrace.getSdkApi().getSdkCurrentTime();
cacheResponseData();
internalLogNetworkCall(startTime);
return headerFields.get();
Expand All @@ -367,7 +367,7 @@ private <R> R retrieveHeaderField(@Nullable String name,
if (name == null) {
return null;
}
long startTime = System.currentTimeMillis();
long startTime = embrace.getSdkApi().getSdkCurrentTime();

Check warning on line 370 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/network/http/EmbraceUrlConnectionOverride.java

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/network/http/EmbraceUrlConnectionOverride.java#L370

Added line #L370 was not covered by tests
if (shouldInterceptHeaderRetrieval(name)) {
// Strip the content encoding and length headers, as we transparently ungzip the content
return defaultValue;
Expand Down Expand Up @@ -467,9 +467,9 @@ public String getRequestProperty(@NonNull String key) {
}

@Override
public int getResponseCode() throws IOException {
public int getResponseCode() {
identifyTraceId();
long startTime = System.currentTimeMillis();
long startTime = embrace.getSdkApi().getSdkCurrentTime();
cacheResponseData();
internalLogNetworkCall(startTime);
return responseCode.get();
Expand All @@ -479,7 +479,7 @@ public int getResponseCode() throws IOException {
@Nullable
public String getResponseMessage() throws IOException {
identifyTraceId();
long startTime = System.currentTimeMillis();
long startTime = embrace.getSdkApi().getSdkCurrentTime();

Check warning on line 482 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/network/http/EmbraceUrlConnectionOverride.java

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/network/http/EmbraceUrlConnectionOverride.java#L482

Added line #L482 was not covered by tests
String responseMsg = this.connection.getResponseMessage();
cacheResponseData();
internalLogNetworkCall(startTime);
Expand Down Expand Up @@ -544,7 +544,7 @@ public boolean usingProxy() {
* ignored.
*/
synchronized void internalLogNetworkCall(long startTime) {
internalLogNetworkCall(startTime, System.currentTimeMillis(), false, null);
internalLogNetworkCall(startTime, embrace.getSdkApi().getSdkCurrentTime(), false, null);
}

/**
Expand Down Expand Up @@ -787,7 +787,7 @@ public Principal getPeerPrincipal() throws SSLPeerUnverifiedException {
@Nullable
private InputStream getWrappedInputStream(InputStream connectionInputStream) {
identifyTraceId();
long startTime = System.currentTimeMillis();
long startTime = embrace.getSdkApi().getSdkCurrentTime();

InputStream in = null;
if (shouldUncompressGzip()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,9 +76,10 @@ internal object EmbraceCrashSamples {
fun triggerLongAnr() {
isSdkStarted()
checkAnrDetectionEnabled()
val start = System.currentTimeMillis()
val embrace = Embrace.getInstance()
val start = embrace.sdkApi.getSdkCurrentTime()

Check warning on line 80 in embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/samples/EmbraceCrashSamples.kt

View check run for this annotation

Codecov / codecov/patch

embrace-android-sdk/src/main/java/io/embrace/android/embracesdk/samples/EmbraceCrashSamples.kt#L79-L80

Added lines #L79 - L80 were not covered by tests
while (true) {
if (System.currentTimeMillis() - start >= LONG_ANR_LENGTH) {
if (embrace.sdkApi.getSdkCurrentTime() - start >= LONG_ANR_LENGTH) {
break
}
}
Expand Down

0 comments on commit 5f7c4a1

Please sign in to comment.