-
-
Notifications
You must be signed in to change notification settings - Fork 458
Profiling - OTEL profiling fix, Stabilization, Logging #4746
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
Profiling - OTEL profiling fix, Stabilization, Logging #4746
Conversation
…ling and handle cases where profiling has been started by otel
…te this to the exporter and SentryTracer
…ion exceptions in provider
🚨 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:
|
Instructions and example for changelogPlease add an entry to Example: ## Unreleased
- Profiling - OTEL profiling fix, Stabilization, Logging ([#4746](https://github.com/getsentry/sentry-java/pull/4746)) If none of the above apply, you can opt out of this check by adding |
Performance metrics 🚀
|
Revision | Plain | With Sentry | Diff |
---|---|---|---|
42206f4 | 419.73 ms | 494.82 ms | 75.09 ms |
066d89d | 377.51 ms | 434.20 ms | 56.69 ms |
d6f8356 | 392.08 ms | 454.42 ms | 62.34 ms |
7dcbbbd | 459.22 ms | 509.85 ms | 50.62 ms |
f2cd43a | 415.48 ms | 485.28 ms | 69.80 ms |
App size
Revision | Plain | With Sentry | Diff |
---|---|---|---|
42206f4 | 1.58 MiB | 2.09 MiB | 521.84 KiB |
066d89d | 1.58 MiB | 2.09 MiB | 521.85 KiB |
d6f8356 | 1.58 MiB | 2.09 MiB | 521.68 KiB |
7dcbbbd | 1.58 MiB | 2.09 MiB | 521.68 KiB |
f2cd43a | 1.58 MiB | 2.09 MiB | 521.84 KiB |
🚨 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:
|
…entry-java into feat/continuous-profiling-04
🚨 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:
|
🚨 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 review |
cursor review |
.../src/main/java/io/sentry/asyncprofiler/convert/JfrAsyncProfilerToSentryProfileConverter.java
Show resolved
Hide resolved
...y-async-profiler/src/main/java/io/sentry/asyncprofiler/profiling/JavaContinuousProfiler.java
Show resolved
Hide resolved
...y-async-profiler/src/main/java/io/sentry/asyncprofiler/profiling/JavaContinuousProfiler.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
val minTimestamp = samples.minOf { it.timestamp } | ||
val maxTimestamp = samples.maxOf { it.timestamp } | ||
val sampleTimeStamp = | ||
DateUtils.nanosToDate((maxTimestamp * 1000 * 1000 * 1000).toLong()).toInstant() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
l
Could use DateUtils.secondsToNanos
...rc/test/java/io/sentry/asyncprofiler/convert/JfrAsyncProfilerToSentryProfileConverterTest.kt
Show resolved
Hide resolved
* add readme and info about commit of the source repository * delete jfr file on jvm exit * further split into smaller methods * deduplicate frames in order to save bandwidth, add converter tests * remove Platform Enum, use string constants instead for compatibility with cross platform frameworks * implement equals and hashcode for SentryStackFrame to make frame deduplication work * bump api * improve error handling, fix start stop start flow * add new testfile * calculate ticksPerNanosecond in constructor * adapt Ratelimiter to check for both ProfileChunk and ProfileChunkUi ratelimiting * update ratelimiter test to check for both profileChunk and profileChunkUi drops * use string constant instead of string * Format code * add non aggregating event collector to send each event individually, deduplicate stacks * adapt converter tests to new non-aggregated converter * Format code * add logging to loadProfileConverter * Format code * fix duplication of events * catch all exception happening when converting from jfr * add exists and writable info to log message * add method to safely delete file * remove setNative call * fix test * fix reference to commit we vendored from * drop event if it cannot be processed to not lose the whole chunk * make format * fix test * Format code * Profiling - OTEL profiling fix, Stabilization, Logging (#4746) * add skipProfiling flag to TransactionOptions to be able to skip profiling and handle cases where profiling has been started by otel * add profilerId to spanContext so that otel span processor can propagate this to the exporter and SentryTracer * immediately end profiling when stopProfiler is called * bump api, fix android api 24 code * catch all exception happening when converting from jfr * simplify JavaContinuous profiler by catching AsyncProfiler instantiation exceptions in provider * add exists and writable info to log message * add method to safely delete file * remove setNative call * fix test * fix reference to commit we vendored from * drop event if it cannot be processed to not lose the whole chunk * Format code * fix test * Format code * fix test * catch exceptions in startProfiler/stopProfiler * fallback to threadId -1 if it cannot be resolved --------- Co-authored-by: Sentry Github Bot <[email protected]> --------- Co-authored-by: Sentry Github Bot <[email protected]>
* delete unused JfrFrame and JfrToSentryProfileconverter * use passed-in profilingTracesHz parameter instead of hardcoded value * start profiler before starting the transaction when ProfileLifecycle.TRACE is used to have the profile ID when SentryTracer is created * use improved way to calculate timestamp of sample * api dump * let profile-lifecycle be set from external_options, add tests for SpringBoot autoconfig * initialize stackTraceFactory only once per chunk * rename profile data classes, add deserialization and tests * extract methods in ProfileConverter, fix SentryProfile serialization and make fields private * use wall=[interval] instead of setting the event to wall and setting the interval separately, this seems to work better and create more samples * start/stop profiler in OtelSentrySpanProcesser in trace mode for root spans * add profiler dependency to jakarta-opentelemetry sample, add needed configs * add dependenies and config to spring-boot-jakarta sample * remove connection status check * extract event visitor * Add enum for ProfileChunk platform * fallback to default temp directory for profiling on jvm if directory is not configured * cleanup some minor things * remove ProfilingInitializer, fix comments * Format code * add getter/setter to sample and metadata * fix compile error * add comment/todo for deleteOnExit * Profiling - Deduplication and cleanup (#4681) * add readme and info about commit of the source repository * delete jfr file on jvm exit * further split into smaller methods * deduplicate frames in order to save bandwidth, add converter tests * remove Platform Enum, use string constants instead for compatibility with cross platform frameworks * implement equals and hashcode for SentryStackFrame to make frame deduplication work * bump api * improve error handling, fix start stop start flow * add new testfile * calculate ticksPerNanosecond in constructor * adapt Ratelimiter to check for both ProfileChunk and ProfileChunkUi ratelimiting * update ratelimiter test to check for both profileChunk and profileChunkUi drops * use string constant instead of string * Format code * add non aggregating event collector to send each event individually, deduplicate stacks * adapt converter tests to new non-aggregated converter * Format code * add logging to loadProfileConverter * Format code * fix duplication of events * catch all exception happening when converting from jfr * add exists and writable info to log message * add method to safely delete file * remove setNative call * fix test * fix reference to commit we vendored from * drop event if it cannot be processed to not lose the whole chunk * make format * fix test * Format code * Profiling - OTEL profiling fix, Stabilization, Logging (#4746) * add skipProfiling flag to TransactionOptions to be able to skip profiling and handle cases where profiling has been started by otel * add profilerId to spanContext so that otel span processor can propagate this to the exporter and SentryTracer * immediately end profiling when stopProfiler is called * bump api, fix android api 24 code * catch all exception happening when converting from jfr * simplify JavaContinuous profiler by catching AsyncProfiler instantiation exceptions in provider * add exists and writable info to log message * add method to safely delete file * remove setNative call * fix test * fix reference to commit we vendored from * drop event if it cannot be processed to not lose the whole chunk * Format code * fix test * Format code * fix test * catch exceptions in startProfiler/stopProfiler * fallback to threadId -1 if it cannot be resolved --------- Co-authored-by: Sentry Github Bot <[email protected]> --------- Co-authored-by: Sentry Github Bot <[email protected]> --------- Co-authored-by: Sentry Github Bot <[email protected]>
SpanContext
when span/transaction is created by OTELScopes
do not start profiler if profilerId already inSpanContext
💡 Motivation and Context
💚 How did you test it?
📝 Checklist
sendDefaultPII
is enabled.🔮 Next steps