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

Add Continuous Profiling support (v8) #3710

Open
wants to merge 9 commits into
base: 8.x.x
Choose a base branch
from

Conversation

stefanosiano
Copy link
Member

@stefanosiano stefanosiano commented Sep 23, 2024

📜 Description

This is the first part for the continuous profiling support for Android.
Internal doc is here

Part 1
Add the implementation of the continuous profiler itself

Part 2 -> #3711
Create new envelope type and payload
Create and send profile_chunk envelope

Part 3 -> #3725
Instantiate profiler
Set ProfileContext to transactions and spans
Add performance collectors to the profiler

Part 4 -> #3730
Handle app start profiling

Part 5 -> #3844
Add new user-facing APIs

Part 6 -> #3926
Handle rate limiting

Part 7
Update docs
User docs -> getsentry/sentry-docs#11872

💡 Motivation and Context

💚 How did you test it?

Unit tests

📝 Checklist

  • I reviewed the submitted code.
  • I added tests to verify the changes.
  • No new PII added or SDK only sends newly added PII if sendDefaultPII is enabled.
  • I updated the docs if needed.
  • Review from the native team if needed.
  • No breaking change or entry added to the changelog.
  • No breaking change for hybrid SDKs or communicated to hybrid SDKs.

🔮 Next steps

made AndroidProfiler's executor nullable, as timeout will be handled differently for continuous profiling
@stefanosiano stefanosiano changed the title Add Continuous Profiling support Add Continuous Profiling support (v8) Sep 23, 2024
Copy link
Contributor

github-actions bot commented Sep 23, 2024

Fails
🚫 Please consider adding a changelog entry for the next release.
Messages
📖 Do not forget to update Sentry-docs with your feature once the pull request gets approved.

Instructions and example for changelog

Please add an entry to CHANGELOG.md to the "Unreleased" section. Make sure the entry includes this PR's number.

Example:

## Unreleased

- Add Continuous Profiling support (v8) ([#3710](https://github.com/getsentry/sentry-java/pull/3710))

If none of the above apply, you can opt out of this check by adding #skip-changelog to the PR description.

Generated by 🚫 dangerJS against b0e8333

Copy link
Contributor

github-actions bot commented Sep 23, 2024

Performance metrics 🚀

  Plain With Sentry Diff
Startup time 392.98 ms 453.73 ms 60.76 ms
Size 1.65 MiB 2.31 MiB 680.97 KiB

@stefanosiano stefanosiano marked this pull request as ready for review September 23, 2024 10:46
# Conflicts:
#	sentry-android-core/src/main/java/io/sentry/android/core/AndroidProfiler.java
#	sentry-android-core/src/main/java/io/sentry/android/core/AndroidTransactionProfiler.java
# Conflicts:
#	sentry-android-core/src/main/java/io/sentry/android/core/AndroidProfiler.java
* added profile_chunk envelope create
* added IHub.captureProfileChunk and ISentryClient.captureProfileChunk
* added profilerId and chunkId reset logic to AndroidContinuousProfiler
* added absolute timestamps to ProfileMeasurementValue
* added ProfileContext to Contexts
* removed timestampMillis from MemoryCollectionData and CpuCollectionData, now it uses timestamp.nanotime() to achieve same result
* continuous profiler doesn't stop anymore when an error occurs, but continue scheduling restart

Instantiate continuous profiling v8 (p3) (#3725)
* added profile context to SentryTracer
* removed isProfilingEnabled from AndroidContinuousProfiler, as it's useless
* added continuous profiler to SentryOptions
* added DefaultTransactionPerformanceCollector to AndroidContinuousProfiler
* updated DefaultTransactionPerformanceCollector to work with string ids other than transactions
* fixed ProfileChunk measurements being modifiable from other code
* added thread id and name to SpanContext.data
* added profiler_id to span data
* close continuous profiler on scopes close
* renamed TransactionPerformanceCollector to CompositePerformanceCollector
* added SpanContext.data ser/deser

Handle App Start Continuous Profiling v8 (p4) (#3730)
* create app start continuous profiler instead of transaction profiler, based on config
* updated SentryAppStartProfilingOptions with isContinuousProfilingEnabled flag
* updated SentryOptions with isContinuousProfilingEnabled() method
* cut profiler setup out in a specific function to improve readability of AndroidOptionsInitializer

Add new APIs for Continuous Profiling v8 (p5) (#3844)
* AndroidContinuousProfiler now retrieve the scopes on start()
* removed profilesSampleRate from sample app to enable continuous profiling
* added Sentry.startProfiler and Sentry.stopProfiler APIs
# Conflicts:
#	sentry-opentelemetry/sentry-opentelemetry-bootstrap/src/main/java/io/sentry/opentelemetry/OtelSpanFactory.java
#	sentry-opentelemetry/sentry-opentelemetry-extra/api/sentry-opentelemetry-extra.api
* continuous profiler now doesn't start if offline or rate limited
* continuous profiler stops when rate limited
* continuous profiler prevents sending chunks after being closed
* added profile_chunk rate limit
* continuous profiler now reset its id when rate limited or offline
Copy link
Contributor

github-actions bot commented Dec 6, 2024

🚨 Detected changes in high risk code 🚨

High-risk code has higher potential to break the SDK and may be hard to test. To prevent severe bugs, apply the rollout process for releasing such changes and be extra careful when changing and reviewing these files:

  • sentry/src/main/java/io/sentry/transport/RateLimiter.java

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.

2 participants