[OpAmp] Memory profiling support for remote config#2919
Conversation
Test improved
# Conflicts: # opamp/src/main/java/com/splunk/opentelemetry/opamp/RemoteConfigProcessor.java # opamp/src/main/java/com/splunk/opentelemetry/opamp/effectiveconfig/EnvVarsEffectiveConfigFileFactory.java
…-cfg-profiler-interval
# Conflicts: # opamp/src/main/java/com/splunk/opentelemetry/opamp/RemoteConfigProcessor.java # opamp/src/test/java/com/splunk/opentelemetry/opamp/RemoteConfigProcessorTest.java
JMV metrics installed on demand
There was a problem hiding this comment.
Pull request overview
This PR adds remote-configuration-driven control over profiling, including a new “reinitialize” path to apply updated profiler settings without requiring separate start/stop commands, and ties JVM memory-metrics installation to the memory-profiler enablement state. It also refactors periodic recording construction from a builder API into a factory and removes now-redundant JVM metrics installer wiring.
Changes:
- Add
requestReinitializeProfiling()and implement reinitialize logic inProfilingSupervisor(stop if active, restart if enabled), including JVM memory-metrics install/uninstall based on config + property override. - Update OpAMP remote config processing to update profiler settings and trigger reinitialization only when effective profiler configuration changes; improve failure handling by reporting FAILED status without rethrowing.
- Refactor periodic recording flusher creation into
PeriodicRecordingFlusherFactoryand make JVM memory metrics installers idempotent with explicit uninstall support (plus updated/added tests).
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| testing/jmh-benchmarks/src/main/java/com/splunk/opentelemetry/profiler/old/AgentInternalsFilter.java | Update filtered JFR thread name from “Sequencer” to “Flusher”. |
| profiler/src/test/java/com/splunk/opentelemetry/profiler/ProfilingSupervisorTest.java | Rework supervisor tests for factory-based flusher creation, reinitialize behavior, and JVM memory metrics toggling. |
| profiler/src/test/java/com/splunk/opentelemetry/profiler/ProfilerConfigurationTest.java | Adjust tests to validate new equals/hashCode behavior. |
| profiler/src/test/java/com/splunk/opentelemetry/profiler/PeriodicRecordingFlusherFactoryTest.java | Rename and adapt tests to factory create(...) API. |
| profiler/src/main/java/com/splunk/opentelemetry/profiler/StackTraceFilter.java | Update filtered JFR thread name from “Sequencer” to “Flusher”. |
| profiler/src/main/java/com/splunk/opentelemetry/profiler/ProfilingSupervisor.java | Add reinitialize command, integrate JVM memory metrics install/uninstall, and use PeriodicRecordingFlusherFactory. |
| profiler/src/main/java/com/splunk/opentelemetry/profiler/ProfilerConfiguration.java | Introduce equals/hashCode to support change detection in remote config application logic. |
| profiler/src/main/java/com/splunk/opentelemetry/profiler/PeriodicRecordingFlusherFactory.java | Replace builder with factory that constructs PeriodicRecordingFlusher instances. |
| profiler/src/main/java/com/splunk/opentelemetry/profiler/PeriodicRecordingFlusher.java | Remove static builder entrypoint (now created via factory). |
| profiler/build.gradle.kts | Add dependency on :instrumentation:jvm-metrics for new supervisor-managed metrics integration (compile/test). |
| opamp/src/test/java/com/splunk/opentelemetry/opamp/RemoteConfigProcessorTest.java | Update expectations to use reinitialize and add failure + “update while running” coverage. |
| opamp/src/main/java/com/splunk/opentelemetry/opamp/RemoteConfigProcessor.java | Apply profiler config updates conditionally and request reinitialize on change; report failures without rethrowing. |
| instrumentation/jvm-metrics/src/test/java/com/splunk/opentelemetry/instrumentation/jvmmetrics/otel/OtelGcMemoryMetricsTest.java | Add tests for idempotent install/uninstall behavior. |
| instrumentation/jvm-metrics/src/test/java/com/splunk/opentelemetry/instrumentation/jvmmetrics/otel/OtelAllocatedMemoryMetricsTest.java | Add tests for idempotent install/uninstall behavior. |
| instrumentation/jvm-metrics/src/test/java/com/splunk/opentelemetry/instrumentation/jvmmetrics/JvmMetricsInstallerTest.java | Remove tests for deleted JvmMetricsInstaller. |
| instrumentation/jvm-metrics/src/main/java/com/splunk/opentelemetry/instrumentation/jvmmetrics/otel/OtelGcMemoryMetrics.java | Make install idempotent and add uninstall/cleanup. |
| instrumentation/jvm-metrics/src/main/java/com/splunk/opentelemetry/instrumentation/jvmmetrics/otel/OtelAllocatedMemoryMetrics.java | Make install idempotent and add uninstall/cleanup. |
| instrumentation/jvm-metrics/src/main/java/com/splunk/opentelemetry/instrumentation/jvmmetrics/JvmMetricsInstaller.java | Remove agent listener installer (metrics now managed by profiler supervisor). |
| instrumentation/jvm-metrics/build.gradle.kts | Remove unneeded dependencies after deleting JvmMetricsInstaller. |
| custom/src/main/java/com/splunk/opentelemetry/logging/SplunkSlf4jLoggingCustomizer.java | Update log message example thread name to “JFR Recording Flusher”. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
Tick the box to add this pull request to the merge queue (same as
|
| val otelInstrumentationVersion: String = rootProject.extra["otelInstrumentationVersion"] as String | ||
| val javaagentInstrumentationProjects = | ||
| project(":instrumentation").subprojects.filter { | ||
| it.name !in setOf("compile-stub", "nocode-testing") |
There was a problem hiding this comment.
what was wrong with using splunk.instrumentation-conventions as the marker?
There was a problem hiding this comment.
There were two reasons:
- After profiler included :instrumentation:jmv-metrics there were errors from gradle: "Cannot mutate the dependencies of configuration ':agent:javaagentLibs' after the configuration was resolved"
- This marker caused including ":instrumentation:servlet-3-testing" module into production code.
I finally decided to explicitly list all relevant ":instrumentation" submodules.
There was a problem hiding this comment.
The way of handling :instrumentation submodules was changed because after :profiler included :instrumentation:jmv-metrics there were errors from gradle:
"Cannot mutate the dependencies of configuration ':agent:javaagentLibs' after the configuration was resolved"
The splunk.instrumentation-conventions marker also caused including :instrumentation:servlet-3-testing module into production code.
There was a problem hiding this comment.
I wasn't able to reproduce the Cannot mutate the dependencies of configuration failure.
The splunk.instrumentation-conventions marker also caused including :instrumentation:servlet-3-testing module into production code.
This can be avoided by renaming splunk.instrumentation-conventions to splunk.instrumentation-testing-conventions and creating splunk.instrumentation-conventions that adds the splunk.instrumentation-testing-conventions plugin. Then the testing module should still function and not get included. Upstream uses a similar split.
PR includes: