-
-
Notifications
You must be signed in to change notification settings - Fork 435
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
Startup Profiling 2 - Add options and sampling logic #3121
Changes from 4 commits
ed69adc
3f7d206
1727f76
a44b5e3
c4b79d2
7d13d65
4aa19d2
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 | ||||
---|---|---|---|---|---|---|
|
@@ -4,6 +4,7 @@ | |||||
import io.sentry.cache.EnvelopeCache; | ||||||
import io.sentry.cache.IEnvelopeCache; | ||||||
import io.sentry.config.PropertiesProviderFactory; | ||||||
import io.sentry.instrumentation.file.SentryFileWriter; | ||||||
import io.sentry.internal.debugmeta.NoOpDebugMetaLoader; | ||||||
import io.sentry.internal.debugmeta.ResourcesDebugMetaLoader; | ||||||
import io.sentry.internal.modules.CompositeModulesLoader; | ||||||
|
@@ -21,6 +22,8 @@ | |||||
import io.sentry.util.thread.MainThreadChecker; | ||||||
import io.sentry.util.thread.NoOpMainThreadChecker; | ||||||
import java.io.File; | ||||||
import java.io.IOException; | ||||||
import java.io.Writer; | ||||||
import java.lang.reflect.InvocationTargetException; | ||||||
import java.util.Arrays; | ||||||
import java.util.List; | ||||||
|
@@ -47,6 +50,9 @@ private Sentry() {} | |||||
/** whether to use a single (global) Hub as opposed to one per thread. */ | ||||||
private static volatile boolean globalHubMode = GLOBAL_HUB_DEFAULT_MODE; | ||||||
|
||||||
private static final @NotNull String STARTUP_PROFILING_CONFIG_FILE_NAME = | ||||||
"startup_profiling_config"; | ||||||
|
||||||
/** | ||||||
* Returns the current (threads) hub, if none, clones the mainHub and returns it. | ||||||
* | ||||||
|
@@ -225,9 +231,7 @@ private static synchronized void init( | |||||
|
||||||
// If the executorService passed in the init is the same that was previously closed, we have to | ||||||
// set a new one | ||||||
final ISentryExecutorService sentryExecutorService = options.getExecutorService(); | ||||||
// If the passed executor service was previously called we set a new one | ||||||
if (sentryExecutorService.isClosed()) { | ||||||
if (options.getExecutorService().isClosed()) { | ||||||
options.setExecutorService(new SentryExecutorService()); | ||||||
} | ||||||
|
||||||
|
@@ -242,6 +246,58 @@ private static synchronized void init( | |||||
notifyOptionsObservers(options); | ||||||
|
||||||
finalizePreviousSession(options, HubAdapter.getInstance()); | ||||||
|
||||||
handleStartupProfilingConfig(options, options.getExecutorService()); | ||||||
} | ||||||
|
||||||
@SuppressWarnings("FutureReturnValueIgnored") | ||||||
private static void handleStartupProfilingConfig( | ||||||
final @NotNull SentryOptions options, | ||||||
final @NotNull ISentryExecutorService sentryExecutorService) { | ||||||
sentryExecutorService.submit( | ||||||
romtsn marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
() -> { | ||||||
final String cacheDirPath = options.getCacheDirPathWithoutDsn(); | ||||||
stefanosiano marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
if (cacheDirPath != null) { | ||||||
final @NotNull File startupProfilingConfigFile = | ||||||
new File(cacheDirPath, STARTUP_PROFILING_CONFIG_FILE_NAME); | ||||||
// We always delete the config file for startup profiling | ||||||
FileUtils.deleteRecursively(startupProfilingConfigFile); | ||||||
if (!options.isEnableStartupProfiling()) { | ||||||
return; | ||||||
} | ||||||
if (!options.isTracingEnabled()) { | ||||||
options | ||||||
.getLogger() | ||||||
.log( | ||||||
SentryLevel.INFO, | ||||||
"Tracing is disabled and startup profiling will not start."); | ||||||
return; | ||||||
} | ||||||
try { | ||||||
if (startupProfilingConfigFile.createNewFile()) { | ||||||
final @NotNull TracesSamplingDecision startupSamplingDecision = | ||||||
sampleStartupProfiling(options); | ||||||
final @NotNull SentryStartupProfilingOptions startupProfilingOptions = | ||||||
new SentryStartupProfilingOptions(options, startupSamplingDecision); | ||||||
try (Writer fileWriter = new SentryFileWriter(startupProfilingConfigFile)) { | ||||||
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.
Suggested change
SentryFileWriter is specific for performance instrumentation, I guess we don't want to create spans for our own file operations :) 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. Also, let's use FileOutputStream maybe, to align with all other places where we write to a file? |
||||||
options.getSerializer().serialize(startupProfilingOptions, fileWriter); | ||||||
} | ||||||
} | ||||||
} catch (IOException e) { | ||||||
options | ||||||
.getLogger() | ||||||
.log(SentryLevel.ERROR, "Unable to create startup profiling config file. ", e); | ||||||
} | ||||||
} | ||||||
}); | ||||||
} | ||||||
|
||||||
private static @NotNull TracesSamplingDecision sampleStartupProfiling( | ||||||
final @NotNull SentryOptions options) { | ||||||
TransactionContext startupTransactionContext = new TransactionContext("ui.load", ""); | ||||||
romtsn marked this conversation as resolved.
Show resolved
Hide resolved
|
||||||
startupTransactionContext.setForNextStartup(true); | ||||||
SamplingContext startupSamplingContext = new SamplingContext(startupTransactionContext, null); | ||||||
return new TracesSampler(options).sample(startupSamplingContext); | ||||||
} | ||||||
|
||||||
@SuppressWarnings("FutureReturnValueIgnored") | ||||||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,146 @@ | ||
package io.sentry; | ||
|
||
import io.sentry.vendor.gson.stream.JsonToken; | ||
import java.io.IOException; | ||
import java.util.Map; | ||
import java.util.concurrent.ConcurrentHashMap; | ||
import org.jetbrains.annotations.NotNull; | ||
import org.jetbrains.annotations.Nullable; | ||
|
||
final class SentryStartupProfilingOptions implements JsonUnknown, JsonSerializable { | ||
|
||
boolean profileSampled; | ||
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. should these be final? 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. nope, as these are read and deserialized from json (L100) |
||
@Nullable Double profileSampleRate; | ||
boolean traceSampled; | ||
@Nullable Double traceSampleRate; | ||
@Nullable String profilingTracesDirPath; | ||
boolean isProfilingEnabled; | ||
|
||
private @Nullable Map<String, Object> unknown; | ||
|
||
SentryStartupProfilingOptions() { | ||
traceSampled = false; | ||
traceSampleRate = null; | ||
profileSampled = false; | ||
profileSampleRate = null; | ||
profilingTracesDirPath = null; | ||
isProfilingEnabled = false; | ||
} | ||
|
||
SentryStartupProfilingOptions( | ||
final @NotNull SentryOptions options, | ||
final @NotNull TracesSamplingDecision samplingDecision) { | ||
traceSampled = samplingDecision.getSampled(); | ||
traceSampleRate = samplingDecision.getSampleRate(); | ||
profileSampled = samplingDecision.getProfileSampled(); | ||
profileSampleRate = samplingDecision.getProfileSampleRate(); | ||
profilingTracesDirPath = options.getProfilingTracesDirPath(); | ||
isProfilingEnabled = options.isProfilingEnabled(); | ||
} | ||
|
||
// JsonSerializable | ||
|
||
public static final class JsonKeys { | ||
public static final String PROFILE_SAMPLED = "profile_sampled"; | ||
public static final String PROFILE_SAMPLE_RATE = "profile_sample_rate"; | ||
public static final String TRACE_SAMPLED = "trace_sampled"; | ||
public static final String TRACE_SAMPLE_RATE = "trace_sample_rate"; | ||
public static final String PROFILING_TRACES_DIR_PATH = "profiling_traces_dir_path"; | ||
public static final String IS_PROFILING_ENABLED = "is_profiling_enabled"; | ||
} | ||
|
||
@Override | ||
public void serialize(final @NotNull ObjectWriter writer, final @NotNull ILogger logger) | ||
throws IOException { | ||
writer.beginObject(); | ||
writer.name(JsonKeys.PROFILE_SAMPLED).value(logger, profileSampled); | ||
writer.name(JsonKeys.PROFILE_SAMPLE_RATE).value(logger, profileSampleRate); | ||
writer.name(JsonKeys.TRACE_SAMPLED).value(logger, traceSampled); | ||
writer.name(JsonKeys.TRACE_SAMPLE_RATE).value(logger, traceSampleRate); | ||
writer.name(JsonKeys.PROFILING_TRACES_DIR_PATH).value(logger, profilingTracesDirPath); | ||
writer.name(JsonKeys.IS_PROFILING_ENABLED).value(logger, isProfilingEnabled); | ||
|
||
if (unknown != null) { | ||
for (String key : unknown.keySet()) { | ||
Object value = unknown.get(key); | ||
writer.name(key); | ||
writer.value(logger, value); | ||
} | ||
} | ||
writer.endObject(); | ||
} | ||
|
||
@Nullable | ||
@Override | ||
public Map<String, Object> getUnknown() { | ||
return unknown; | ||
} | ||
|
||
@Override | ||
public void setUnknown(@Nullable Map<String, Object> unknown) { | ||
this.unknown = unknown; | ||
} | ||
|
||
public static final class Deserializer | ||
implements JsonDeserializer<SentryStartupProfilingOptions> { | ||
|
||
@Override | ||
public @NotNull SentryStartupProfilingOptions deserialize( | ||
@NotNull JsonObjectReader reader, @NotNull ILogger logger) throws Exception { | ||
reader.beginObject(); | ||
SentryStartupProfilingOptions options = new SentryStartupProfilingOptions(); | ||
Map<String, Object> unknown = null; | ||
|
||
while (reader.peek() == JsonToken.NAME) { | ||
final String nextName = reader.nextName(); | ||
switch (nextName) { | ||
case JsonKeys.PROFILE_SAMPLED: | ||
Boolean profileSampled = reader.nextBooleanOrNull(); | ||
if (profileSampled != null) { | ||
options.profileSampled = profileSampled; | ||
} | ||
break; | ||
case JsonKeys.PROFILE_SAMPLE_RATE: | ||
Double profileSampleRate = reader.nextDoubleOrNull(); | ||
if (profileSampleRate != null) { | ||
options.profileSampleRate = profileSampleRate; | ||
} | ||
break; | ||
case JsonKeys.TRACE_SAMPLED: | ||
Boolean traceSampled = reader.nextBooleanOrNull(); | ||
if (traceSampled != null) { | ||
options.traceSampled = traceSampled; | ||
} | ||
break; | ||
case JsonKeys.TRACE_SAMPLE_RATE: | ||
Double traceSampleRate = reader.nextDoubleOrNull(); | ||
if (traceSampleRate != null) { | ||
options.traceSampleRate = traceSampleRate; | ||
} | ||
break; | ||
case JsonKeys.PROFILING_TRACES_DIR_PATH: | ||
String profilingTracesDirPath = reader.nextStringOrNull(); | ||
if (profilingTracesDirPath != null) { | ||
options.profilingTracesDirPath = profilingTracesDirPath; | ||
} | ||
break; | ||
case JsonKeys.IS_PROFILING_ENABLED: | ||
Boolean isProfilingEnabled = reader.nextBooleanOrNull(); | ||
if (isProfilingEnabled != null) { | ||
options.isProfilingEnabled = isProfilingEnabled; | ||
} | ||
break; | ||
default: | ||
if (unknown == null) { | ||
unknown = new ConcurrentHashMap<>(); | ||
} | ||
reader.nextUnknown(logger, unknown, nextName); | ||
break; | ||
} | ||
} | ||
options.setUnknown(unknown); | ||
reader.endObject(); | ||
return options; | ||
} | ||
} | ||
} |
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.
As mentioned in the other PR, maybe let's squash these changelogs into one?