feat(triggers): Add Mbean selector to Trigger Creation Form#2192
feat(triggers): Add Mbean selector to Trigger Creation Form#2192Josh-Matsuoka wants to merge 12 commits intocryostatio:mainfrom
Conversation
| isIs: boolean; | ||
| } | ||
|
|
||
| export interface MbeanAttributeMap { |
| onAccept: (s: string) => void; | ||
| } | ||
|
|
||
| export interface MbeanSelectorOption { |
andrewazores
left a comment
andrewazores
left a comment
There was a problem hiding this comment.
Just a little confused about the MBean query list and filtering for the supported attributes.
| 'SystemLoadAverage', | ||
| 'ProcessCpuLoad', | ||
| 'TotalPhysicalMemorySize', | ||
| 'FreePhyiscalMemorySize', |
There was a problem hiding this comment.
typo? Phyiscal -> Physical
| const MbeanOptions = React.useMemo(() => { | ||
| var attributes: MbeanSelectorOption[] = []; | ||
| mbeans.forEach((m: MbeanAttributeMap) => { | ||
| m.attributes.forEach((i: MBeanAttributeInfo) => { |
There was a problem hiding this comment.
If supportedTriggerAttributes is just a hardcoded list of the MBeans we expect to see present in the remote JVM, then do we even need to send a query at this point... ? It seems like just returning the supported attributes list would do the same thing more simply, until we actually have some wider support on the backend (agent side) for processing other MBean metrics in the CEL expressions. Or am I missing something here?
There was a problem hiding this comment.
You're correct that hardcoding it would achieve the same thing, I wanted to lay the groundwork here for generic mbean attributes down the line but for now we can get rid of the query if you'd prefer and just hardcode the list options.
There was a problem hiding this comment.
If it's currently functionally equivalent then I think for now let's just go with the hardcoded list so that we can get that UI enhancement in, and work on the broader-scoped change that affects multiple components when we're further out from a dev freeze. Maybe the hardcoded version of this can go directly into #2155 and this PR can build on top of that do to the enhanced filtering based on actual live data pulled from the Agent.
There was a problem hiding this comment.
Sounds good, I'll add it to that PR
2 participants
Welcome to Cryostat! 👋
Before contributing, make sure you have:
mainbranch[chore, ci, docs, feat, fix, test]To recreate commits with GPG signature
git fetch upstream && git rebase --force --gpg-sign upstream/mainFixes: #
Depends on: #2155 cryostatio/cryostat-core#667 cryostatio/cryostat-agent#849 cryostatio/cryostat#1467
Description of the change:
How to manually test: