Skip to content

Commit

Permalink
Consolidate internal API into EmbraceInternalApi (#21)
Browse files Browse the repository at this point in the history
## Goal

We declare internal APIs in a number of different way: directly putting them in `Embrace.java` and mark them as internal with an annotation, using `SdkFacade` so we can mock it for tests, and putting them into `EmbraceInternalInterface`. Going forward, we only want to do the latter, and this set of commits achieve that.

## Testing

Fixed existing tests that now pass
  • Loading branch information
bidetofevil authored Oct 23, 2023
2 parents 3b721fe + 6e930cb commit 009b076
Show file tree
Hide file tree
Showing 28 changed files with 597 additions and 273 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ internal class EmbraceNodeIterator {
backgroundWorker.submit {
findClickedElement(semanticsNodes, x, y)?.let {
val clickedView = ClickedView(it, x, y)
Embrace.getInstance().internalInterface?.logComposeTap(Pair(clickedView.x, clickedView.y), clickedView.tag)
Embrace.getInstance().internalInterface.logComposeTap(Pair(clickedView.x, clickedView.y), clickedView.tag)
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ public static void _onMessageReceived(@NonNull RemoteMessage message) {
private static void handleRemoteMessage(@NonNull RemoteMessage message) {
try {
//flag process is already running to avoid track warm startup
Embrace.getInstance().setProcessStartedByNotification();
Embrace.getInstance().getInternalInterface().setProcessStartedByNotification();

String messageId = null;
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,15 +38,12 @@ public class EmbraceOkHttp3ApplicationInterceptor implements Interceptor {
static final String UNKNOWN_MESSAGE = "An error occurred during the execution of this network request";
final Embrace embrace;

private final SdkFacade sdkFacade;

public EmbraceOkHttp3ApplicationInterceptor() {
this(Embrace.getInstance(), new SdkFacade());
this(Embrace.getInstance());
}

EmbraceOkHttp3ApplicationInterceptor(Embrace embrace, SdkFacade sdkFacade) {
EmbraceOkHttp3ApplicationInterceptor(Embrace embrace) {
this.embrace = embrace;
this.sdkFacade = sdkFacade;
}

@Override
Expand All @@ -69,7 +66,7 @@ public Response intercept(Chain chain) throws IOException {
causeName(e, UNKNOWN_EXCEPTION),
causeMessage(e, UNKNOWN_MESSAGE),
request.header(embrace.getTraceIdHeader()),
sdkFacade.isNetworkSpanForwardingEnabled() ? request.header(TRACEPARENT_HEADER_NAME) : null,
embrace.getInternalInterface().isNetworkSpanForwardingEnabled() ? request.header(TRACEPARENT_HEADER_NAME) : null,
null
)
);
Expand All @@ -91,7 +88,7 @@ public Response intercept(Chain chain) throws IOException {
errorType != null ? errorType : UNKNOWN_EXCEPTION,
errorMessage != null ? errorMessage : UNKNOWN_MESSAGE,
request.header(embrace.getTraceIdHeader()),
sdkFacade.isNetworkSpanForwardingEnabled() ? request.header(TRACEPARENT_HEADER_NAME) : null,
embrace.getInternalInterface().isNetworkSpanForwardingEnabled() ? request.header(TRACEPARENT_HEADER_NAME) : null,
null
)
);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -58,15 +58,13 @@ public final class EmbraceOkHttp3NetworkInterceptor implements Interceptor {
};

final Embrace embrace;
private final SdkFacade sdkFacade;

public EmbraceOkHttp3NetworkInterceptor() {
this(Embrace.getInstance(), new SdkFacade());
this(Embrace.getInstance());
}

EmbraceOkHttp3NetworkInterceptor(Embrace embrace, SdkFacade sdkFacade) {
EmbraceOkHttp3NetworkInterceptor(Embrace embrace) {
this.embrace = embrace;
this.sdkFacade = sdkFacade;
}

@Override
Expand All @@ -77,7 +75,7 @@ public Response intercept(Chain chain) throws IOException {
return chain.proceed(originalRequest);
}

boolean networkSpanForwardingEnabled = sdkFacade.isNetworkSpanForwardingEnabled();
boolean networkSpanForwardingEnabled = embrace.getInternalInterface().isNetworkSpanForwardingEnabled();

String traceparent = null;
if (networkSpanForwardingEnabled && originalRequest.header(TRACEPARENT_HEADER_NAME) == null) {
Expand Down Expand Up @@ -123,7 +121,8 @@ public Response intercept(Chain chain) throws IOException {
contentLength = 0L;
}

boolean shouldCaptureNetworkData = embrace.shouldCaptureNetworkBody(request.url().toString(), request.method());
boolean shouldCaptureNetworkData =
embrace.getInternalInterface().shouldCaptureNetworkBody(request.url().toString(), request.method());

if (shouldCaptureNetworkData &&
ENCODING_GZIP.equalsIgnoreCase(networkResponse.header(CONTENT_ENCODING_HEADER_NAME)) &&
Expand Down

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
package io.embrace.android.embracesdk.okhttp3

import io.embrace.android.embracesdk.Embrace
import io.embrace.android.embracesdk.internal.EmbraceInternalInterface
import io.embrace.android.embracesdk.network.EmbraceNetworkRequest
import io.embrace.android.embracesdk.network.http.NetworkCaptureData
import io.embrace.android.embracesdk.okhttp3.EmbraceOkHttp3ApplicationInterceptor.UNKNOWN_EXCEPTION
Expand Down Expand Up @@ -64,7 +65,7 @@ internal class EmbraceOkHttp3InterceptorsTest {
private lateinit var postNetworkInterceptorTestInterceptor: Interceptor
private lateinit var okHttpClient: OkHttpClient
private lateinit var mockEmbrace: Embrace
private lateinit var mockSdkFacade: SdkFacade
private lateinit var mockInternalInterface: EmbraceInternalInterface
private lateinit var getRequestBuilder: Request.Builder
private lateinit var postRequestBuilder: Request.Builder
private lateinit var capturedEmbraceNetworkRequest: CapturingSlot<EmbraceNetworkRequest>
Expand All @@ -79,13 +80,16 @@ internal class EmbraceOkHttp3InterceptorsTest {
fun setup() {
server = MockWebServer()
mockEmbrace = mockk(relaxed = true)
mockSdkFacade = mockk(relaxed = true)
applicationInterceptor = EmbraceOkHttp3ApplicationInterceptor(mockEmbrace, mockSdkFacade)
mockInternalInterface = mockk(relaxed = true)
every { mockInternalInterface.shouldCaptureNetworkBody(any(), "POST") } answers { true }
every { mockInternalInterface.shouldCaptureNetworkBody(any(), "GET") } answers { false }
every { mockInternalInterface.isNetworkSpanForwardingEnabled() } answers { isNetworkSpanForwardingEnabled }
applicationInterceptor = EmbraceOkHttp3ApplicationInterceptor(mockEmbrace)
preNetworkInterceptorTestInterceptor = TestInspectionInterceptor(
beforeRequestSent = { request -> preNetworkInterceptorBeforeRequestSupplier.invoke(request) },
afterResponseReceived = { response -> preNetworkInterceptorAfterResponseSupplier.invoke(response) }
)
networkInterceptor = EmbraceOkHttp3NetworkInterceptor(mockEmbrace, mockSdkFacade)
networkInterceptor = EmbraceOkHttp3NetworkInterceptor(mockEmbrace)
postNetworkInterceptorTestInterceptor = TestInspectionInterceptor(
beforeRequestSent = { request -> postNetworkInterceptorBeforeRequestSupplier.invoke(request) },
afterResponseReceived = { response -> postNetworkInterceptorAfterResponseSupplier.invoke(response) }
Expand All @@ -106,11 +110,9 @@ internal class EmbraceOkHttp3InterceptorsTest {
.header(requestHeaderName, requestHeaderValue)
capturedEmbraceNetworkRequest = slot()
every { mockEmbrace.isStarted } answers { isSDKStarted }
every { mockEmbrace.shouldCaptureNetworkBody(any(), "POST") } answers { true }
every { mockEmbrace.shouldCaptureNetworkBody(any(), "GET") } answers { false }
every { mockEmbrace.recordNetworkRequest(capture(capturedEmbraceNetworkRequest)) } answers { }
every { mockEmbrace.generateW3cTraceparent() } answers { GENERATED_TRACEPARENT }
every { mockSdkFacade.isNetworkSpanForwardingEnabled } answers { isNetworkSpanForwardingEnabled }
every { mockEmbrace.internalInterface } answers { mockInternalInterface }
}

@After
Expand Down Expand Up @@ -184,8 +186,8 @@ internal class EmbraceOkHttp3InterceptorsTest {
isSDKStarted = false
server.enqueue(createBaseMockResponse())
runGetRequest()
verify(exactly = 0) { mockSdkFacade.isNetworkSpanForwardingEnabled }
verify(exactly = 0) { mockEmbrace.shouldCaptureNetworkBody(any(), any()) }
verify(exactly = 0) { mockInternalInterface.isNetworkSpanForwardingEnabled() }
verify(exactly = 0) { mockEmbrace.internalInterface.shouldCaptureNetworkBody(any(), any()) }
}

@Test
Expand Down
32 changes: 16 additions & 16 deletions embrace-android-sdk/api/embrace-android-sdk.api
Original file line number Diff line number Diff line change
Expand Up @@ -37,15 +37,14 @@ public final class io/embrace/android/embracesdk/Embrace : io/embrace/android/em
public fun getDeviceId ()Ljava/lang/String;
public fun getFlutterInternalInterface ()Lio/embrace/android/embracesdk/FlutterInternalInterface;
public static fun getInstance ()Lio/embrace/android/embracesdk/Embrace;
public fun getInternalInterface ()Lio/embrace/android/embracesdk/EmbraceInternalInterface;
public fun getInternalInterface ()Lio/embrace/android/embracesdk/internal/EmbraceInternalInterface;
public fun getLastRunEndState ()Lio/embrace/android/embracesdk/Embrace$LastRunEndState;
public fun getReactNativeInternalInterface ()Lio/embrace/android/embracesdk/ReactNativeInternalInterface;
public fun getSessionProperties ()Ljava/util/Map;
public fun getTraceIdHeader ()Ljava/lang/String;
public fun getUnityInternalInterface ()Lio/embrace/android/embracesdk/UnityInternalInterface;
public fun isStarted ()Z
public fun isTracingAvailable ()Z
public fun logComposeTap (Landroid/util/Pair;Ljava/lang/String;)V
public fun logCustomStacktrace ([Ljava/lang/StackTraceElement;)V
public fun logCustomStacktrace ([Ljava/lang/StackTraceElement;Lio/embrace/android/embracesdk/Severity;)V
public fun logCustomStacktrace ([Ljava/lang/StackTraceElement;Lio/embrace/android/embracesdk/Severity;Ljava/util/Map;)V
Expand Down Expand Up @@ -80,12 +79,10 @@ public final class io/embrace/android/embracesdk/Embrace : io/embrace/android/em
public fun setAppId (Ljava/lang/String;)Z
public fun setDartVersion (Ljava/lang/String;)V
public fun setEmbraceFlutterSdkVersion (Ljava/lang/String;)V
public fun setProcessStartedByNotification ()V
public fun setUserAsPayer ()V
public fun setUserEmail (Ljava/lang/String;)V
public fun setUserIdentifier (Ljava/lang/String;)V
public fun setUsername (Ljava/lang/String;)V
public fun shouldCaptureNetworkBody (Ljava/lang/String;Ljava/lang/String;)Z
public fun start (Landroid/content/Context;)V
public fun start (Landroid/content/Context;Z)V
public fun start (Landroid/content/Context;ZLio/embrace/android/embracesdk/Embrace$AppFramework;)V
Expand Down Expand Up @@ -116,18 +113,6 @@ public final class io/embrace/android/embracesdk/Embrace$LastRunEndState : java/
public static fun values ()[Lio/embrace/android/embracesdk/Embrace$LastRunEndState;
}

public abstract interface class io/embrace/android/embracesdk/EmbraceInternalInterface {
public abstract fun logComposeTap (Landroid/util/Pair;Ljava/lang/String;)V
public abstract fun logError (Ljava/lang/String;Ljava/util/Map;Ljava/lang/String;Z)V
public abstract fun logHandledException (Ljava/lang/Throwable;Lio/embrace/android/embracesdk/LogType;Ljava/util/Map;[Ljava/lang/StackTraceElement;)V
public abstract fun logInfo (Ljava/lang/String;Ljava/util/Map;)V
public abstract fun logWarning (Ljava/lang/String;Ljava/util/Map;Ljava/lang/String;)V
public abstract fun recordAndDeduplicateNetworkRequest (Ljava/lang/String;Lio/embrace/android/embracesdk/network/EmbraceNetworkRequest;)V
public abstract fun recordCompletedNetworkRequest (Ljava/lang/String;Ljava/lang/String;JJJJILjava/lang/String;Lio/embrace/android/embracesdk/network/http/NetworkCaptureData;)V
public abstract fun recordIncompleteNetworkRequest (Ljava/lang/String;Ljava/lang/String;JJLjava/lang/String;Ljava/lang/String;Ljava/lang/String;Lio/embrace/android/embracesdk/network/http/NetworkCaptureData;)V
public abstract fun recordIncompleteNetworkRequest (Ljava/lang/String;Ljava/lang/String;JJLjava/lang/Throwable;Ljava/lang/String;Lio/embrace/android/embracesdk/network/http/NetworkCaptureData;)V
}

public final class io/embrace/android/embracesdk/EmbraceSamples {
public static final field INSTANCE Lio/embrace/android/embracesdk/EmbraceSamples;
public static final fun causeNdkIllegalInstruction ()V
Expand Down Expand Up @@ -192,6 +177,21 @@ public final class io/embrace/android/embracesdk/WebViewClientSwazzledHooks {
public abstract interface annotation class io/embrace/android/embracesdk/annotation/StartupActivity : java/lang/annotation/Annotation {
}

public abstract interface class io/embrace/android/embracesdk/internal/EmbraceInternalInterface {
public abstract fun isNetworkSpanForwardingEnabled ()Z
public abstract fun logComposeTap (Landroid/util/Pair;Ljava/lang/String;)V
public abstract fun logError (Ljava/lang/String;Ljava/util/Map;Ljava/lang/String;Z)V
public abstract fun logHandledException (Ljava/lang/Throwable;Lio/embrace/android/embracesdk/LogType;Ljava/util/Map;[Ljava/lang/StackTraceElement;)V
public abstract fun logInfo (Ljava/lang/String;Ljava/util/Map;)V
public abstract fun logWarning (Ljava/lang/String;Ljava/util/Map;Ljava/lang/String;)V
public abstract fun recordAndDeduplicateNetworkRequest (Ljava/lang/String;Lio/embrace/android/embracesdk/network/EmbraceNetworkRequest;)V
public abstract fun recordCompletedNetworkRequest (Ljava/lang/String;Ljava/lang/String;JJJJILjava/lang/String;Lio/embrace/android/embracesdk/network/http/NetworkCaptureData;)V
public abstract fun recordIncompleteNetworkRequest (Ljava/lang/String;Ljava/lang/String;JJLjava/lang/String;Ljava/lang/String;Ljava/lang/String;Lio/embrace/android/embracesdk/network/http/NetworkCaptureData;)V
public abstract fun recordIncompleteNetworkRequest (Ljava/lang/String;Ljava/lang/String;JJLjava/lang/Throwable;Ljava/lang/String;Lio/embrace/android/embracesdk/network/http/NetworkCaptureData;)V
public abstract fun setProcessStartedByNotification ()V
public abstract fun shouldCaptureNetworkBody (Ljava/lang/String;Ljava/lang/String;)Z
}

public final class io/embrace/android/embracesdk/network/EmbraceNetworkRequest {
public static fun fromCompletedRequest (Ljava/lang/String;Lio/embrace/android/embracesdk/network/http/HttpMethod;JJJJI)Lio/embrace/android/embracesdk/network/EmbraceNetworkRequest;
public static fun fromCompletedRequest (Ljava/lang/String;Lio/embrace/android/embracesdk/network/http/HttpMethod;JJJJILjava/lang/String;)Lio/embrace/android/embracesdk/network/EmbraceNetworkRequest;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -4,11 +4,14 @@ import io.embrace.android.embracesdk.IntegrationTestRule.Harness
import io.embrace.android.embracesdk.config.local.LocalConfig
import io.embrace.android.embracesdk.config.local.NetworkLocalConfig
import io.embrace.android.embracesdk.config.local.SdkLocalConfig
import io.embrace.android.embracesdk.config.remote.NetworkCaptureRuleRemoteConfig
import io.embrace.android.embracesdk.config.remote.NetworkSpanForwardingRemoteConfig
import io.embrace.android.embracesdk.config.remote.RemoteConfig
import io.embrace.android.embracesdk.config.remote.SpansRemoteConfig
import io.embrace.android.embracesdk.fakes.FakeClock
import io.embrace.android.embracesdk.fakes.FakeConfigService
import io.embrace.android.embracesdk.fakes.fakeNetworkBehavior
import io.embrace.android.embracesdk.fakes.fakeNetworkSpanForwardingBehavior
import io.embrace.android.embracesdk.fakes.fakeSdkModeBehavior
import io.embrace.android.embracesdk.fakes.fakeSpansBehavior
import io.embrace.android.embracesdk.fakes.injection.FakeCoreModule
Expand Down Expand Up @@ -138,6 +141,9 @@ internal class IntegrationTestRule(
localCfg = { DEFAULT_SDK_LOCAL_CONFIG },
remoteCfg = { DEFAULT_SDK_REMOTE_CONFIG }
),
networkSpanForwardingBehavior = fakeNetworkSpanForwardingBehavior {
NetworkSpanForwardingRemoteConfig(pctEnabled = 100.0f)
},
spansBehavior = fakeSpansBehavior {
SpansRemoteConfig(pctEnabled = 100f)
}
Expand Down Expand Up @@ -196,7 +202,16 @@ internal class IntegrationTestRule(
)

private val DEFAULT_SDK_REMOTE_CONFIG = RemoteConfig(
disabledUrlPatterns = setOf("dontlogmebro.pizza")
disabledUrlPatterns = setOf("dontlogmebro.pizza"),
networkCaptureRules = setOf(
NetworkCaptureRuleRemoteConfig(
id = "test",
duration = 10000,
method = "GET",
urlRegex = "capture.me",
expiresIn = 10000
)
)
)

val DEFAULT_LOCAL_CONFIG = LocalConfig(
Expand Down
Original file line number Diff line number Diff line change
@@ -1,9 +1,12 @@
package io.embrace.android.embracesdk

import android.app.Activity
import io.embrace.android.embracesdk.annotation.StartupActivity
import io.embrace.android.embracesdk.logging.EmbraceInternalErrorService
import io.embrace.android.embracesdk.payload.EventMessage
import io.embrace.android.embracesdk.payload.SessionMessage
import org.junit.Assert.assertEquals
import org.robolectric.Robolectric
import java.util.concurrent.CountDownLatch
import java.util.concurrent.TimeUnit
import java.util.concurrent.TimeoutException
Expand Down Expand Up @@ -59,10 +62,18 @@ internal fun IntegrationTestRule.Harness.getLastSentSessionMessage(): SessionMes
* should always be 30s long. Additionally, it performs assertions against fields that
* are guaranteed not to change in the start/end message.
*/
internal fun IntegrationTestRule.Harness.recordSession(action: () -> Unit): SessionMessage {
internal fun IntegrationTestRule.Harness.recordSession(
simulateAppStartup: Boolean = false,
action: () -> Unit
): SessionMessage {
// get the activity service & simulate the lifecycle event that triggers a new session.
val activityService = checkNotNull(Embrace.getImpl().activityService)
val activityController = if (simulateAppStartup) Robolectric.buildActivity(Activity::class.java) else null

activityController?.create()
activityController?.start()
activityService.onForeground()
activityController?.resume()

// assert a session was started.
val startSession = getLastSentSessionMessage()
Expand All @@ -74,12 +85,15 @@ internal fun IntegrationTestRule.Harness.recordSession(action: () -> Unit): Sess

// end session 30s later by entering background
fakeClock.tick(30000)
activityController?.pause()
activityService.onBackground()
activityController?.stop()

val endSession = getLastSentSessionMessage()
assertEquals("en", endSession.session.messageType)
// TODO: future: increase number of assertions on what is always in a start message?


// return the session end message for further assertions.
return endSession
}
Expand Down
Loading

0 comments on commit 009b076

Please sign in to comment.