Skip to content
This repository was archived by the owner on Aug 20, 2025. It is now read-only.

Conversation

@iraghumitra
Copy link
Contributor

@iraghumitra iraghumitra commented Aug 17, 2017

Contributor Comments

This PR is on top of METRON-1068. This PR gets column names from rest-api instead of directly fetching data from ES.

This contains code changes in METRON-1068 as well.

Tests

If you run the metron-alerts UI using any one of the ways mentioned below you would notice that UI fetches the data from matron-rest api not from ES.

Dev: From 'metron/metron-interface/metron-alerts' you can run ./scripts/start-dev.sh and open the GUI at localhost:4200

E2E: From 'metron/metron-interface/metron-alerts' you can run ./scripts/start-server-for-e2e.sh in one terminal and run npm run e2e in a seperate terminal

Deployment: Follow the steps mentioned in Readme and you should see a login page and data being fetched from rest-api's

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:

  • Is there a JIRA ticket associated with this PR? If not one needs to be created at Metron Jira.
  • 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.
  • Has your PR been rebased against the latest commit within the target branch (typically master)?

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:

    mvn -q clean integration-test install && build_utils/verify_licenses.sh 
    
  • 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:

    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.

Integrated pom.xml to build metron alerts
- Added translate pipe to rename col names
- Added option to rename col's
…t and right side of the textbox

- Take out trashcan for recent
- Change recent to 10 by default but store 25, I’ll do a design for how that needs to look
- when the search box overflows, the save search button should be centered vertically with the expanded textbook
- Search button should be square
2. Corrected the mapping for alert id
3. Fixed issue with severity colour coding, the color coding was disappearing if we hover in and hover out of the severity cell
4. Moved alert-severity.directive to directives folder
5. Removed bootstrap from angular-cli as this is already included by ng build
Added license header to all files
Renamed mock folder to mock-data
…han one search request

Escaping the values in search request
Removed the unused variable in AlertService
Added filter support for score 'threat:triage:score'
Mapping _id field to _uid for sorting
Added ColumnMetaData type in return types
Corrected the application name in expressjs server
Removed aggs in QueryBuilder as it is not used yet
 - Fixed the fields in Alerts
 - Renamed MetadataUtil to more appropriate ElasticsearchUtils
 - Added AlertsSearchResponse as a obj to hold the search responses
 - Added abstract class 'DataSource' that holds all the api requests needed by GUI
 - Added new class 'ElasticSearchLocalstorageImpl' that provides solr and local storage implementation for DataSource
Abstracted search-request from query-builder
Search data to be fired after getting all the columns for display
import {MetronRestApiUtils} from '../utils/metron-rest-api-utils';
import {AlertSource} from '../model/alert-source';

export class RestApiImpl extends ElasticSearchLocalstorageImpl {
Copy link
Contributor

Choose a reason for hiding this comment

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

Could we just call this SearchService or something similar? There are going to be other functions we add to the REST layer so this class is going to grow to be very large eventually if we just have one generic REST service. To me, it would make more sense to create a matching service for each REST controller. This is how it was done with the management UI.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@merrimanr the abstraction was created to facilitate rest-api creation. I am also a fan of having http calls in services. But should we remove the abstraction once all the rest-api's are implemented. This way we will have a clear visibility into the api's that are available and api's that needs to be worked on.

Please feel free to suggest. The code change would be small either ways :)

Copy link
Contributor

Choose a reason for hiding this comment

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

I think having an extra layer around services would be unnecessary. My preference would be to have matching angular services and rest controllers.

@iraghumitra
Copy link
Contributor Author

@merrimanr I moved the rest-api calls to services

private dataSource: DataSource,
private ngZone: NgZone) { }

public search(searchRequest: SearchRequest): Observable<AlertsSearchResponse> {
Copy link
Contributor

Choose a reason for hiding this comment

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

Is it possible to make this service class match the rest layer?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

import {HttpUtil} from '../utils/httpUtil';

@Injectable()
export class ClusterMetaDataService {
Copy link
Contributor

Choose a reason for hiding this comment

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

Shouldn't this be in a search.service.ts class instead of having it's own service class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


export class MetronRestApiUtils {

public static extractColumnNameDataFromRestApi(res: Response): ColumnMetadata[] {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this would make more sense if this were part of the service class. Why have it in a separate class?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

Renamed alert-search-response to search-response
Moved all search api's to search.service
@iraghumitra
Copy link
Contributor Author

@merrimanr I have made services similar to rest classes. I had to rename few files, classes and variables.

@merrimanr
Copy link
Contributor

Thanks @iraghumitra. I tested this in full dev and everything worked as expected. +1

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants