Skip to content

Commit 8fca29a

Browse files
lbloderlbloder
committed
make initUtil methods more readable, return value from options, improve tests
1 parent 737c147 commit 8fca29a

File tree

2 files changed

+82
-79
lines changed

2 files changed

+82
-79
lines changed

‎sentry-async-profiler/src/test/java/io/sentry/asyncprofiler/init/AsyncProfilerInitUtilTest.kt‎

Lines changed: 16 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import io.sentry.asyncprofiler.profiling.JavaContinuousProfiler
99
import io.sentry.asyncprofiler.provider.AsyncProfilerProfileConverterProvider
1010
import io.sentry.util.InitUtil
1111
import kotlin.test.Test
12+
import kotlin.test.assertSame
1213
import org.mockito.kotlin.mock
1314

1415
class AsyncProfilerInitUtilTest {
@@ -21,59 +22,52 @@ class AsyncProfilerInitUtilTest {
2122
}
2223

2324
@Test
24-
fun `initialize Converter returns no-op profiler if profiling disabled`() {
25+
fun `initialize Converter returns no-op converter if profiling disabled`() {
2526
val options = SentryOptions()
2627
val converter = InitUtil.initializeProfileConverter(options)
2728
assert(converter is NoOpProfileConverter)
2829
}
2930

3031
@Test
31-
fun `initialize Profiler returns no-op profiler if profiler already initialized`() {
32+
fun `initialize profiler returns the existing profiler from options if already initialized`() {
33+
val initialProfiler =
34+
JavaContinuousProfiler(mock<ILogger>(), "", 10, mock<ISentryExecutorService>())
3235
val options =
3336
SentryOptions().also {
3437
it.setProfileSessionSampleRate(1.0)
35-
it.tracesSampleRate = 1.0
36-
it.setContinuousProfiler(
37-
JavaContinuousProfiler(mock<ILogger>(), "", 10, mock<ISentryExecutorService>())
38-
)
38+
it.setContinuousProfiler(initialProfiler)
3939
}
4040

4141
val profiler = InitUtil.initializeProfiler(options)
42-
assert(profiler is NoOpContinuousProfiler)
42+
assertSame(initialProfiler, profiler)
4343
}
4444

4545
@Test
46-
fun `initialize converter returns no-op converter if converter already initialized`() {
46+
fun `initialize converter returns the existing converter from options if already initialized`() {
47+
val initialConverter = AsyncProfilerProfileConverterProvider.AsyncProfilerProfileConverter()
4748
val options =
4849
SentryOptions().also {
4950
it.setProfileSessionSampleRate(1.0)
50-
it.tracesSampleRate = 1.0
51-
it.profilerConverter = AsyncProfilerProfileConverterProvider.AsyncProfilerProfileConverter()
51+
it.profilerConverter = initialConverter
5252
}
5353

5454
val converter = InitUtil.initializeProfileConverter(options)
55-
assert(converter is NoOpProfileConverter)
55+
assertSame(initialConverter, converter)
5656
}
5757

5858
@Test
5959
fun `initialize Profiler returns JavaContinuousProfiler if profiling enabled but profiler not yet initialized`() {
60-
val options =
61-
SentryOptions().also {
62-
it.setProfileSessionSampleRate(1.0)
63-
it.tracesSampleRate = 1.0
64-
}
60+
val options = SentryOptions().also { it.setProfileSessionSampleRate(1.0) }
6561
val profiler = InitUtil.initializeProfiler(options)
62+
assertSame(profiler, options.continuousProfiler)
6663
assert(profiler is JavaContinuousProfiler)
6764
}
6865

6966
@Test
70-
fun `initialize Profiler returns AsyncProfilerProfileConverterProvider if profiling enabled but profiler not yet initialized`() {
71-
val options =
72-
SentryOptions().also {
73-
it.setProfileSessionSampleRate(1.0)
74-
it.tracesSampleRate = 1.0
75-
}
67+
fun `initialize Converter returns AsyncProfilerProfileConverterProvider if profiling enabled but profiler not yet initialized`() {
68+
val options = SentryOptions().also { it.setProfileSessionSampleRate(1.0) }
7669
val converter = InitUtil.initializeProfileConverter(options)
70+
assertSame(converter, options.profilerConverter)
7771
assert(converter is AsyncProfilerProfileConverterProvider.AsyncProfilerProfileConverter)
7872
}
7973
}

‎sentry/src/main/java/io/sentry/util/InitUtil.java‎

Lines changed: 66 additions & 57 deletions
Original file line numberDiff line numberDiff line change
@@ -54,74 +54,83 @@ public static boolean shouldInit(
5454
}
5555

5656
public static IContinuousProfiler initializeProfiler(@NotNull SentryOptions options) {
57-
IContinuousProfiler continuousProfiler = NoOpContinuousProfiler.getInstance();
58-
59-
if (options.isContinuousProfilingEnabled()
60-
&& options.getContinuousProfiler() == NoOpContinuousProfiler.getInstance()) {
61-
try {
62-
String profilingTracesDirPath = options.getProfilingTracesDirPath();
63-
if (profilingTracesDirPath == null) {
64-
File tempDir = new File(System.getProperty("java.io.tmpdir"), "sentry_profiling_traces");
65-
boolean createDirectorySuccess = tempDir.mkdirs() || tempDir.exists();
66-
67-
if (!createDirectorySuccess) {
68-
throw new IllegalArgumentException(
69-
"Creating a fallback directory for profiling failed in "
70-
+ tempDir.getAbsolutePath());
71-
}
72-
profilingTracesDirPath = tempDir.getAbsolutePath();
73-
options.setProfilingTracesDirPath(profilingTracesDirPath);
74-
}
75-
76-
continuousProfiler =
77-
ProfilingServiceLoader.loadContinuousProfiler(
78-
options.getLogger(),
79-
profilingTracesDirPath,
80-
options.getProfilingTracesHz(),
81-
options.getExecutorService());
82-
83-
if (!(continuousProfiler instanceof NoOpContinuousProfiler)) {
84-
options.setContinuousProfiler(continuousProfiler);
85-
options.getLogger().log(SentryLevel.INFO, "Successfully loaded profiler");
86-
} else {
87-
options
88-
.getLogger()
89-
.log(
90-
SentryLevel.WARNING,
91-
"Could not load profiler, profiling will be disabled. If you are using Spring or Spring Boot with the OTEL Agent profiler init will be retried.");
92-
}
93-
94-
return continuousProfiler;
95-
96-
} catch (Exception e) {
57+
if (!shouldInitializeProfiler(options)) {
58+
return options.getContinuousProfiler();
59+
}
60+
61+
try {
62+
String profilingTracesDirPath = getOrCreateProfilingTracesDir(options);
63+
IContinuousProfiler profiler =
64+
ProfilingServiceLoader.loadContinuousProfiler(
65+
options.getLogger(),
66+
profilingTracesDirPath,
67+
options.getProfilingTracesHz(),
68+
options.getExecutorService());
69+
70+
if (profiler instanceof NoOpContinuousProfiler) {
9771
options
9872
.getLogger()
99-
.log(SentryLevel.ERROR, "Failed to create default profiling traces directory", e);
73+
.log(
74+
SentryLevel.WARNING,
75+
"Could not load profiler, profiling will be disabled. If you are using Spring or Spring Boot with the OTEL Agent profiler init will be retried.");
76+
} else {
77+
options.setContinuousProfiler(profiler);
78+
options.getLogger().log(SentryLevel.INFO, "Successfully loaded profiler");
10079
}
80+
} catch (Exception e) {
81+
options
82+
.getLogger()
83+
.log(SentryLevel.ERROR, "Failed to create default profiling traces directory", e);
10184
}
102-
return continuousProfiler;
85+
86+
return options.getContinuousProfiler();
10387
}
10488

10589
public static IProfileConverter initializeProfileConverter(@NotNull SentryOptions options) {
106-
IProfileConverter converter = NoOpProfileConverter.getInstance();
107-
108-
if (options.isContinuousProfilingEnabled()
109-
&& options.getProfilerConverter() instanceof NoOpProfileConverter) {
90+
if (!shouldInitializeProfileConverter(options)) {
91+
return options.getProfilerConverter();
92+
}
11093

111-
converter = ProfilingServiceLoader.loadProfileConverter();
94+
IProfileConverter converter = ProfilingServiceLoader.loadProfileConverter();
11295

96+
if (converter instanceof NoOpProfileConverter) {
97+
options
98+
.getLogger()
99+
.log(
100+
SentryLevel.WARNING,
101+
"Could not load profile converter. If you are using Spring or Spring Boot with the OTEL Agent, profile converter init will be retried.");
102+
} else {
113103
options.setProfilerConverter(converter);
104+
options.getLogger().log(SentryLevel.INFO, "Successfully loaded profile converter");
105+
}
114106

115-
if (!(converter instanceof NoOpProfileConverter)) {
116-
options.getLogger().log(SentryLevel.INFO, "Successfully loaded profile converter");
117-
} else {
118-
options
119-
.getLogger()
120-
.log(
121-
SentryLevel.WARNING,
122-
"Could not load profile converter. If you are using Spring or Spring Boot with the OTEL Agent, profile converter init will be retried.");
123-
}
107+
return options.getProfilerConverter();
108+
}
109+
110+
private static boolean shouldInitializeProfiler(@NotNull SentryOptions options) {
111+
return options.isContinuousProfilingEnabled()
112+
&& options.getContinuousProfiler() instanceof NoOpContinuousProfiler;
113+
}
114+
115+
private static boolean shouldInitializeProfileConverter(@NotNull SentryOptions options) {
116+
return options.isContinuousProfilingEnabled()
117+
&& options.getProfilerConverter() instanceof NoOpProfileConverter;
118+
}
119+
120+
private static String getOrCreateProfilingTracesDir(@NotNull SentryOptions options) {
121+
String profilingTracesDirPath = options.getProfilingTracesDirPath();
122+
if (profilingTracesDirPath != null) {
123+
return profilingTracesDirPath;
124124
}
125-
return converter;
125+
126+
File tempDir = new File(System.getProperty("java.io.tmpdir"), "sentry_profiling_traces");
127+
if (!tempDir.mkdirs() && !tempDir.exists()) {
128+
throw new IllegalArgumentException(
129+
"Creating a fallback directory for profiling failed in " + tempDir.getAbsolutePath());
130+
}
131+
132+
profilingTracesDirPath = tempDir.getAbsolutePath();
133+
options.setProfilingTracesDirPath(profilingTracesDirPath);
134+
return profilingTracesDirPath;
126135
}
127136
}

0 commit comments

Comments
 (0)