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

AndroidTransactionProfiler is now initialized the first time a transa… #2009

Merged
merged 5 commits into from
Apr 28, 2022

Conversation

stefanosiano
Copy link
Member

@stefanosiano stefanosiano commented Apr 26, 2022

📜 Description

AndroidTransactionProfiler is now initialized the first time a transaction is started, since in the constructor we don't have the options, yet

💡 Motivation and Context

When AndroidTransactionProfiler is created and set in the options, in the AndroidOptionsInitializer, we don't know what options were set through SentryAndroid.init() yet, as the profiler is created right before it.
We can set the internal variables of the profiler that depends on the options on the first profile, as we would be sure to already have all options set up.

💚 How did you test it?

I'm building some ui tests (that will be released in another pr)

📝 Checklist

  • I reviewed the submitted code
  • I added tests to verify the changes
  • I updated the docs if needed
  • No breaking changes

…ction is started, since in the constructor we don't have the options, yet
…ction is started, since in the constructor we don't have the options, yet
@codecov-commenter
Copy link

codecov-commenter commented Apr 26, 2022

Codecov Report

Merging #2009 (8bacc67) into 6.x.x (2aa0338) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##              6.x.x    #2009   +/-   ##
=========================================
  Coverage     80.79%   80.79%           
  Complexity     3131     3131           
=========================================
  Files           228      228           
  Lines         11619    11619           
  Branches       1555     1555           
=========================================
  Hits           9387     9387           
  Misses         1648     1648           
  Partials        584      584           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2aa0338...8bacc67. Read the comment docs.

@marandaneto
Copy link
Contributor

@stefanosiano would you mind using the PR template to explain why this is needed?

@stefanosiano
Copy link
Member Author

@marandaneto

@stefanosiano would you mind using the PR template to explain why this is needed?

Yeah, you are right. I wrote it in the description, but i added the template now

@marandaneto
Copy link
Contributor

@stefanosiano We always add a test for a regression, would you mind adding it? thanks.

@stefanosiano stefanosiano merged commit 9f69a65 into 6.x.x Apr 28, 2022
@stefanosiano stefanosiano deleted the feat/profiling-options-fixes branch April 28, 2022 13:22
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants