METRON-1856: Parser aggregation #1360
Conversation
…n into METRON-1856-parser-grouping
|
@ruffle1986 Can you double-check the state of this PR? There are over 100 commits here going back to November, which doesn't seem quite right. |
|
@ruffle1986 - it still looks like there's still something amiss with this PR. Can you re-apply your changes on a clean branch so there aren't 145 commits? As I look through the commit history, this looks like it was a collaborative effort with @tiborm, however I only see your name on it. Either this should be split up, or more likely it looks like this belongs in a feature branch. Either way, we can't accept this in this state. EDIT - I'm also concerned about the size of the change here weighing in at 5k lines. We've discussed this multiple times before as a community, and it was agreed that this large of a change should
|
|
This is obviously a lot of work, and something that would be cool to have ( and useful), but we are going to have to have a plan to get it in more than landing this one pr….. let’s have a thread to kick that off so we can organize around getting it in. That will also generate some familiarity with what is going on that could enable more review engagement. The pool for TS reviewers is small enough as it is. I would expect the [DISCUSS] to talk about Who, What, Why, How of the effort and to open the discussion on landing. I would be prepared to discuss or have ideas about ALS -> alternative landing scenarios. IE> can this be broken down or not? |
|
@mmiklavc There's definitely something wrong with the commit history so I'm gonna clean this up once and for all. Everyone in the UI team has worked on this. The reason why you can see my name everywhere is because I had to clean the commit history last year as well because it was messed. It seems like it's the nature of this long living feature branch and there must be something wrong with our approach to keep it up-to-date with the master. First I tried rebasing it with master but it caused headaches when others wanted to have the latest version on their local computer. Then we agreed on using merging commits to make it obvious what was going on. We already have a discussion thread on this. Unfortunately I couldn't find it the mailing list but remember? This was the thread where we introduced the benefits of a better state management strategy and a new module called Ngrx. As I can remember, you liked the idea and we got the green light. However the commit history is messed up, once it's solved, I think it's enough to introduce this change set in one feature branch because it's one feature called Parser Aggregation. For the record, @merrimanr reviewed it a few months ago and he liked the approach how we achieved better state management. He reported a few bugs and we also found a few after manual testing and we also found some uncovered edge cases. It had never been easier to scale the UI code and add the required features, fixes in order to meet the criteria. Steps:
Does that make sense? |
|
I'm closing this PR and once I've had the clean history, will reopen it in another branch. |
Contributor Comments
Link to the original ASF Jira ticket: https://issues.apache.org/jira/browse/METRON-1856
About the feature
The use is able to group individual sensors together. The purpose of it is being able to start/stop or enable/disable sensors of the same kind. It's for the sake of convenience.
How?
Go to a sensor list screen and simply grab a sensor and drop it on another. There's a grid icon that indicates the fact that the row is draggable. Grab the grid and move it around. By doing to this, you can aggregate two sensors. After that, a pane appears where you can add the name of the two groups. You can also add a description as a reminder about what's going on.
Then you'll the newly created group and the two aggregated sensors below. The purple rectangles next to the group name are for pointing out that they're groups obviously. The sensors that belong to a particular group are listed below. There's a dash at the beginning of the row to show the connection between the group and the sensors.
Once sensors are grouped together you're unable to perform operations on them individually. You can only perform operations on the group.
Once a group is created, you can add as many sensors to it as you want.
You can only add sensors to stopped or disabled groups.
You can only aggregate stopped or disabled sensors together.
If a sensor belongs to a group, you can remove it by dragging and moving it out from the group's area.
You can edit the group name and the description anytime by clicking on the pen icon on the right.
It could be hard to notice but it's possible to put a sensor between two already grouped sensors inside a group. There's a blue line between rows to make it obvious.
It's very important to point it out that the result of the aggregation is not immediately persisted on the backend. You have to click on the "APPLY" button at the bottom of the screen.
Different colors for each row mean different updates:
Once the changes are persisted after hitting the Apply button, the colors disappear.
You can undo all of your updates since the latest save by clicking on the "DISCARD ALL" button next to the APPLY button.
Important Blocker
This PR cannot be merged back to master until @merrimanr is done with the related updates of the backend part of the parser aggregation feature. Until then, I close this PR and reopen it once he's done.
Testing
metron-alertsfoldernpm cihttp://localhost:4200/sensorsin your browserPull Request Checklist
Thank you for submitting a contribution to Apache Metron.
Please refer to our Development Guidelines for the complete guide to follow for contributions.
Please refer also to our Build Verification Guidelines for complete smoke testing guides.
In order to streamline the review of the contribution we ask you follow these guidelines and ask you to double check the following:
For all changes:
For code changes:
Have you included steps to reproduce the behavior or problem that is being changed or addressed?
Have you included steps or a guide to how the change may be verified and tested manually?
Have you ensured that the full suite of tests and checks have been executed in the root metron folder via:
Have you written or updated unit tests and or integration tests to verify your changes?
If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent?
For documentation related changes:
Have you ensured that format looks appropriate for the output in which it is rendered by building and verifying the site-book? If not then run the following commands and the verify changes via
site-book/target/site/index.html:Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.
It is also recommended that travis-ci is set up for your personal repository such that your branches are built there before submitting a pull request.