Feat: Perf. for fragments

[marandaneto]

📜 Description

Feat: Perf. for fragments

💡 Motivation and Context

Able to start spans out of fragments

💚 How did you test it?

📝 Checklist

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

🔮 Next steps

[codecov-commenter]

Codecov Report

Merging #1528 (d745940) into main (e7fb5c3) will not change coverage.
The diff coverage is 66.66%.

@@            Coverage Diff            @@
##               main    #1528   +/-   ##
=========================================
  Coverage     76.03%   76.03%           
  Complexity     1955     1955           
=========================================
  Files           192      192           
  Lines          6765     6765           
  Branches        675      675           
=========================================
  Hits           5144     5144           
  Misses         1294     1294           
  Partials        327      327           

Impacted Files	Coverage Δ
sentry/src/main/java/io/sentry/IHub.java	90.47% <66.66%> (ø)

---

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 e7fb5c3...d745940. Read the comment docs.

[bruno-garcia]

If we use the accessor like I suggested above, it would affect this too given the new hub could have perf enabled but the first not. So technically we'd need to evaluate this on each call hubAcessor().options....

Unless we get a new instance of this class for each Hub (each Sentry.init) in which case it would be fine we bind to the on Hub and disable when the hub closes.

[philipphofmann]

Good catch @bruno-garcia. I think this would to the trick

Suggested change

	private val isPerformanceEnabled = hub.options.isTracingEnabled && enableAutoFragmentLifecycleTracing
	private val isPerformanceEnabled get() = hub.options.isTracingEnabled && enableAutoFragmentLifecycleTracing

[marandaneto]

its a new instance per activity, as the callback is bound to the fragment manager, and each activity has its own fragmentmanager, I guess its the same as #1528 (comment)
so in the case that a new activity is setup and isTracingEnabled is disabled, nothing is gonna happen.

[philipphofmann]

On Cocoa, we only track spans until onResume is called because otherwise, the duration of a transaction would vary vastly. We could also create spans for onCreate, onStart and onResume. Then you get more insight into how long each of the lifecycle methods took.

[philipphofmann]

Good catch @bruno-garcia. I think this would to the trick

Suggested change

	private val isPerformanceEnabled = hub.options.isTracingEnabled && enableAutoFragmentLifecycleTracing
	private val isPerformanceEnabled get() = hub.options.isTracingEnabled && enableAutoFragmentLifecycleTracing

[philipphofmann]

h: If isPerformanceEnabled is evaluated on each call it could be that it is disabled here and never finish the span. If isPerformanceEnabled is set to false and we have a running span we could just discard the span.

[marandaneto]

you cannot mutate the isPerformanceEnabled after the SDK is inited, so I don't see the point of taking care of this corner case, the same happens on ActivityLifecycleIntegration

[philipphofmann]

Couldn't you do some crazy stuff like this?

public class MyApplication extends Application {

  private SentryOptions myOptions;

  @Override
  public void onCreate() {
    super.onCreate();
    
    SentryAndroid.init(
        this,
        options -> {
          myOptions = options;
        });

    // some time passes
    myOptions.setTracesSampleRate(null);
  }
}

[marandaneto]

you could, yes, this would be misusing the API hence not really a case that we should take care, if we take this option in consideration, there are N cases that would break the whole SDK, or using reflection and setting non null fields to null etc

[marandaneto]

also init should be called only once, its part of the spec.

[marandaneto]

On Cocoa, we only track spans until onResume is called because otherwise, the duration of a transaction would vary vastly. We could also create spans for onCreate, onStart and onResume. Then you get more insight into how long each of the lifecycle methods took.

the idea is that first, we measure the impact of adding spans for every single lifecycle method and later expand upon feedback, fragment has like 12 lifecycle methods, it's too noisy

[philipphofmann]

the idea is that first, we measure the impact of adding spans for every single lifecycle method and later expand upon feedback, fragment has like 12 lifecycle methods, it's too noisy

Didn't know that. We currently don't have much "noise" on Android, so adding some makes sense to me as performance is about getting more insight. But I'm fine with the let's see how it goes approach.

[marandaneto]

the idea is that first, we measure the impact of adding spans for every single lifecycle method and later expand upon feedback, fragment has like 12 lifecycle methods, it's too noisy

Didn't know that. We currently don't have much "noise" on Android, so adding some makes sense to me as performance is about getting more insight. But I'm fine with the let's see how it goes approach.

we've discussed this for Activity instrumentation, I'm just taking the same approach.
the difference is also that on Activitys and Fragments lifecycle, most of the work, if not only, is in the onCreate method, nothing else.

[bruno-garcia]