[OpAMP] Enable/disable snapshot profiling with remote config - #2955
Conversation
# Conflicts: # opamp/src/main/java/com/splunk/opentelemetry/opamp/OpampActivator.java # opamp/src/main/java/com/splunk/opentelemetry/opamp/RemoteConfigProcessor.java # opamp/src/main/java/com/splunk/opentelemetry/opamp/ServerToAgentMessageHandler.java # opamp/src/test/java/com/splunk/opentelemetry/opamp/RemoteConfigProcessorImplTest.java # profiler/src/main/java/com/splunk/opentelemetry/profiler/snapshot/SnapshotProfilingSupervisor.java
| OptionalConfigurableSupplier<ProfilerConfiguration> configSupplier, | ||
| JFR jfr, | ||
| AutoConfiguredOpenTelemetrySdk sdk, | ||
| BlockingQueue<ProfilingCommand> commandQueue, |
There was a problem hiding this comment.
[for reviewer] It was not used anywhere, so I removed this parameter from constructor.
There was a problem hiding this comment.
Good catch. I think it might have been used for testing at some point, or the original idea was that other components might need to have the queue to send stuff into it....but it's encapsulated and I guess it's not needed. We can always add it back if/when that changes.
There was a problem hiding this comment.
[for reviewer] This class follows the pattern of using async processing established in ProfilingSupervisor class.
breedx-splk
left a comment
breedx-splk
left a comment
There was a problem hiding this comment.
Looks good, thanks. I like that it's following the pattern established by the other profiler, but I don't love that they now have two separate threads across both implementations. For now, though, I think it's fine....especially until we decide about how we generally want to handle additional mutable/state changes going forward.
I think it's worth pointing out, just for historical purposes, that there are still a number of ways in which the reported/actual state (seen through effective config) may not reflect the actual state of the profiler(s). We can address that later if/when it becomes an issue.
3 participants
Adds support for dynamic enable/disable snapshot profiling using remote configuration.