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

Open up Transaction Factory methods #2640

Closed
kylannjohnson opened this issue Apr 5, 2023 · 9 comments · Fixed by #2701
Closed

Open up Transaction Factory methods #2640

kylannjohnson opened this issue Apr 5, 2023 · 9 comments · Fixed by #2701

Comments

@kylannjohnson
Copy link

Problem Statement

Perhaps there is a workaround, but I would like to create a slight variation of ActivityLifecycleIntegration.

It seems that this integration is the way Sentry creates the App Start Span that times app start up before the first activity is created and the integration callback is issued. This integration calls the method below on the Hub to use TransactionOption to set the start time of the transaction to that of the App Start Span.

@ApiStatus.Internal
  @NotNull
  ITransaction startTransaction(
      final @NotNull TransactionContext transactionContext,
      final @NotNull TransactionOptions transactionOptions);

Solution Brainstorm

Rather than the App Start Span being attached to a transaction for ui.load, and thus attached to the first activity, I'd like it to be attached to a transaction that I dictate. I currently have a scoped transaction that I think will "block" the ui.load one from posting so I can't use this integration as-is (unless I missed something).

As you can see above, the @ApiStatus of the method is internal (as is TransactionOptions) and I don't want to depend on internal APIs. Is there a workaround you could suggest or do you have any interesting in publicizing these methods and types? Or perhaps could we separate out the integrations?

@romtsn
Copy link
Member

romtsn commented Apr 6, 2023

Hi @kylannjohnson, would you mind sharing a bit more details into how exactly you'd like to use this method? Just want to better understand your usecase, to see if we are potentially doing something wrong in ActivityLifecycleIntegration and should rather alter the default behavior to suit your needs.

Is it that you want to set the start timestamp of your custom transaction to the actual app startup timestamp?

@kylannjohnson
Copy link
Author

Hey @romtsn. Thanks for the question. I suppose exposing the method is one way to solve my issue, but there could be others.

I got the idea from inspecting the ActivityLifecycleIntegration code, but stubbled upon it being ApiStatus.Internal when I was attempting to perform the same action in my own integration. What I was attempting to do was attach the same Cold/Warm start span that the integration creates to one of my transactions (instead of a ui.load transaction). I like the method the integration uses of capturing a timestamp and then creating a child Span to capture a time before the SDK was initialized. However, our team doesn't want to focus on Activity transitions. Instead we want to focus on the operations that initialize and launch our application.

Said another way, I'd want the "first' activity to indicate the end of the cold start (just like the integration), but I don't want to have to enable ui.load transactions to do it. We have a transaction that captures "app launch", and are looking at adding a cold start metric to it. If possible, I'd love to use a supported SDK API rather than a custom one and certainly not one that uses Internal APIs.

@kahest
Copy link
Member

kahest commented Apr 11, 2023

Hey @kylannjohnson, thank you for the detailled response! We'll look into, maybe there's even a way to make a broader use case out of this. That said, please be aware that most of our Android engineers are at KotlinConf Amsterdam this week, so replies may be a bit slower than usual. Thanks!

@kylannjohnson
Copy link
Author

@kahest No problem. Let me know if I can help making anything clearer. I'm happy to provide feedback.

@romtsn romtsn moved this from Needs Discussion to Backlog in Mobile & Cross Platform SDK Apr 19, 2023
@romtsn
Copy link
Member

romtsn commented Apr 21, 2023

hi @kylannjohnson sorry for the delayed reply. I think your usecase makes total sense and we should expose the method, but I guess what you're looking for is rather this method:

ISpan startChild(
@NotNull String operation,
@Nullable String description,
@Nullable SentryDate timestamp,
@NotNull Instrumenter instrumenter);

because you mentioned you want to set start time of a span and not a transaction right?

The problem with spans is that they are not indexed, so if you need something beyond simple span visualization, then I'd recommend you to also/instead send it as metric. It will show up in the transaction details like this, and you will also be able to search for it in discover. In this case we won't need to expose anything.
image

You can look up how we set the metric ourselves here:

if (!sentStartMeasurement && hasAppStartSpan(transaction.getSpans())) {
final Long appStartUpInterval = AppStartState.getInstance().getAppStartInterval();
// if appStartUpInterval is null, metrics are not ready to be sent
if (appStartUpInterval != null) {
final MeasurementValue value =
new MeasurementValue(
(float) appStartUpInterval, MeasurementUnit.Duration.MILLISECOND.apiName());
final String appStartKey =
AppStartState.getInstance().isColdStart()
? MeasurementValue.KEY_APP_START_COLD
: MeasurementValue.KEY_APP_START_WARM;
transaction.getMeasurements().put(appStartKey, value);
sentStartMeasurement = true;
}
}

Let me know if this is enough for you or we should look into exposing the method rather sooner than later. Thanks!

@lbloder lbloder self-assigned this Apr 27, 2023
@markushi
Copy link
Member

Let's start simple and add APIs to alter the start/end timestamps.

@kylannjohnson
Copy link
Author

Hey All. @romtsn , yes, that's the method I end up using. However, in my case, I think I'm altering both the start of a Span and the start of the transaction because I'm basically setting the root span of the transaction. (Or, I'm setting the "oldest" Span in the transaction).

This means that I had to use the Hub method to start a Transaction with a context

 @ApiStatus.Internal
  @NotNull
  ITransaction startTransaction(
      final @NotNull TransactionContext transactionContext,
      final @NotNull TransactionOptions transactionOptions);

That would set the start time of the transaction via the Context... then you could use the method you mention

        // context set above in this example
        val transaction = startTransaction(context, options)

        transaction.startChild(
            APP_START_COLD_OPERATION_NAME,
            APP_START_COLD_DESCRIPTION,
            appStartTime,
            SENTRY
        ).finish(OK, appStartEndTime)  // Immediately finish span (not transaction)

@kylannjohnson
Copy link
Author

Also, we capture App Start as a measurement, just as you say. But, we use one of the SDK's EventProcessors to turn the app.start.cold span into the app_start_cold measurement.

// the app start measurement is only sent once and only if the transaction has
// the app.start span, which is automatically created by the SDK.
if (!sentStartMeasurement && hasAppStartSpan(transaction.getSpans())) {
final Long appStartUpInterval = AppStartState.getInstance().getAppStartInterval();
// if appStartUpInterval is null, metrics are not ready to be sent
if (appStartUpInterval != null) {
final MeasurementValue value =
new MeasurementValue(
(float) appStartUpInterval, MeasurementUnit.Duration.MILLISECOND.apiName());
final String appStartKey =
AppStartState.getInstance().isColdStart()
? MeasurementValue.KEY_APP_START_COLD
: MeasurementValue.KEY_APP_START_WARM;
transaction.getMeasurements().put(appStartKey, value);
sentStartMeasurement = true;
}
}

@romtsn
Copy link
Member

romtsn commented May 2, 2023

@kylannjohnson alright, thanks for the details! we'll have to make both of the aforementioned methods public then cc @lbloder

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
Archived in project
Development

Successfully merging a pull request may close this issue.

5 participants