Skip to content
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

feat(triggers): include smart-triggered recordings in Harvester upload logic #217

Merged
merged 21 commits into from
Oct 4, 2023

Conversation

andrewazores
Copy link
Member

@andrewazores andrewazores commented Oct 3, 2023

  • create harvest package
  • split out HarvestModule
  • split out TriggerModule
  • track smart-triggered recordings in harvester logic

See #197
Related to #129


To test:

Using the same smoketest patch as before in #197

diff --git a/smoketest.sh b/smoketest.sh
index 187c5a7e..f8821bd2 100755
--- a/smoketest.sh
+++ b/smoketest.sh
@@ -202,7 +202,7 @@ runDemoApps() {
     podman run \
         --name quarkus-test-agent-1 \
         --pod cryostat-pod \
-        --env JAVA_OPTS="-Dquarkus.http.host=0.0.0.0 -Djava.util.logging.manager=org.jboss.logmanager.LogManager -Dcom.sun.management.jmxremote.port=9097 -Dcom.sun.management.jmxremote.ssl=false -Dcom.sun.management.jmxremote.authenticate=false -javaagent:/deployments/app/cryostat-agent.jar" \
+        --env JAVA_OPTS="-Dquarkus.http.host=0.0.0.0 -Djava.util.logging.manager=org.jboss.logmanager.LogManager -Dcom.sun.management.jmxremote.port=9097 -Dcom.sun.management.jmxremote.ssl=false -Dcom.sun.management.jmxremote.authenticate=false -javaagent:/deployments/app/cryostat-agent.jar=[ThreadCount>0]~default.jfc,[ThreadCount>0&&TargetDuration>duration(\"30s\")]~Profiling" \
         --env QUARKUS_HTTP_PORT=10010 \
         --env ORG_ACME_CRYOSTATSERVICE_ENABLED="false" \
         --env CRYOSTAT_AGENT_APP_NAME="quarkus-test-agent-1" \
@@ -221,7 +221,7 @@ runDemoApps() {
     podman run \
         --name quarkus-test-agent-2 \
         --pod cryostat-pod \
-        --env JAVA_OPTS="-Dquarkus.http.host=0.0.0.0 -Djava.util.logging.manager=org.jboss.logmanager.LogManager -javaagent:/deployments/app/cryostat-agent.jar" \
+        --env JAVA_OPTS="-Dquarkus.http.host=0.0.0.0 -Djava.util.logging.manager=org.jboss.logmanager.LogManager -javaagent:/deployments/app/cryostat-agent.jar=[ThreadCount>0&&TargetDuration>duration(\"2m\")]~Profiling" \
         --env QUARKUS_HTTP_PORT=10011 \
         --env ORG_ACME_CRYOSTATSERVICE_ENABLED="false" \
         --env CRYOSTAT_AGENT_APP_NAME="quarkus-test-agent-2" \
@@ -234,6 +234,7 @@ runDemoApps() {
         --env CRYOSTAT_AGENT_TRUST_ALL="true" \
         --env CRYOSTAT_AGENT_AUTHORIZATION="Basic $(echo user:pass | base64)" \
         --env CRYOSTAT_AGENT_API_WRITES_ENABLED="true" \
+        --env CRYOSTAT_AGENT_SMART_TRIGGER_EVALUATION_PERIOD_MS=10000 \
         --rm -d quay.io/andrewazores/quarkus-test:latest
 
     # copy a jboss-client.jar into /clientlib first

mvn install the agent, rebuild the quarkus-test sample, then run the smoketest. Try also with some variety of other trigger conditions. If the Agent is configured both with Smart Triggers and Harvester, then any recordings created by Smart Trigger should also become managed by the Harvester - when the Harvester performs its periodic uploads, the data from the Smart Triggers should be captured in a JFR Snapshot and uploaded to the Cryostat server. This is done using a transient JFR Snapshot recording, so it is all collated together into one recording. If a Smart Trigger recording is manually stopped then the Harvester should also automatically push that data to Cryostat. The triggered recording will be removed by the Agent if it is externally stopped and there may be error messages in the UI since as far as the server can tell, the remote recording disappeared unexpectedly from the target (#196).

@andrewazores andrewazores added feat New feature or request safe-to-test labels Oct 3, 2023
@Josh-Matsuoka
Copy link
Contributor

Looking good so far, I'll do a few more tests to double check but so far seems good.

@andrewazores
Copy link
Member Author

wdyt @Josh-Matsuoka ? Ready to go?

Copy link
Contributor

@Josh-Matsuoka Josh-Matsuoka left a comment

Choose a reason for hiding this comment

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

Looks good to me

@andrewazores andrewazores merged commit 2d0ed81 into cryostatio:main Oct 4, 2023
11 of 14 checks passed
@andrewazores andrewazores deleted the trigger-harvester branch October 4, 2023 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request safe-to-test
Projects
No open projects
Status: Done
Development

Successfully merging this pull request may close these issues.

2 participants