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

[Task] Devise a trigger condition syntax and implement processing from config #130

Closed
Tracked by #128
andrewazores opened this issue May 26, 2023 · 5 comments
Closed
Tracked by #128
Assignees
Labels
feat New feature or request help wanted Extra attention is needed

Comments

@andrewazores
Copy link
Member

andrewazores commented May 26, 2023

Idea 1

The syntax should easily map to SmallRye Config, so it should be something that can be expressed and understood when expressed as an environment variable name, as well as when expressed as a system property or properties file entry.

The syntax must also be able to specify:

  • the metric to observe
  • a condition about that metric
  • a trigger value for the condition
  • optional parameters such as a duration threshold that the condition must persist for to trigger
  • the name of a .jfc event template file that should be used when the trigger starts a recording

For example, <Process CPU Load (%), Value Greater Than, 0.2, For 30 Seconds> is a 4-tuple that could be expressed in such a syntax, which should cause a self-explanatory recording trigger. This might be expressed in a rudimentary way as PROCESSCPU_GT_20_30S=profiling.jfc, as a very rough example. This simple environment variable could be split on _ characters. The first field would be used to look up in a table which metric should be observed, and then implementation-specific details for each metric would yield an observation function. The second field, GT, would be matched against supported operations, in this case the > comparator, to apply to the result of the observation function and the parsed value of the third field, 20. In this case this third field could be interpreted as hundredths, ie 0.20. The fourth optional field of 30S would be interpreted as the duration threshold, so the implementation would need to maintain samples over time and check the condition across all of the samples before trigger the recording.


Idea 2

Rather than specifying the trigger conditions as SmallRye Config properties, a more freeform syntax could be used and passed to the agent as an argument.

https://docs.oracle.com/javase/8/docs/api/java/lang/instrument/package-summary.html

$ java -javaagent:/path/to/cryostat-agent.jar=argumentstring

where argumentstring could be more flexible and easier to parse than environment variable names, since more characters should be permissible to help delineate fields. This could also be used to pass a path to the agent that contains something like a JSON file containing a serialized form of the triggers to be used.

@andrewazores
Copy link
Member Author

andrewazores commented May 26, 2023

The idea of using a configuration file for this purpose could also fit with simply defining an optional path to such a file via SmallRye Config property.

In a containers/k8s/OpenShift story, this could be supplied by a ConfigMap:
https://kubernetes.io/docs/tasks/configure-pod-container/configure-pod-configmap/

The ConfigMap can be mounted as a file in the container, and a config property used to tell the agent to load that file to figure out what triggers it should set up.

This same technique can already be used to override the agent's own default microprofile-config.properties, so this idea does also slot in with Idea 1 above in that sense.

If the trigger definition syntax can be expressed in terms of environment variables then the user can either perform this configuration by environment variable or by ConfigMap and embedding the trigger configuration into the existing .properties.

If the trigger syntax cannot be expressed as environment variables or otherwise does not work well with SmallRye Config properties, then a separate config file (ex. ConfigMap) defined in some other format such as JSON/YAML could be used.

@andrewazores
Copy link
Member Author

andrewazores commented Jun 29, 2023

Here's some info I put elsewhere for another reason, but which I just had the thought could be very relevant here as well:

https://github.com/cryostatio/cryostat/issues/990#issuecomment-1613364915

tl;dr "Common Expression Language" looks like it would be a good syntax for describing smart triggers, and this looks a lot like the JavaScript-based matchExpressions already used by the Cryostat server for Automated Rules and Stored Credentials. This could be used with either Idea 1 or Idea 2 from the original issue I filed here. For Idea 2 I think it is relatively straightforward, an expression or list of expression strings would be passed as part of the Java command line and would become arguments to the agentmain. For Idea 2, the expressions could be provided as the value of a SmallRye Config property, or perhaps a series of properties with a common key format so that multiple expressions can be provided. For the follow-up about using a mounted ConfigMap, this still slots in well, since these CEL expressions can be easily embedded into ex. a YAML document that backs a ConfigMap.

@andrewazores
Copy link
Member Author

@Josh-Matsuoka ^

@Josh-Matsuoka
Copy link
Contributor

Sounds like a good approach to me, I'll do some reading/look into it!

@andrewazores
Copy link
Member Author

Done in #197

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat New feature or request help wanted Extra attention is needed
Projects
No open projects
Status: Done
Status: Done
Development

No branches or pull requests

2 participants