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

feat(profiling): Remove SentryOptions deps from AndroidProfiler #3051

Merged
merged 25 commits into from
Nov 17, 2023
Merged
Show file tree
Hide file tree
Changes from 22 commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
73a4296
chore(profiling): Create independent Profiler for Hybrid SDKs
krystofwoldrich Oct 30, 2023
4b78a33
Fix tx profiler close
krystofwoldrich Oct 30, 2023
533d836
Format code
getsentry-bot Oct 30, 2023
63588b7
Remove unused internal class
krystofwoldrich Oct 30, 2023
ced557f
Merge remote-tracking branch 'origin/kw-refactor-make-profiler-tx-ind…
krystofwoldrich Oct 30, 2023
8743222
Fix unit tests
krystofwoldrich Oct 30, 2023
4cfa249
Format code
getsentry-bot Oct 30, 2023
cc3e39f
add tests
krystofwoldrich Oct 30, 2023
5c2534e
Format code
getsentry-bot Oct 30, 2023
878d8fa
Merge remote-tracking branch 'origin/kw-refactor-make-profiler-tx-ind…
krystofwoldrich Oct 30, 2023
1440b79
Add changelog
krystofwoldrich Oct 30, 2023
0fcc994
Update api dump
krystofwoldrich Oct 30, 2023
f8d338d
make profiler methods synchronized
krystofwoldrich Nov 3, 2023
fcaea03
Merge remote-tracking branch 'origin/main' into kw-refactor-make-prof…
krystofwoldrich Nov 3, 2023
8e1f987
update changelog
krystofwoldrich Nov 3, 2023
2f3f864
Merge branch 'main' into kw-refactor-make-profiler-tx-independent
krystofwoldrich Nov 10, 2023
95f2df9
Merge branch 'main' into kw-hybrid-profiling
krystofwoldrich Nov 15, 2023
d8c160a
feat(profiling): Remove SentryOptions deps from AndroidProfiler
krystofwoldrich Nov 15, 2023
ece2492
Revert file utils changes
krystofwoldrich Nov 15, 2023
9e7b081
Format code
getsentry-bot Nov 15, 2023
e2b7f5d
Add changelog
krystofwoldrich Nov 15, 2023
cdfc501
Merge commit '9e7b08183e910c2b6096d4b69587190330775302' into kw-hybri…
krystofwoldrich Nov 15, 2023
dff3136
Fix tests
krystofwoldrich Nov 15, 2023
b154784
fix changelog
krystofwoldrich Nov 15, 2023
8e97da6
Merge branch 'main' into kw-hybrid-profiling
krystofwoldrich Nov 17, 2023
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
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
### Fixes

