Skip to content

Commit

Permalink
Merge a7fe761 into 19fca04
Browse files Browse the repository at this point in the history
  • Loading branch information
markushi authored Jul 25, 2023
2 parents 19fca04 + a7fe761 commit 68087ab
Show file tree
Hide file tree
Showing 8 changed files with 215 additions and 45 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -192,6 +192,8 @@ private void startTracing(final @NotNull Activity activity) {

final TransactionOptions transactionOptions = new TransactionOptions();
if (options.isEnableActivityLifecycleTracingAutoFinish()) {
transactionOptions.setDeadlineTimeout(
TransactionOptions.SENTRY_AUTO_TRANSACTION_DEADLINE_MS);
transactionOptions.setIdleTimeout(options.getIdleTimeout());
transactionOptions.setTrimEnd(true);
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -236,6 +236,7 @@ private void startTracing(final @NotNull UiElement target, final @NotNull String

final TransactionOptions transactionOptions = new TransactionOptions();
transactionOptions.setWaitForChildren(true);
transactionOptions.setDeadlineTimeout(TransactionOptions.SENTRY_AUTO_TRANSACTION_DEADLINE_MS);
transactionOptions.setIdleTimeout(options.getIdleTimeout());
transactionOptions.setTrimEnd(true);

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,7 @@ class ActivityLifecycleIntegrationTest {
}

@Test
fun `Transaction op is ui_load`() {
fun `Transaction op is ui_load and idle+deadline timeouts are set`() {
val sut = fixture.getSut()
fixture.options.tracesSampleRate = 1.0
sut.register(fixture.hub, fixture.options)
Expand All @@ -365,11 +365,14 @@ class ActivityLifecycleIntegrationTest {
sut.onActivityCreated(activity, fixture.bundle)

verify(fixture.hub).startTransaction(
check {
check<TransactionContext> {
assertEquals("ui.load", it.operation)
assertEquals(TransactionNameSource.COMPONENT, it.transactionNameSource)
},
any<TransactionOptions>()
check<TransactionOptions> { transactionOptions ->
assertEquals(fixture.options.idleTimeout, transactionOptions.idleTimeout)
assertEquals(TransactionOptions.SENTRY_AUTO_TRANSACTION_DEADLINE_MS, transactionOptions.deadlineTimeout)
}
)
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -203,6 +203,24 @@ class SentryGestureListenerTracingTest {
)
}

@Test
fun `captures transaction and both idle+deadline timeouts are set`() {
val sut = fixture.getSut<View>()

sut.onSingleTapUp(fixture.event)

verify(fixture.hub).startTransaction(
any<TransactionContext>(),
check<TransactionOptions> { transactionOptions ->
assertEquals(fixture.options.idleTimeout, transactionOptions.idleTimeout)
assertEquals(
TransactionOptions.SENTRY_AUTO_TRANSACTION_DEADLINE_MS,
transactionOptions.deadlineTimeout
)
}
)
}

@Test
fun `captures transaction with interaction event type as op`() {
val sut = fixture.getSut<View>()
Expand Down
3 changes: 3 additions & 0 deletions sentry/api/sentry.api
Original file line number Diff line number Diff line change
Expand Up @@ -2297,15 +2297,18 @@ public abstract interface class io/sentry/TransactionFinishedCallback {
}

public final class io/sentry/TransactionOptions : io/sentry/SpanOptions {
public static final field SENTRY_AUTO_TRANSACTION_DEADLINE_MS J
public fun <init> ()V
public fun getCustomSamplingContext ()Lio/sentry/CustomSamplingContext;
public fun getDeadlineTimeout ()Ljava/lang/Long;
public fun getIdleTimeout ()Ljava/lang/Long;
public fun getStartTimestamp ()Lio/sentry/SentryDate;
public fun getTransactionFinishedCallback ()Lio/sentry/TransactionFinishedCallback;
public fun isBindToScope ()Z
public fun isWaitForChildren ()Z
public fun setBindToScope (Z)V
public fun setCustomSamplingContext (Lio/sentry/CustomSamplingContext;)V
public fun setDeadlineTimeout (Ljava/lang/Long;)V
public fun setIdleTimeout (Ljava/lang/Long;)V
public fun setStartTimestamp (Lio/sentry/SentryDate;)V
public fun setTransactionFinishedCallback (Lio/sentry/TransactionFinishedCallback;)V
Expand Down
132 changes: 99 additions & 33 deletions sentry/src/main/java/io/sentry/SentryTracer.java
Original file line number Diff line number Diff line change
Expand Up @@ -37,10 +37,14 @@ public final class SentryTracer implements ITransaction {
*/
private @NotNull FinishStatus finishStatus = FinishStatus.NOT_FINISHED;

private volatile @Nullable TimerTask timerTask;
private volatile @Nullable TimerTask idleTimeoutTask;
private volatile @Nullable TimerTask deadlineTimeoutTask;

private volatile @Nullable Timer timer = null;
private final @NotNull Object timerLock = new Object();
private final @NotNull AtomicBoolean isFinishTimerRunning = new AtomicBoolean(false);

private final @NotNull AtomicBoolean isIdleFinishTimerRunning = new AtomicBoolean(false);
private final @NotNull AtomicBoolean isDeadlineTimerRunning = new AtomicBoolean(false);

private final @NotNull Baggage baggage;
private @NotNull TransactionNameSource transactionNameSource;
Expand Down Expand Up @@ -92,7 +96,8 @@ public SentryTracer(
transactionPerformanceCollector.start(this);
}

if (transactionOptions.getIdleTimeout() != null) {
if (transactionOptions.getIdleTimeout() != null
|| transactionOptions.getDeadlineTimeout() != null) {
timer = new Timer(true);
scheduleFinish();
}
Expand All @@ -101,34 +106,71 @@ public SentryTracer(
@Override
public void scheduleFinish() {
synchronized (timerLock) {
cancelTimer();
cancelIdleTimer();
cancelDeadlineTimer();

if (timer != null) {
isFinishTimerRunning.set(true);
timerTask =
new TimerTask() {
@Override
public void run() {
finishFromTimer();
}
};

try {
timer.schedule(timerTask, transactionOptions.getIdleTimeout());
} catch (Throwable e) {
hub.getOptions()
.getLogger()
.log(SentryLevel.WARNING, "Failed to schedule finish timer", e);
// if we failed to schedule the finish timer for some reason, we finish it here right away
finishFromTimer();
final @Nullable Long idleTimeout = transactionOptions.getIdleTimeout();
final @Nullable Long deadlineTimeOut = transactionOptions.getDeadlineTimeout();

if (idleTimeout != null) {
isIdleFinishTimerRunning.set(true);
idleTimeoutTask =
new TimerTask() {
@Override
public void run() {
onIdleTimeoutReached();
}
};

try {
timer.schedule(idleTimeoutTask, idleTimeout);
} catch (Throwable e) {
hub.getOptions()
.getLogger()
.log(SentryLevel.WARNING, "Failed to schedule finish timer", e);
// if we failed to schedule the finish timer for some reason, we finish it here right
// away
onIdleTimeoutReached();
}
}
if (deadlineTimeOut != null) {
isDeadlineTimerRunning.set(true);
deadlineTimeoutTask =
new TimerTask() {
@Override
public void run() {
onDeadlineTimeoutReached();
}
};

try {
timer.schedule(deadlineTimeoutTask, deadlineTimeOut);
} catch (Throwable e) {
hub.getOptions()
.getLogger()
.log(SentryLevel.WARNING, "Failed to schedule finish timer", e);
// if we failed to schedule the finish timer for some reason, we finish it here right
// away
onDeadlineTimeoutReached();
}
}
}
}
}

private void finishFromTimer() {
final SpanStatus status = getStatus();
private void onIdleTimeoutReached() {
final @Nullable SpanStatus status = getStatus();
finish((status != null) ? status : SpanStatus.OK);
isFinishTimerRunning.set(false);
isIdleFinishTimerRunning.set(false);
}

private void onDeadlineTimeoutReached() {
final @Nullable SpanStatus status = getStatus();
forceFinish(
(status != null) ? status : SpanStatus.DEADLINE_EXCEEDED,
transactionOptions.getIdleTimeout() != null);
isDeadlineTimerRunning.set(false);
}

@Override
Expand Down Expand Up @@ -222,6 +264,8 @@ public void finish(
if (timer != null) {
synchronized (timerLock) {
if (timer != null) {
cancelIdleTimer();
cancelDeadlineTimer();
timer.cancel();
timer = null;
}
Expand All @@ -244,12 +288,22 @@ public void finish(
}
}

private void cancelTimer() {
private void cancelIdleTimer() {
synchronized (timerLock) {
if (timerTask != null) {
timerTask.cancel();
isFinishTimerRunning.set(false);
timerTask = null;
if (idleTimeoutTask != null) {
idleTimeoutTask.cancel();
isIdleFinishTimerRunning.set(false);
idleTimeoutTask = null;
}
}
}

private void cancelDeadlineTimer() {
synchronized (timerLock) {
if (deadlineTimeoutTask != null) {
deadlineTimeoutTask.cancel();
isDeadlineTimerRunning.set(false);
deadlineTimeoutTask = null;
}
}
}
Expand Down Expand Up @@ -360,7 +414,7 @@ private ISpan createChild(

Objects.requireNonNull(parentSpanId, "parentSpanId is required");
Objects.requireNonNull(operation, "operation is required");
cancelTimer();
cancelIdleTimer();
final Span span =
new Span(
root.getTraceId(),
Expand Down Expand Up @@ -720,8 +774,14 @@ Span getRoot() {

@TestOnly
@Nullable
TimerTask getTimerTask() {
return timerTask;
TimerTask getIdleTimeoutTask() {
return idleTimeoutTask;
}

@TestOnly
@Nullable
TimerTask getDeadlineTimeoutTask() {
return deadlineTimeoutTask;
}

@TestOnly
Expand All @@ -733,7 +793,13 @@ Timer getTimer() {
@TestOnly
@NotNull
AtomicBoolean isFinishTimerRunning() {
return isFinishTimerRunning;
return isIdleFinishTimerRunning;
}

@TestOnly
@NotNull
AtomicBoolean isDeadlineTimerRunning() {
return isDeadlineTimerRunning;
}

@TestOnly
Expand Down
32 changes: 32 additions & 0 deletions sentry/src/main/java/io/sentry/TransactionOptions.java
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
/** Sentry Transaction options */
public final class TransactionOptions extends SpanOptions {

public static final long SENTRY_AUTO_TRANSACTION_DEADLINE_MS = 300000;

/**
* Arbitrary data used in {@link SamplingContext} to determine if transaction is going to be
* sampled.
Expand Down Expand Up @@ -34,6 +36,16 @@ public final class TransactionOptions extends SpanOptions {
*/
private @Nullable Long idleTimeout = null;

/**
* The deadline time, measured in ms, to wait until the transaction will be force-finished with
* deadline-exceeded status./
*
* <p>When set to {@code null} the transaction won't be forcefully finished.
*
* <p>The default is 30 seconds.
*/
private @Nullable Long deadlineTimeout = null;

/**
* When `waitForChildren` is set to `true` and this callback is set, it's called before the
* transaction is captured.
Expand Down Expand Up @@ -121,6 +133,26 @@ public void setWaitForChildren(boolean waitForChildren) {
return idleTimeout;
}

/**
* Sets the deadlineTimeout. If set, an transaction and it's child spans will be force-finished
* with status {@link SpanStatus#DEADLINE_EXCEEDED} in case the transaction isn't finished in
* time.
*
* @param deadlineTimeoutMs - the deadlineTimeout, in ms - or null if no deadline should be set
*/
public void setDeadlineTimeout(@Nullable Long deadlineTimeoutMs) {
this.deadlineTimeout = deadlineTimeoutMs;
}

/**
* Gets the deadlineTimeout
*
* @return deadlineTimeout - the deadlineTimeout, in ms - or null if no deadline is set
*/
public @Nullable Long getDeadlineTimeout() {
return deadlineTimeout;
}

/**
* Sets the idleTimeout
*
Expand Down
Loading

0 comments on commit 68087ab

Please sign in to comment.