[File based config] Apply config format changes defined by GDI spec #2558
Conversation
Refactoring profiler config methods to use config from the instance property, not form argument
…tivator.java Co-authored-by: jason plumb <75337021+breedx-splk@users.noreply.github.com>
# Conflicts: # profiler/src/main/java/com/splunk/opentelemetry/profiler/LogExporterBuilder.java # profiler/src/main/java/com/splunk/opentelemetry/profiler/ProfilerDeclarativeConfiguration.java # profiler/src/test/java/com/splunk/opentelemetry/profiler/LogExporterBuilderTest.java # profiler/src/test/java/com/splunk/opentelemetry/profiler/ProfilerDeclarativeConfigurationTest.java
|
|
||
| private static final String MEMORY_PROFILER = "memory_profiler"; | ||
| private static final String MEMORY_EVENT_RATE = "event_rate"; | ||
| private static final String MEMORY_SAMPLING_INTERVAL = "sampling_interval"; |
There was a problem hiding this comment.
Imo these aren't really interchangeable. Idk whether this counts as nitpicking. We wish to maintain a consistent event rate (or rather we wish to not create too many events). For this we don't really sample at a fixed interval. When using jdk.ObjectAllocationSample event (off by default) jdk will sample to main the desired rate. When we sample ourself we calculate the probability that would give us the desired amount of events and choose the events based on that probability from the set of all events.
There was a problem hiding this comment.
Both: CPU profiler and Snapshot profiler have sampling_interval option. Memory profiler just got consistent with them. If Rate vs interval is the concern then maybe we should revisit GDI spec. I appreciate the consistency between profilers and languages we just got and I'm not sure if JFR really keeps the rate super stable anyway (nodejs may be even worse)
@breedx-splk - WDYT ?
There was a problem hiding this comment.
https://github.com/signalfx/gdi-specification/blob/main/specification/configuration.md#file-based-configuration
Does not have sampling_interval under memory_profiler. My concern is that since we aren't really sampling these periodically using sampling_interval would be misleading.
Language distributions MAY specify additional configuration values in locations where it makes the most sense convention and usability wise.
as far as I can tell we are free to introduce whatever properties we need
2 participants
See https://github.com/signalfx/gdi-specification/blob/main/specification/configuration.md#file-based-configuration