METRON-2034: REST endpoint for getting all parser topology status should return group name

[merrimanr]

Contributor Comments

This PR fixes a bug in the REST application that fails to convert a parser topology id to a group name when calling the http://node1:8082/api/v1/storm endpoint. The StormStatusServiceImplTest was also updated to included this case.

Testing

This has been tested in full dev:

1. Spin up full dev and verify data is flowing to Elasticsearch.
2. Define a group name for the OOTB bro, snort, yaf parser topology that is included in full dev:

curl -X POST --header 'Content-Type: application/json' --header 'Accept: application/json' -d '{
    "name": "group1",
    "description": "group1",
    "sensors": [
      "bro",
      "snort",
      "yaf"
    ]
  }' 'http://user:password@node1:8082/api/v1/sensor/parser/group'

3. Request a storm status summary for all topologies:

curl -X GET --header 'Accept: application/json' 'http://node1:8082/api/v1/storm'

The bro__snort__yaf Storm topology should be returned with the name set to group1:

{
    "id": "bro__snort__yaf-4-1556557869",
    "name": "group1",
    "status": "ACTIVE",
    "latency": 2751.941,
    "throughput": 6.045,
    "emitted": 3710,
    "acked": 3627
  }

Pull 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:

• [x] Is there a JIRA ticket associated with this PR? If not one needs to be created at Metron Jira.
• [x] Does your PR title start with METRON-XXXX where XXXX is the JIRA number you are trying to resolve? Pay particular attention to the hyphen "-" character.
• [x] Has your PR been rebased against the latest commit within the target branch (typically master)?

For code changes:

• [x] Have you included steps to reproduce the behavior or problem that is being changed or addressed?

• [x] Have you included steps or a guide to how the change may be verified and tested manually?

• [x] Have you ensured that the full suite of tests and checks have been executed in the root metron folder via:

  mvn -q clean integration-test install && dev-utilities/build-utils/verify_licenses.sh 
  

• [x] Have you written or updated unit tests and or integration tests to verify your changes?

• [x] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?

• [x] 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:

• [x] 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:

  cd site-book
  mvn site
  

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.

[ruffle1986]

@merrimanr I've tested it with full dev and now the UI is able to display if a group of sensors is running or not properly. So this is fine but there's another issue. The reason why it has become a blocker for the latest release candidate is that bro, snort, and yaf is already aggregated.

And the topology info for the three of them coming back from the storm endpoint is just fine. It says it's ACTIVE but since the UI doesn't get any information about their "group", it's unable to render the three sensors as they are grouped together.

So currently, the name of the topology item regarding the three aforementioned sensors is "bro__snort__yaf" but there is no group with the same name so the UI can't figure it's a group and the three sensors belong to it.

[merrimanr]

You're correct @ruffle1986. The group should already be added when full dev spins up rather than having to add it in a separate step. I am testing a change that should fix this and will push it out once that's done.

[merrimanr]

The latest commit should setup the parser group in full dev automatically.

[mmiklavc]

@merrimanr now that https://github.com/apache/metron/tree/feature/METRON-1856-parser-aggregation is under way, do we have enough detail to bridge the gap between what the UI needs and the backend piece, in particular the documentation? I'm not completely sure if global config is the best place for it or not, since this only really seems to be leverage by REST application (ie, it's not used globally), but I don't have a particularly excellent alternative to recommend currently.

[merrimanr]

We are not at a point in the review where we can spin up or test anything. Agreed, documentation belongs in the REST README.

[mmiklavc]

Let's hold off on this item until the end, or whenever we can reasonably start doing some validation with the actual UI components in this feature branch. It sounds like this may have come from some offline collaboration and testing, so we should make sure that this accomplishes the desired results now that the aggregation feature is going through review.

[merrimanr]

I filed this as a bug back in March based on feedback from the contributors working on the parser aggregation feature branch. This fix is required for that feature to work correctly, which is why I think we should move documentation to a follow on Jira.

[mmiklavc]

Can you link to what you're describing? I'm not seeing much on the ticket - https://issues.apache.org/jira/browse/METRON-2034

[merrimanr]

Are you saying you want a more detailed description the bug as it relates to the UI? This might be easier to demonstrate when the UI part is further along.

[mmiklavc]

Are you saying you want a more detailed description the bug as it relates to the UI? This might be easier to demonstrate when the UI part is further along.

I'm not seeing where the "based on feedback from the contributors working on the parser aggregation feature branch" interaction happened. Was that a private message/interaction? All I'm saying is that if there was a comment on another PR, or a discuss thread, or a convo in Slack - just add it here for context.

[merrimanr]

It was a Slack conversation. @ruffle1986 was testing and noticed group names were not working for the endpoint that gets status for all topologies (vs the endpoint that gets status for a single topology). He brought it to my attention and I created this fix as a result.

[mmiklavc]

I filed this as a bug back in March based on feedback from the contributors working on the parser aggregation feature branch. This fix is required for that feature to work correctly, which is why I think we should move documentation to a follow on Jira.

I'm not sure I see the point of splitting up a documentation task here - seems like extra overhead with an unclear benefit, but if that's your preference, you're welcome to add a sub-task to the master feature branch Jira that would need to be checked off before the FB is completed.