Skip to content

Commit

Permalink
Merge branch 'main' into kw/fix-handling-hybrid-unhandled-exceptions
Browse files Browse the repository at this point in the history
  • Loading branch information
krystofwoldrich committed Jun 18, 2024
2 parents 7ce635b + 2e90ac7 commit f59fbfc
Show file tree
Hide file tree
Showing 17 changed files with 142 additions and 31 deletions.
4 changes: 4 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,11 @@

### Fixes

- Move onFinishCallback before span or transaction is finished ([#3459](https://github.com/getsentry/sentry-java/pull/3459))
- Add timestamp when a profile starts ([#3442](https://github.com/getsentry/sentry-java/pull/3442))
- Move fragment auto span finish to onFragmentStarted ([#3424](https://github.com/getsentry/sentry-java/pull/3424))
- Remove profiling timeout logic and disable profiling on API 21 ([#3478](https://github.com/getsentry/sentry-java/pull/3478))
- Properly reset metric flush flag on metric emission ([#3493](https://github.com/getsentry/sentry-java/pull/3493))

## 7.10.0

Expand Down
3 changes: 2 additions & 1 deletion sentry-android-core/api/sentry-android-core.api
Original file line number Diff line number Diff line change
Expand Up @@ -81,7 +81,8 @@ public class io/sentry/android/core/AndroidProfiler$ProfileEndData {
public class io/sentry/android/core/AndroidProfiler$ProfileStartData {
public final field startCpuMillis J
public final field startNanos J
public fun <init> (JJ)V
public final field startTimestamp Ljava/util/Date;
public fun <init> (JJLjava/util/Date;)V
}

public final class io/sentry/android/core/AnrIntegration : io/sentry/Integration, java/io/Closeable {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
import android.os.Process;
import android.os.SystemClock;
import io.sentry.CpuCollectionData;
import io.sentry.DateUtils;
import io.sentry.ILogger;
import io.sentry.ISentryExecutorService;
import io.sentry.MemoryCollectionData;
Expand All @@ -17,6 +18,7 @@
import io.sentry.util.Objects;
import java.io.File;
import java.util.ArrayDeque;
import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.Map;
Expand All @@ -33,10 +35,13 @@ public class AndroidProfiler {
public static class ProfileStartData {
public final long startNanos;
public final long startCpuMillis;
public final @NotNull Date startTimestamp;

public ProfileStartData(final long startNanos, final long startCpuMillis) {
public ProfileStartData(
final long startNanos, final long startCpuMillis, final @NotNull Date startTimestamp) {
this.startNanos = startNanos;
this.startCpuMillis = startCpuMillis;
this.startTimestamp = startTimestamp;
}
}

Expand Down Expand Up @@ -190,6 +195,7 @@ public void onFrameMetricCollected(
}

profileStartNanos = SystemClock.elapsedRealtimeNanos();
final @NotNull Date profileStartTimestamp = DateUtils.getCurrentDateTime();
long profileStartCpuMillis = Process.getElapsedCpuTime();

// We don't make any check on the file existence or writeable state, because we don't want to
Expand All @@ -201,7 +207,7 @@ public void onFrameMetricCollected(
// tests)
Debug.startMethodTracingSampling(traceFile.getPath(), BUFFER_SIZE_BYTES, intervalUs);
isRunning = true;
return new ProfileStartData(profileStartNanos, profileStartCpuMillis);
return new ProfileStartData(profileStartNanos, profileStartCpuMillis, profileStartTimestamp);
} catch (Throwable e) {
endAndCollect(false, null);
logger.log(SentryLevel.ERROR, "Unable to start a profile: ", e);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@
import android.os.Build;
import android.os.Process;
import android.os.SystemClock;
import io.sentry.DateUtils;
import io.sentry.HubAdapter;
import io.sentry.IHub;
import io.sentry.ILogger;
Expand All @@ -24,6 +25,7 @@
import io.sentry.android.core.internal.util.SentryFrameMetricsCollector;
import io.sentry.util.Objects;
import java.util.ArrayList;
import java.util.Date;
import java.util.List;
import org.jetbrains.annotations.NotNull;
import org.jetbrains.annotations.Nullable;
Expand All @@ -44,6 +46,7 @@ final class AndroidTransactionProfiler implements ITransactionProfiler {
private @Nullable AndroidProfiler profiler = null;
private long profileStartNanos;
private long profileStartCpuMillis;
private @NotNull Date profileStartTimestamp;

/**
* @deprecated please use a constructor that doesn't takes a {@link IHub} instead, as it would be
Expand Down Expand Up @@ -95,6 +98,7 @@ public AndroidTransactionProfiler(
this.profilingTracesHz = profilingTracesHz;
this.executorService =
Objects.requireNonNull(executorService, "The ISentryExecutorService is required.");
this.profileStartTimestamp = DateUtils.getCurrentDateTime();
}

private void init() {
Expand Down Expand Up @@ -165,6 +169,7 @@ private boolean onFirstStart() {
}
profileStartNanos = startData.startNanos;
profileStartCpuMillis = startData.startCpuMillis;
profileStartTimestamp = startData.startTimestamp;
return true;
}

Expand Down Expand Up @@ -275,6 +280,7 @@ public synchronized void bindTransaction(final @NotNull ITransaction transaction
// done in the background when the trace file is read
return new ProfilingTraceData(
endData.traceFile,
profileStartTimestamp,
transactionList,
transactionName,
transactionId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ class ActivityLifecycleIntegrationTest {
sut.ttidSpanMap.values.first().finish()
sut.ttfdSpanMap.values.first().finish()

// then transaction should not be immediatelly finished
// then transaction should not be immediately finished
verify(fixture.hub, never())
.captureTransaction(
anyOrNull(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -178,6 +178,7 @@ class AndroidProfilerTest {
val endData = profiler.endAndCollect(false, null)
assertNotNull(startData?.startNanos)
assertNotNull(startData?.startCpuMillis)
assertNotNull(startData?.startTimestamp)
assertNotNull(endData?.endNanos)
assertNotNull(endData?.endCpuMillis)
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,12 +96,13 @@ class SentryFragmentLifecycleCallbacks(

override fun onFragmentStarted(fragmentManager: FragmentManager, fragment: Fragment) {
addBreadcrumb(fragment, FragmentLifecycleState.STARTED)

// ViewPager2 locks background fragments to STARTED state
stopTracing(fragment)
}

override fun onFragmentResumed(fragmentManager: FragmentManager, fragment: Fragment) {
addBreadcrumb(fragment, FragmentLifecycleState.RESUMED)

stopTracing(fragment)
}

override fun onFragmentPaused(fragmentManager: FragmentManager, fragment: Fragment) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -229,11 +229,11 @@ class SentryFragmentLifecycleCallbacksTest {
}

@Test
fun `When fragment is resumed, it should stop tracing if enabled`() {
fun `When fragment is started, it should stop tracing if enabled`() {
val sut = fixture.getSut(enableAutoFragmentLifecycleTracing = true)

sut.onFragmentCreated(fixture.fragmentManager, fixture.fragment, savedInstanceState = null)
sut.onFragmentResumed(fixture.fragmentManager, fixture.fragment)
sut.onFragmentStarted(fixture.fragmentManager, fixture.fragment)

verify(fixture.span).finish(
check {
Expand All @@ -243,12 +243,12 @@ class SentryFragmentLifecycleCallbacksTest {
}

@Test
fun `When fragment is resumed, it should stop tracing if enabled but keep status`() {
fun `When fragment is started, it should stop tracing if enabled but keep status`() {
val sut = fixture.getSut(enableAutoFragmentLifecycleTracing = true)

whenever(fixture.span.status).thenReturn(SpanStatus.ABORTED)
sut.onFragmentCreated(fixture.fragmentManager, fixture.fragment, savedInstanceState = null)
sut.onFragmentResumed(fixture.fragmentManager, fixture.fragment)
sut.onFragmentStarted(fixture.fragmentManager, fixture.fragment)

verify(fixture.span).finish(
check {
Expand Down
5 changes: 4 additions & 1 deletion sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -1420,7 +1420,7 @@ public final class io/sentry/ProfilingTraceData : io/sentry/JsonSerializable, io
public static final field TRUNCATION_REASON_NORMAL Ljava/lang/String;
public static final field TRUNCATION_REASON_TIMEOUT Ljava/lang/String;
public fun <init> (Ljava/io/File;Lio/sentry/ITransaction;)V
public fun <init> (Ljava/io/File;Ljava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ILjava/lang/String;Ljava/util/concurrent/Callable;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Boolean;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/util/Map;)V
public fun <init> (Ljava/io/File;Ljava/util/Date;Ljava/util/List;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;ILjava/lang/String;Ljava/util/concurrent/Callable;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/Boolean;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/lang/String;Ljava/util/Map;)V
public fun getAndroidApiLevel ()I
public fun getBuildId ()Ljava/lang/String;
public fun getCpuArchitecture ()Ljava/lang/String;
Expand All @@ -1439,6 +1439,7 @@ public final class io/sentry/ProfilingTraceData : io/sentry/JsonSerializable, io
public fun getProfileId ()Ljava/lang/String;
public fun getRelease ()Ljava/lang/String;
public fun getSampledProfile ()Ljava/lang/String;
public fun getTimestamp ()Ljava/util/Date;
public fun getTraceFile ()Ljava/io/File;
public fun getTraceId ()Ljava/lang/String;
public fun getTransactionId ()Ljava/lang/String;
Expand All @@ -1465,6 +1466,7 @@ public final class io/sentry/ProfilingTraceData : io/sentry/JsonSerializable, io
public fun setProfileId (Ljava/lang/String;)V
public fun setRelease (Ljava/lang/String;)V
public fun setSampledProfile (Ljava/lang/String;)V
public fun setTimestamp (Ljava/util/Date;)V
public fun setTraceId (Ljava/lang/String;)V
public fun setTransactionId (Ljava/lang/String;)V
public fun setTransactionName (Ljava/lang/String;)V
Expand Down Expand Up @@ -1499,6 +1501,7 @@ public final class io/sentry/ProfilingTraceData$JsonKeys {
public static final field PROFILE_ID Ljava/lang/String;
public static final field RELEASE Ljava/lang/String;
public static final field SAMPLED_PROFILE Ljava/lang/String;
public static final field TIMESTAMP Ljava/lang/String;
public static final field TRACE_ID Ljava/lang/String;
public static final field TRANSACTION_ID Ljava/lang/String;
public static final field TRANSACTION_LIST Ljava/lang/String;
Expand Down
2 changes: 2 additions & 0 deletions sentry/src/main/java/io/sentry/MetricsAggregator.java
Original file line number Diff line number Diff line change
Expand Up @@ -239,6 +239,8 @@ public void flush(boolean force) {
force = true;
}

flushScheduled = false;

final @NotNull Set<Long> flushableBuckets = getFlushableBuckets(force);
if (flushableBuckets.isEmpty()) {
logger.log(SentryLevel.DEBUG, "Metrics: nothing to flush");
Expand Down
21 changes: 21 additions & 0 deletions sentry/src/main/java/io/sentry/ProfilingTraceData.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@
import java.io.File;
import java.io.IOException;
import java.util.ArrayList;
import java.util.Date;
import java.util.HashMap;
import java.util.List;
import java.util.Locale;
Expand Down Expand Up @@ -64,6 +65,7 @@ public final class ProfilingTraceData implements JsonUnknown, JsonSerializable {
private @NotNull String profileId;
private @NotNull String environment;
private @NotNull String truncationReason;
private @NotNull Date timestamp;
private final @NotNull Map<String, ProfileMeasurement> measurementsMap;

// Stacktrace (file)
Expand All @@ -78,6 +80,7 @@ public ProfilingTraceData(
final @NotNull File traceFile, final @NotNull ITransaction transaction) {
this(
traceFile,
DateUtils.getCurrentDateTime(),
new ArrayList<>(),
transaction.getName(),
transaction.getEventId().toString(),
Expand All @@ -101,6 +104,7 @@ public ProfilingTraceData(

public ProfilingTraceData(
@NotNull File traceFile,
@NotNull Date profileStartTimestamp,
@NotNull List<ProfilingTransactionData> transactions,
@NotNull String transactionName,
@NotNull String transactionId,
Expand All @@ -120,6 +124,7 @@ public ProfilingTraceData(
@NotNull String truncationReason,
final @NotNull Map<String, ProfileMeasurement> measurementsMap) {
this.traceFile = traceFile;
this.timestamp = profileStartTimestamp;
this.cpuArchitecture = cpuArchitecture;
this.deviceCpuFrequenciesReader = deviceCpuFrequenciesReader;

Expand Down Expand Up @@ -262,6 +267,10 @@ public boolean isDeviceIsEmulator() {
return truncationReason;
}

public @NotNull Date getTimestamp() {
return timestamp;
}

public @NotNull Map<String, ProfileMeasurement> getMeasurementsMap() {
return measurementsMap;
}
Expand Down Expand Up @@ -306,6 +315,10 @@ public void setDevicePhysicalMemoryBytes(final @NotNull String devicePhysicalMem
this.devicePhysicalMemoryBytes = devicePhysicalMemoryBytes;
}

public void setTimestamp(final @NotNull Date timestamp) {
this.timestamp = timestamp;
}

public void setTruncationReason(final @NotNull String truncationReason) {
this.truncationReason = truncationReason;
}
Expand Down Expand Up @@ -386,6 +399,7 @@ public static final class JsonKeys {
public static final String SAMPLED_PROFILE = "sampled_profile";
public static final String TRUNCATION_REASON = "truncation_reason";
public static final String MEASUREMENTS = "measurements";
public static final String TIMESTAMP = "timestamp";
}

@Override
Expand Down Expand Up @@ -422,6 +436,7 @@ public void serialize(final @NotNull ObjectWriter writer, final @NotNull ILogger
writer.name(JsonKeys.SAMPLED_PROFILE).value(sampledProfile);
}
writer.name(JsonKeys.MEASUREMENTS).value(logger, measurementsMap);
writer.name(JsonKeys.TIMESTAMP).value(logger, timestamp);
if (unknown != null) {
for (String key : unknown.keySet()) {
Object value = unknown.get(key);
Expand Down Expand Up @@ -602,6 +617,12 @@ public static final class Deserializer implements JsonDeserializer<ProfilingTrac
data.measurementsMap.putAll(measurements);
}
break;
case JsonKeys.TIMESTAMP:
Date timestamp = reader.nextDateOrNull(logger);
if (timestamp != null) {
data.timestamp = timestamp;
}
break;
case JsonKeys.SAMPLED_PROFILE:
String sampledProfile = reader.nextStringOrNull();
if (sampledProfile != null) {
Expand Down
44 changes: 30 additions & 14 deletions sentry/src/main/java/io/sentry/SentryTracer.java
Original file line number Diff line number Diff line change
Expand Up @@ -200,26 +200,45 @@ public void finish(
if (!root.isFinished()
&& (!transactionOptions.isWaitForChildren() || hasAllChildrenFinished())) {

final @NotNull AtomicReference<List<PerformanceCollectionData>> performanceCollectionData =
new AtomicReference<>();
// We set the new spanFinishedCallback here instead of creation time, calling the old one to
// avoid the user overwrites it by setting a custom spanFinishedCallback on the root span
final @Nullable SpanFinishedCallback oldCallback = this.root.getSpanFinishedCallback();
this.root.setSpanFinishedCallback(
span -> {
if (oldCallback != null) {
oldCallback.execute(span);
}

// Let's call the finishCallback here, when the root span has a finished date but it's
// not finished, yet
final @Nullable TransactionFinishedCallback finishedCallback =
transactionOptions.getTransactionFinishedCallback();
if (finishedCallback != null) {
finishedCallback.execute(this);
}

if (transactionPerformanceCollector != null) {
performanceCollectionData.set(transactionPerformanceCollector.stop(this));
}
});

// any un-finished childs will remain unfinished
// as relay takes care of setting the end-timestamp + deadline_exceeded
// see
// https://github.com/getsentry/relay/blob/40697d0a1c54e5e7ad8d183fc7f9543b94fe3839/relay-general/src/store/transactions/processor.rs#L374-L378
root.finish(finishStatus.spanStatus, finishTimestamp);

List<PerformanceCollectionData> performanceCollectionData = null;
if (transactionPerformanceCollector != null) {
performanceCollectionData = transactionPerformanceCollector.stop(this);
}

ProfilingTraceData profilingTraceData = null;
if (Boolean.TRUE.equals(isSampled()) && Boolean.TRUE.equals(isProfileSampled())) {
profilingTraceData =
hub.getOptions()
.getTransactionProfiler()
.onTransactionFinish(this, performanceCollectionData, hub.getOptions());
.onTransactionFinish(this, performanceCollectionData.get(), hub.getOptions());
}
if (performanceCollectionData != null) {
performanceCollectionData.clear();
if (performanceCollectionData.get() != null) {
performanceCollectionData.get().clear();
}

hub.configureScope(
Expand All @@ -232,11 +251,6 @@ public void finish(
});
});
final SentryTransaction transaction = new SentryTransaction(this);
final TransactionFinishedCallback finishedCallback =
transactionOptions.getTransactionFinishedCallback();
if (finishedCallback != null) {
finishedCallback.execute(this);
}

if (timer != null) {
synchronized (timerLock) {
Expand Down Expand Up @@ -605,7 +619,9 @@ private boolean hasAllChildrenFinished() {
final List<Span> spans = new ArrayList<>(this.children);
if (!spans.isEmpty()) {
for (final Span span : spans) {
if (!span.isFinished()) {
// This is used in the spanFinishCallback, when the span isn't finished, but has a finish
// date
if (!span.isFinished() && span.getFinishDate() == null) {
return false;
}
}
Expand Down
Loading

0 comments on commit f59fbfc

Please sign in to comment.