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

4.0.0 GA - Performance Monitoring #1151

Closed
17 of 22 tasks
bruno-garcia opened this issue Jan 5, 2021 · 12 comments
Closed
17 of 22 tasks

4.0.0 GA - Performance Monitoring #1151

bruno-garcia opened this issue Jan 5, 2021 · 12 comments
Assignees
Labels
performance Performance API issues
Milestone

Comments

@bruno-garcia
Copy link
Member

bruno-garcia commented Jan 5, 2021

4.0.0 will GA the Java Performance API. That means both Android and Spring will have it, but spring includes also auto instrumentation.

Java main API:

Android:

  • Alpha: Add docs for manual instrumentation (Make sure Kotlin samples are there) Performance docs for Android sentry-docs#2851
  • Alpha: Verify it works properly with offline caching (always-on on Android)
  • On the samples in this repo, opt-in for performance with random sampling 1.0.

Spring:

Discussions

@maciejwalkowiak
Copy link
Contributor

Discuss new API to take callback and wrap the whole transaction (to be discussed on the 11th of Jan)

The proposal for this API:

Sentry.runTransaction("name", tx -> {
  // some code
});

Where runTransaction method would look like:

public static void runTransaction(String transactionName, Consumer<ITransaction> runnable) {
  ITransaction transaction = startTransaction(transactionName);
  try {
    runnable.accept(transaction);
    transaction.setStatus(SpanStatus.OK);
  } catch (Throwable e) {
    transaction.setThrowable(e);
    transaction.setStatus(SpanStatus.INTERNAL_ERROR);
  } finally {
    transaction.finish();
  }
}

With such simple API we don't give a an option to set different status on error, but it can be a good enough tradeoff.

@bruno-garcia
Copy link
Member Author

Do we bind the transaction to the scope to integrations can find it? Seems like that could be done here, if we push a scope in there so it has its own isolated scope.

That would work on Java but not on Android (global hub mode).

We can add an overload that takes a configuration object later if we want to extend this API but I think by default we should push a scope and bind the transaction to it.

Which remind me that we bind it automatically and that needs to be removed, or at least configurable, and default false.

@maciejwalkowiak
Copy link
Contributor

We can add an overload that takes a configuration object later if we want to extend this API but I think by default we should push a scope and bind the transaction to it.

Which remind me that we bind it automatically and that needs to be removed, or at least configurable, and default false.

Please elaborate. Currently when startTransaction is called we do stack.peek().getScope().setTransaction(transaction);, making this configurable on the startTransaction method level would make the API clumsy as we would need to add bunch of new overloaded methods.

@marandaneto
Copy link
Contributor

marandaneto commented Jan 7, 2021

@bruno-garcia can we close #1130 and use only this one? checkbox items will keep out of sync, I prefer a single source of truth.

Or to remove the Android items from here

@bruno-garcia
Copy link
Member Author

Oh I just did it but realized that was for an alpha.

@marandaneto I'll just add the 2 points from that issue in here with a note (for the alpha) so we can focus on those for the next milestone (Android Performance Alpha release)

@marandaneto marandaneto added the performance Performance API issues label Jan 8, 2021
@marandaneto
Copy link
Contributor

Oh I just did it but realized that was for an alpha.

@marandaneto I'll just add the 2 points from that issue in here with a note (for the alpha) so we can focus on those for the next milestone (Android Performance Alpha release)

urgh, yeah also missed the alpha subtitle, thx

@marandaneto
Copy link
Contributor

@bruno-garcia what do you mean by Opt-in to the Java/Kotlin samples for performance? wouldn't it be the same as https://sentry-docs-git-feat-android-performance.sentry.dev/platforms/android/performance/ ?

@bruno-garcia
Copy link
Member Author

@marandaneto I reworded it: Let's turn performance monitoring on on all our samples in this repo?

@maciejwalkowiak
Copy link
Contributor

Clarify differences in API to align with .NET (and help build Flutter and iOS that come next): getsentry/sentry-dotnet#663 (PR: #1165)

Is there anything more we should clarify?

@bruno-garcia
Copy link
Member Author

Clarify differences in API to align with .NET (and help build Flutter and iOS that come next): getsentry/sentry-dotnet#663 (PR: #1165)

Is there anything more we should clarify?

Checked the check box 🥳 we're close eh

@bruno-garcia
Copy link
Member Author

We've GA'ed and everything that was required is done. Follow up tasks linked can be done on their own now.

@dashed
Copy link
Member

dashed commented Feb 1, 2021

@bruno-garcia 🎉

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
performance Performance API issues
Projects
None yet
Development

No branches or pull requests

4 participants