-
-
Notifications
You must be signed in to change notification settings - Fork 461
Fix profiling init for Spring and Spring Boot w Agent auto-init #4815
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
Changes from 1 commit
a4ce01a
39225ff
40395fb
f8f253f
c02c3dd
d180c19
5e75584
dde02b9
0514903
2cb26e1
737c147
41d2c6a
87071dd
8fca29a
40bb0a2
4a52a0d
f04bdbc
c3c36b6
3ebaeb5
fa864ed
1bb7e19
adf084c
23ebbf7
937782e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,13 @@ | ||
| package io.sentry.spring.boot.jakarta; | ||
|
|
||
| import com.jakewharton.nopen.annotation.Open; | ||
| import io.sentry.spring.jakarta.SentryProfilerConfiguration; | ||
| import org.springframework.boot.autoconfigure.condition.ConditionalOnClass; | ||
| import org.springframework.context.annotation.Configuration; | ||
| import org.springframework.context.annotation.Import; | ||
|
|
||
| @Configuration(proxyBeanMethods = false) | ||
| @ConditionalOnClass(name = {"io.sentry.opentelemetry.agent.AgentMarker"}) | ||
| @Open | ||
| @Import(SentryProfilerConfiguration.class) | ||
| public class SentryProfilerAutoConfiguration {} | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,3 +1,4 @@ | ||
| io.sentry.spring.boot.jakarta.SentryAutoConfiguration | ||
| io.sentry.spring.boot.jakarta.SentryProfilerAutoConfiguration | ||
| io.sentry.spring.boot.jakarta.SentryLogbackAppenderAutoConfiguration | ||
| io.sentry.spring.boot.jakarta.SentryWebfluxAutoConfiguration |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,94 @@ | ||
| package io.sentry.spring.jakarta; | ||
|
|
||
| import com.jakewharton.nopen.annotation.Open; | ||
| import io.sentry.IContinuousProfiler; | ||
| import io.sentry.IProfileConverter; | ||
| import io.sentry.NoOpContinuousProfiler; | ||
| import io.sentry.NoOpProfileConverter; | ||
| import io.sentry.Sentry; | ||
| import io.sentry.SentryLevel; | ||
| import io.sentry.SentryOptions; | ||
| import io.sentry.profiling.ProfilingServiceLoader; | ||
| import org.springframework.boot.autoconfigure.condition.ConditionalOnMissingBean; | ||
| import org.springframework.context.annotation.Bean; | ||
| import org.springframework.context.annotation.Configuration; | ||
|
|
||
| /** | ||
| * Handles late initialization of the profiler if the application is run with the OTEL Agent in | ||
| * auto-init mode. In that case the agent cannot initialize the profiler yet and falls back to No-Op | ||
| * implementations. This Configuration sets the profiler and converter on the options if that was | ||
| * the case. | ||
| */ | ||
| @Configuration(proxyBeanMethods = false) | ||
| @Open | ||
| public class SentryProfilerConfiguration { | ||
|
|
||
| @Bean | ||
| @ConditionalOnMissingBean(name = "sentryOpenTelemetryProfilerConfiguration") | ||
| public IContinuousProfiler sentryOpenTelemetryProfilerConfiguration() { | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Extract init into util to streamline initialization code |
||
| SentryOptions options = Sentry.getGlobalScope().getOptions(); | ||
| IContinuousProfiler profiler = NoOpContinuousProfiler.getInstance(); | ||
|
|
||
| if (Sentry.isEnabled() | ||
| && options.isContinuousProfilingEnabled() | ||
| && options.getContinuousProfiler() instanceof NoOpContinuousProfiler) { | ||
|
|
||
| options | ||
| .getLogger() | ||
| .log( | ||
| SentryLevel.DEBUG, | ||
| "Continuous profiler is NoOp, attempting to reload with Spring Boot classloader"); | ||
|
|
||
| String path = options.getProfilingTracesDirPath(); | ||
|
|
||
| profiler = | ||
| ProfilingServiceLoader.loadContinuousProfiler( | ||
| options.getLogger(), | ||
| path != null ? path : "", | ||
| options.getProfilingTracesHz(), | ||
| options.getExecutorService()); | ||
|
|
||
| options.setContinuousProfiler(profiler); | ||
|
|
||
| if (!(profiler instanceof NoOpContinuousProfiler)) { | ||
| options | ||
| .getLogger() | ||
| .log( | ||
| SentryLevel.INFO, | ||
| "Successfully loaded continuous profiler via Spring Boot classloader"); | ||
| } | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Sentry profiler: Custom configuration discarded.When Sentry is disabled, the method returns a new |
||
| return profiler; | ||
| } | ||
|
|
||
| @Bean | ||
| @ConditionalOnMissingBean(name = "sentryOpenTelemetryProfilerConverterConfiguration") | ||
| public IProfileConverter sentryOpenTelemetryProfilerConverterConfiguration() { | ||
|
Collaborator
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Extract init into util to streamline initialization code |
||
| SentryOptions options = Sentry.getGlobalScope().getOptions(); | ||
| IProfileConverter converter = NoOpProfileConverter.getInstance(); | ||
|
|
||
| if (Sentry.isEnabled() | ||
| && options.isContinuousProfilingEnabled() | ||
| && options.getProfilerConverter() instanceof NoOpProfileConverter) { | ||
|
|
||
| options | ||
| .getLogger() | ||
| .log( | ||
| SentryLevel.DEBUG, | ||
| "Profile converter is NoOp, attempting to reload with Spring Boot classloader"); | ||
|
|
||
| converter = ProfilingServiceLoader.loadProfileConverter(); | ||
|
|
||
| options.setProfilerConverter(converter); | ||
|
|
||
| if (!(converter instanceof NoOpProfileConverter)) { | ||
| options | ||
| .getLogger() | ||
| .log( | ||
| SentryLevel.INFO, | ||
| "Successfully loaded profile converter via Spring Boot classloader"); | ||
| } | ||
| } | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Bug: Sentry Disabled: Configured Converter IgnoredWhen Sentry is disabled, the method returns a new |
||
| return converter; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,21 @@ | ||
| package io.sentry; | ||
|
|
||
| import io.sentry.protocol.profiling.SentryProfile; | ||
| import java.io.IOException; | ||
| import org.jetbrains.annotations.NotNull; | ||
|
|
||
| public final class NoOpProfileConverter implements IProfileConverter { | ||
|
|
||
| private static final NoOpProfileConverter instance = new NoOpProfileConverter(); | ||
|
|
||
| private NoOpProfileConverter() {} | ||
|
|
||
| public static NoOpProfileConverter getInstance() { | ||
| return instance; | ||
| } | ||
|
|
||
| @Override | ||
| public @NotNull SentryProfile convertFromFile(@NotNull String jfrFilePath) throws IOException { | ||
| return new SentryProfile(); | ||
| } | ||
| } |
Uh oh!
There was an error while loading. Please reload this page.
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.
Make it conditional on the async profiler class too?
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.
Sounds good, I see no reason to run this if profiler isn't there.
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.
What might become an issue is, if we ever have a second
IContinuousProfilerimplementation, we also have to update the condition