- (Internal) Extract Android Profiler and Measurements for Hybrid SDKs ([#3016](https://github.com/getsentry/sentry-java/pull/3016))
- (Internal) Remove SentryOptions dependency from AndroidProfiler ([#3051](https://github.com/getsentry/sentry-java/pull/3051))
krystofwoldrich marked this conversation as resolved.
Show resolved Hide resolved
- Ensure DSN uses http/https protocol ([#3044](https://github.com/getsentry/sentry-java/pull/3044))

### Features
Expand Down
2 changes: 1 addition & 1 deletion sentry-android-core/api/sentry-android-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -50,7 +50,7 @@ public class io/sentry/android/core/AndroidMemoryCollector : io/sentry/ICollecto
}

public class io/sentry/android/core/AndroidProfiler {
public fun <init> (Ljava/lang/String;ILio/sentry/android/core/internal/util/SentryFrameMetricsCollector;Lio/sentry/android/core/SentryAndroidOptions;Lio/sentry/android/core/BuildInfoProvider;)V
public fun <init> (Ljava/lang/String;ILio/sentry/android/core/internal/util/SentryFrameMetricsCollector;Lio/sentry/ISentryExecutorService;Lio/sentry/ILogger;Lio/sentry/android/core/BuildInfoProvider;)V
public fun close ()V
public fun endAndCollect (ZLjava/util/List;)Lio/sentry/android/core/AndroidProfiler$ProfileEndData;
public fun start ()Lio/sentry/android/core/AndroidProfiler$ProfileStartData;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@
import android.os.Process;
import android.os.SystemClock;
import io.sentry.CpuCollectionData;
import io.sentry.ILogger;
import io.sentry.ISentryExecutorService;
import io.sentry.MemoryCollectionData;
import io.sentry.PerformanceCollectionData;
import io.sentry.SentryLevel;
Expand Down Expand Up @@ -86,20 +88,23 @@ public ProfileEndData(
private final @NotNull ArrayDeque<ProfileMeasurementValue> frozenFrameRenderMeasurements =
new ArrayDeque<>();
private final @NotNull Map<String, ProfileMeasurement> measurementsMap = new HashMap<>();
private final @NotNull SentryAndroidOptions options;
private final @NotNull BuildInfoProvider buildInfoProvider;
private final @NotNull ISentryExecutorService executorService;
private final @NotNull ILogger logger;
private boolean isRunning = false;

public AndroidProfiler(
final @NotNull String tracesFilesDirPath,
final int intervalUs,
final @NotNull SentryFrameMetricsCollector frameMetricsCollector,
final @NotNull SentryAndroidOptions sentryAndroidOptions,
final @NotNull ISentryExecutorService executorService,
final @NotNull ILogger logger,
final @NotNull BuildInfoProvider buildInfoProvider) {
this.traceFilesDir =
new File(Objects.requireNonNull(tracesFilesDirPath, "TracesFilesDirPath is required"));
this.intervalUs = intervalUs;
this.options = Objects.requireNonNull(sentryAndroidOptions, "SentryAndroidOptions is required");
this.logger = Objects.requireNonNull(logger, "Logger is required");
this.executorService = Objects.requireNonNull(executorService, "ExecutorService is required.");
this.frameMetricsCollector =
Objects.requireNonNull(frameMetricsCollector, "SentryFrameMetricsCollector is required");
this.buildInfoProvider =
Expand All @@ -110,17 +115,13 @@ public AndroidProfiler(
public synchronized @Nullable ProfileStartData start() {
// intervalUs is 0 only if there was a problem in the init
if (intervalUs == 0) {
options
.getLogger()
.log(
SentryLevel.WARNING,
"Disabling profiling because intervaUs is set to %d",
intervalUs);
logger.log(
SentryLevel.WARNING, "Disabling profiling because intervaUs is set to %d", intervalUs);
return null;
}

if (isRunning) {
options.getLogger().log(SentryLevel.WARNING, "Profiling has already started...");
logger.log(SentryLevel.WARNING, "Profiling has already started...");
return null;
}

Expand Down Expand Up @@ -182,18 +183,13 @@ public void onFrameMetricCollected(
// We stop profiling after a timeout to avoid huge profiles to be sent
try {
scheduledFinish =
options
.getExecutorService()
.schedule(
() -> timedOutProfilingData = endAndCollect(true, null),
PROFILING_TIMEOUT_MILLIS);
executorService.schedule(
() -> timedOutProfilingData = endAndCollect(true, null), PROFILING_TIMEOUT_MILLIS);
} catch (RejectedExecutionException e) {
options
.getLogger()
.log(
SentryLevel.ERROR,
"Failed to call the executor. Profiling will not be automatically finished. Did you call Sentry.close()?",
e);
logger.log(
SentryLevel.ERROR,
"Failed to call the executor. Profiling will not be automatically finished. Did you call Sentry.close()?",
e);
}

transactionStartNanos = SystemClock.elapsedRealtimeNanos();
Expand All @@ -211,7 +207,7 @@ public void onFrameMetricCollected(
return new ProfileStartData(transactionStartNanos, profileStartCpuMillis);
} catch (Throwable e) {
endAndCollect(false, null);
options.getLogger().log(SentryLevel.ERROR, "Unable to start a profile: ", e);
logger.log(SentryLevel.ERROR, "Unable to start a profile: ", e);
isRunning = false;
return null;
}
Expand All @@ -227,7 +223,7 @@ public void onFrameMetricCollected(
}

if (!isRunning) {
options.getLogger().log(SentryLevel.WARNING, "Profiler not running");
logger.log(SentryLevel.WARNING, "Profiler not running");
return null;
}

Expand All @@ -240,7 +236,7 @@ public void onFrameMetricCollected(
// throws)
Debug.stopMethodTracing();
} catch (Throwable e) {
options.getLogger().log(SentryLevel.ERROR, "Error while stopping profiling: ", e);
logger.log(SentryLevel.ERROR, "Error while stopping profiling: ", e);
} finally {
isRunning = false;
}
Expand All @@ -250,7 +246,7 @@ public void onFrameMetricCollected(
long transactionEndCpuMillis = Process.getElapsedCpuTime();

if (traceFile == null) {
options.getLogger().log(SentryLevel.ERROR, "Trace file does not exists");
logger.log(SentryLevel.ERROR, "Trace file does not exists");
return null;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,7 +104,8 @@ private void init() {
tracesFilesDirPath,
(int) SECONDS.toMicros(1) / intervalHz,
frameMetricsCollector,
options,
options.getExecutorService(),
options.getLogger(),
buildInfoProvider);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
import android.view.FrameMetrics;
import android.view.Window;
import androidx.annotation.RequiresApi;
import io.sentry.ILogger;
import io.sentry.SentryLevel;
import io.sentry.SentryOptions;
import io.sentry.android.core.BuildInfoProvider;
Expand All @@ -32,7 +33,7 @@
public final class SentryFrameMetricsCollector implements Application.ActivityLifecycleCallbacks {
private final @NotNull BuildInfoProvider buildInfoProvider;
private final @NotNull Set<Window> trackedWindows = new CopyOnWriteArraySet<>();
private final @NotNull SentryOptions options;
private final @NotNull ILogger logger;
private @Nullable Handler handler;
private @Nullable WeakReference<Window> currentWindow;
private final @NotNull Map<String, FrameMetricsCollectorListener> listenerMap =
Expand All @@ -54,15 +55,33 @@ public SentryFrameMetricsCollector(
this(context, options, buildInfoProvider, new WindowFrameMetricsManager() {});
}

@SuppressLint("NewApi")
public SentryFrameMetricsCollector(
final @NotNull Context context,
final @NotNull ILogger logger,
final @NotNull BuildInfoProvider buildInfoProvider) {
this(context, logger, buildInfoProvider, new WindowFrameMetricsManager() {});
}

@SuppressWarnings("deprecation")
@SuppressLint({"NewApi", "DiscouragedPrivateApi"})
public SentryFrameMetricsCollector(
final @NotNull Context context,
final @NotNull SentryOptions options,
final @NotNull BuildInfoProvider buildInfoProvider,
final @NotNull WindowFrameMetricsManager windowFrameMetricsManager) {
this(context, options.getLogger(), buildInfoProvider, windowFrameMetricsManager);
}

@SuppressWarnings("deprecation")
@SuppressLint({"NewApi", "DiscouragedPrivateApi"})
public SentryFrameMetricsCollector(
final @NotNull Context context,
final @NotNull ILogger logger,
final @NotNull BuildInfoProvider buildInfoProvider,
final @NotNull WindowFrameMetricsManager windowFrameMetricsManager) {
Objects.requireNonNull(context, "The context is required");
this.options = Objects.requireNonNull(options, "SentryOptions is required");
this.logger = Objects.requireNonNull(logger, "Logger is required");
this.buildInfoProvider =
Objects.requireNonNull(buildInfoProvider, "BuildInfoProvider is required");
this.windowFrameMetricsManager =
Expand All @@ -81,8 +100,7 @@ public SentryFrameMetricsCollector(
HandlerThread handlerThread =
new HandlerThread("io.sentry.android.core.internal.util.SentryFrameMetricsCollector");
handlerThread.setUncaughtExceptionHandler(
(thread, e) ->
options.getLogger().log(SentryLevel.ERROR, "Error during frames measurements.", e));
(thread, e) -> logger.log(SentryLevel.ERROR, "Error during frames measurements.", e));
handlerThread.start();
handler = new Handler(handlerThread.getLooper());

Expand All @@ -100,22 +118,19 @@ public SentryFrameMetricsCollector(
try {
choreographer = Choreographer.getInstance();
} catch (Throwable e) {
options
.getLogger()
.log(
SentryLevel.ERROR,
"Error retrieving Choreographer instance. Slow and frozen frames will not be reported.",
e);
logger.log(
SentryLevel.ERROR,
"Error retrieving Choreographer instance. Slow and frozen frames will not be reported.",
e);
}
});
// Let's get the last frame timestamp from the choreographer private field
try {
choreographerLastFrameTimeField = Choreographer.class.getDeclaredField("mLastFrameTimeNanos");
choreographerLastFrameTimeField.setAccessible(true);
} catch (NoSuchFieldException e) {
options
.getLogger()
.log(SentryLevel.ERROR, "Unable to get the frame timestamp from the choreographer: ", e);
logger.log(
SentryLevel.ERROR, "Unable to get the frame timestamp from the choreographer: ", e);
}
frameMetricsAvailableListener =
(window, frameMetrics, dropCountSinceLastInvocation) -> {
Expand Down Expand Up @@ -247,9 +262,7 @@ private void stopTrackingWindow(final @NotNull Window window) {
windowFrameMetricsManager.removeOnFrameMetricsAvailableListener(
window, frameMetricsAvailableListener);
} catch (Exception e) {
options
.getLogger()
.log(SentryLevel.ERROR, "Failed to remove frameMetricsAvailableListener", e);
logger.log(SentryLevel.ERROR, "Failed to remove frameMetricsAvailableListener", e);
}
}
trackedWindows.remove(window);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -86,7 +86,14 @@ class AndroidProfilerTest {
val frameMetricsCollector: SentryFrameMetricsCollector = mock()

fun getSut(interval: Int = 1, buildInfoProvider: BuildInfoProvider = buildInfo): AndroidProfiler {
return AndroidProfiler(options.profilingTracesDirPath!!, interval, frameMetricsCollector, options, buildInfoProvider)
return AndroidProfiler(
options.profilingTracesDirPath!!,
interval,
frameMetricsCollector,
options.executorService,
options.logger,
buildInfoProvider
)
}
}

Expand Down
9 changes: 9 additions & 0 deletions sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -1782,6 +1782,15 @@ public final class io/sentry/SentryExceptionFactory {
public fun getSentryExceptionsFromThread (Lio/sentry/protocol/SentryThread;Lio/sentry/protocol/Mechanism;Ljava/lang/Throwable;)Ljava/util/List;
}

public final class io/sentry/SentryExecutorService : io/sentry/ISentryExecutorService {
public fun <init> ()V
public fun close (J)V
public fun isClosed ()Z
public fun schedule (Ljava/lang/Runnable;J)Ljava/util/concurrent/Future;
public fun submit (Ljava/lang/Runnable;)Ljava/util/concurrent/Future;
public fun submit (Ljava/util/concurrent/Callable;)Ljava/util/concurrent/Future;
}

public final class io/sentry/SentryInstantDate : io/sentry/SentryDate {
public fun <init> ()V
public fun <init> (Ljava/time/Instant;)V
Expand Down
6 changes: 4 additions & 2 deletions sentry/src/main/java/io/sentry/SentryExecutorService.java
Original file line number Diff line number Diff line change
Expand Up @@ -6,10 +6,12 @@
import java.util.concurrent.ScheduledExecutorService;
import java.util.concurrent.ThreadFactory;
import java.util.concurrent.TimeUnit;
import org.jetbrains.annotations.ApiStatus;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.TestOnly;

final class SentryExecutorService implements ISentryExecutorService {
@ApiStatus.Internal
public final class SentryExecutorService implements ISentryExecutorService {

private final @NotNull ScheduledExecutorService executorService;

Expand All @@ -18,7 +20,7 @@ final class SentryExecutorService implements ISentryExecutorService {
this.executorService = executorService;
}

SentryExecutorService() {
public SentryExecutorService() {
this(Executors.newSingleThreadScheduledExecutor(new SentryExecutorServiceThreadFactory()));
}

Expand Down
Loading