Code quality getLatestActiveSpan()

[jnsLn]

Hello,

I would expect getLatestActiveSpan() (Java integration) to return the transaction itself when no active child is present and not null.

ITransaction transaction = Sentry.startTransaction("transactionName", "operation");
ISpan span = transaction .getLatestActiveSpan()

Transaction is an ISpan and is active but it returns null.

Solution:

• Return transaction when no child is present and transaction is active
• Rename to getLatestActiveChild()

[adinauer]

Hello @jnsLn. While it seems like a fair request to me it could potentially break a lot of existing code out there if we were to change it.
So if we don't want to risk that (e.g. in an upcoming major release) we could add a new method instead that does what you're suggesting.

[philipphofmann]

What is your use case for doing this, @jnsLn?

ITransaction transaction = Sentry.startTransaction("transactionName", "operation");
ISpan span = transaction .getLatestActiveSpan();

Is this not sufficient?

ITransaction transaction = Sentry.startTransaction("transactionName", "operation");
ISpan span = transaction .getLatestActiveSpan();

if (span == null) {
    span = transaction.startChild(/***/)
}

[jnsLn]

Hi @adinauer,

thanks for your answer. Fair point about breaking existing code.

@philipphofmann

It's more about how I use the transaction. I put it in my request scope / context. So when I inject it in my services it can be the first or a subsequent span.
Right now I am using a helper that just calls getLatestActiveSpan and in case it returns null I will return the transaction instead. Fairly easy solution and work's fine.

public ISpan getLatestActiveChildOrTransaction(Transaction sentryTransaction) {

	ISpan latestSpan  = sentryTransaction.getLatestActiveSpan();

	if (latestSpan != null)
		return latestSpan;

	return sentryTransaction;
}

So when I ever I inject my transaction I just call getLatestActiveSpanOrTransaction.

A function that return the latest active child or the transaction itself would make it just a bit more convenient to use.

Maybe I am just too picky or confused, but I expected a function called getLatestActiveSpan to return the transaction.

Edit: Updating the Api would be nice, but considering it is an interface the behaviour might not be consistent.

  /**
   * Returns the latest span that is not finished.
   *
   * @return span or null if not found.
   */
  @Nullable
  Span getLatestActiveSpan();
´´´ 

[philipphofmann]

Thanks for the insight, @jnsLn.

Why don't you start a child span for your transaction in all cases? Why do you want to get the latest active span? Can you elaborate on your use case, please?

[jnsLn]

@philipphofmann

Creating a span for all my transactions or doing null check both solve my problem.

I got 2 use cases for latest active span (that are basically the same):

1. We got multiple exception handler, all of them attach the exception to the last active span, finish the child spans and finish the transaction. Exception handler receive the transaction via dependency injection
2. Instead of creating spans at the Top level we create span within the actual call (otherwise we create duplicate code). So when I want to observe ServiceA.foo we create a span in ServiceA.foo and not in its caller ServiceB.bar. Changing all the signatures and provide the transaction or current span as parameter is not feasible. So we Inject the transaction and use getLastActiveSpan to create a child from the last opened span.

[philipphofmann]

all of them attach the exception to the last active span

Why do you attach the exception to the last active span? If the transaction is bound to the scope, we link captured exceptions and transactions for you.

I didn't fully understand your use case, sorry. Anyways it seems like you have a workaround.