Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ public class SnapshotProfilingConfiguration implements AutoConfigurationCustomiz
"splunk.snapshot.profiler.enabled";
private static final String SELECTION_PROBABILITY_KEY = "splunk.snapshot.selection.probability";
private static final String STACK_DEPTH_KEY = "splunk.snapshot.profiler.max.stack.depth";
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it also make sense to change the stack depth property and other properties that start with splunk.snapshot.profiler?

Copy link
Copy Markdown
Contributor

@robsunday robsunday Nov 4, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or actually prefix everything with splunk.profiler.snapshot. , since snapshots are part of profiler. This would also create more logical composition of properties in declarative config yaml. Something like:

instrumentation/development:
  java:  
    splunk:
      profiler:
        enabled: true #splunk.profiler.enabled (default: false)
        directory: #splunk.profiler.directory (default: system specific temp dir)

        snapshot:
          enabled: true
          export:
            interval: 10000
          sampling:
            interval: 5000
          selection:
            probability: 0.025
          staging:
            capacity: 100
          max:
            stack:
              depth: 1000

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

would it also make sense to change the stack depth property and other properties that start with splunk.snapshot.profiler?

Maybe, but that's out of scope right now. I only wanted to align the 3 main ones for now.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or actually prefix everything with splunk.profiler.snapshot. , since snapshots are part of profiler.

This is a step backwards. If you feel strongly about this, then please open an issue in gdi-specification. I think shorter is better than longer, and shallower is better than deeper...but let's not argue about that in this PR please.

private static final String SAMPLING_INTERVAL_KEY = "splunk.snapshot.profiler.sampling.interval";
private static final String SAMPLING_INTERVAL_KEY = "splunk.snapshot.sampling.interval";
private static final String EXPORT_INTERVAL_KEY = "splunk.snapshot.profiler.export.interval";
private static final String STAGING_CAPACITY_KEY = "splunk.snapshot.profiler.staging.capacity";

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ void defaultValuesAreProvidedToOpenTelemetry() {
assertEquals("false", properties.getString("splunk.snapshot.profiler.enabled"));
assertEquals("0.01", properties.getString("splunk.snapshot.selection.probability"));
assertEquals("1024", properties.getString("splunk.snapshot.profiler.max.stack.depth"));
assertEquals("10ms", properties.getString("splunk.snapshot.profiler.sampling.interval"));
assertEquals("10ms", properties.getString("splunk.snapshot.sampling.interval"));
assertEquals("5s", properties.getString("splunk.snapshot.profiler.export.interval"));
assertEquals("2000", properties.getString("splunk.snapshot.profiler.staging.capacity"));
}
Expand Down Expand Up @@ -142,7 +142,7 @@ void getDefaultSnapshotProfilerStackDepthWhenNotSpecified() {
void getConfiguredSnapshotProfilerSamplingInterval(int milliseconds) {
var properties =
DefaultConfigProperties.create(
Map.of("splunk.snapshot.profiler.sampling.interval", String.valueOf(milliseconds)),
Map.of("splunk.snapshot.sampling.interval", String.valueOf(milliseconds)),
COMPONENT_LOADER);
assertEquals(
Duration.ofMillis(milliseconds),
Expand Down Expand Up @@ -245,12 +245,12 @@ void includeSnapshotProfilingStackTraceDepth(int depth) {
void includeSnapshotProfilingSamplingInterval(String interval) {
var properties =
DefaultConfigProperties.create(
Map.of("splunk.snapshot.profiler.sampling.interval", interval), COMPONENT_LOADER);
Map.of("splunk.snapshot.sampling.interval", interval), COMPONENT_LOADER);

SnapshotProfilingConfiguration.log(properties);

var duration = properties.getDuration("splunk.snapshot.profiler.sampling.interval");
log.assertContains("splunk.snapshot.profiler.sampling.interval" + " : " + duration);
var duration = properties.getDuration("splunk.snapshot.sampling.interval");
log.assertContains("splunk.snapshot.sampling.interval" + " : " + duration);
}

@ParameterizedTest
Expand Down