You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@metron.apache.org by tiborm <gi...@git.apache.org> on 2018/07/13 18:30:36 UTC

[GitHub] metron pull request #1103: Feature/metron 1554 pcap query panel

GitHub user tiborm opened a pull request:

    https://github.com/apache/metron/pull/1103

    Feature/metron 1554 pcap query panel

    ## Contributor Comments
    This PR contains the latest Alert UI changes from Ryan's pcapdemo branch. I cherry picked only the UI related commits. In case of one commit which contained boot UI and back-end changes, I resolved conflicts to keep the java files untouched. But please doublecheck, this PR intended to contain only the latest version of the PCAP UI.
    
    
    ## Pull Request Checklist
    
    Thank you for submitting a contribution to Apache Metron.  
    Please refer to our [Development Guidelines](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=61332235) for the complete guide to follow for contributions.  
    Please refer also to our [Build Verification Guidelines](https://cwiki.apache.org/confluence/display/METRON/Verifying+Builds?show-miniview) 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](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel).
    - [ ] 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 && dev-utilities/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](http://www.apache.org/legal/resolved.html#category-a)?
    - [ ] 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](https://travis-ci.org) is set up for your personal repository such that your branches are built there before submitting a pull request.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/tiborm/metron feature/METRON-1554-pcap-query-panel

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/metron/pull/1103.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1103
    
----
commit 872d1b1ee13e358c18956945d71d3667d19fca8a
Author: merrimanr <me...@...>
Date:   2018-04-12T14:57:48Z

    Merge branch 'pcap-front' of https://github.com/simonellistonball/metron into pcaprest
    
    Conflicts:
    	metron-interface/metron-alerts/src/app/app.module.ts

commit b1b6a7dabea1a1d0d132482c8d97af29c0ac2683
Author: merrimanr <me...@...>
Date:   2018-04-13T15:00:15Z

    initial commit
    
     Conflicts:
    	metron-interface/metron-rest/src/main/java/org/apache/metron/rest/MetronRestApplication.java
    	metron-interface/metron-rest/src/main/java/org/apache/metron/rest/controller/PcapQueryController.java
    	metron-interface/metron-rest/src/main/java/org/apache/metron/rest/util/pcapQueryThread.java

commit 55cf2d945a4fcff1e7e2e47a234037ed6f394b2e
Author: merrimanr <me...@...>
Date:   2018-04-18T15:52:56Z

    added license headers

----


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by tiborm <gi...@git.apache.org>.
Github user tiborm commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1103#discussion_r205732629
  
    --- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-packet-line/pcap-packet-line.component.ts ---
    @@ -0,0 +1,55 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +import { Component, OnInit, Input } from '@angular/core';
    +import { PdmlPacket, PdmlProto, PdmlField } from '../model/pdml'
    +
    +@Component({
    +  selector: '[app-pcap-packet-line]',
    +  templateUrl: './pcap-packet-line.component.html',
    +  styleUrls: ['./pcap-packet-line.component.scss']
    +})
    +export class PcapPacketLineComponent implements OnInit {
    +
    +  @Input() packet: PdmlPacket
    +
    +  ip: {
    +    timestamp: PdmlField,
    +    ip_src_addr: PdmlField, ip_src_port: PdmlField,
    +    ip_dest_addr: PdmlField, ip_dest_port: PdmlField,
    +    protocol: PdmlField
    +  }
    +
    +  constructor() { }
    +
    +  ngOnInit() {
    +    let gen_proto: PdmlProto = this.packet.protos.filter(p => p.name == "geninfo")[0]
    --- End diff --
    
    Thanks @merrimanr! I addressed this in a new commit.


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1103#discussion_r204138081
  
    --- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-packet-line/pcap-packet-line.component.ts ---
    @@ -0,0 +1,38 @@
    +import { Component, OnInit, Input } from '@angular/core';
    --- End diff --
    
    we need a license header here


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1103#discussion_r204138196
  
    --- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-packet/pcap-packet.component.ts ---
    @@ -0,0 +1,22 @@
    +import { Component, OnInit, Input } from '@angular/core';
    --- End diff --
    
    we need a license header here


---

[GitHub] metron issue #1103: Feature/metron 1554 pcap query panel

Posted by mmiklavc <gi...@git.apache.org>.
Github user mmiklavc commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    @tiborm Thanks for the contribution! A few comments for starters:
    - Can you provide a test script for reviewers?
    - What's this 10k+ line spec file? Are these working AngularJS unit tests, and if so, are they now part of the build? Please comment, and if they are not part of the automated build, please provide instructions for running them even if it's simply pointing to an existing doc.
    - When you have run through the items in the PR checklist from above, please mark them as such.


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1103#discussion_r204138294
  
    --- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-panel/pcap-panel.component.ts ---
    @@ -0,0 +1,65 @@
    +import { Component, OnInit, Input } from '@angular/core';
    --- End diff --
    
    we need a license header here


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    @justinleet I disagree with "Polling seems pretty avoidable".  I feel like that would be significant work and I'm not sure the Hadoop Job object is asynchronous anyways.  We would have to poll somewhere.  Maybe we poll in the middle-tier but we don't have any examples right now where the middle-tier exposes a callback to a client.  That would all be new development with new dependencies added, new unit and integration test patterns, etc.  If we really think this is necessary (I would vote no) then I feel like it should be a separate , follow on PR.


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by mmiklavc <gi...@git.apache.org>.
Github user mmiklavc commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    Looks like all comments have been addressed, reviewers have been able to test successfully, and any remaining follow-on work has been added to the feature branch Jira/Epic. I'm +1 by inspection. Nice work gentlemen. 


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by tiborm <gi...@git.apache.org>.
Github user tiborm commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    @mmiklavc Thanks for the comment! I extended the JIRA ticket with user story like test scenarios.
    Also added a short description of to the PR description about how to spin up a full dev with pcap.


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by tiborm <gi...@git.apache.org>.
GitHub user tiborm reopened a pull request:

    https://github.com/apache/metron/pull/1103

    METRON-1671: Initial PCAP UI

    ## Contributor Comments
    This PR contains the initial cut of PCAP UI.
    
    <img width="2029" alt="screen shot 2018-07-16 at 9 29 49 am" src="https://user-images.githubusercontent.com/2437400/42747095-28d510bc-88db-11e8-8501-98c82f6ec521.png">
    
    The user is able to submit a PCAP process job by the search button (magnifier icon) at the top of the page. In the same filter bar User is able to narrow the result set by filtering to the following fields:
    
      - IP Source Address
      - IP Source Port
      - IP Dest Address
      - IP Dest Port
      - Protocol
      - Include Reverse Traffic
      - Free text filtering
    
    While the PCAP job is in progress a progress bar shows up below the filter bar. (Sorry, having no screenshot about it.)
    
    <img width="2028" alt="screen shot 2018-07-16 at 9 30 43 am" src="https://user-images.githubusercontent.com/2437400/42747099-2cbd9db6-88db-11e8-8be8-7f2bb2971fe3.png">
    
    The result of the PCAP processing is visualized in a grid. User able to open each record to get access to the details of the packet.
    
    
    # Running tests
    If you like to run the unit tests run:
    npm install
    npm test
    
    For addition info please check the readme.md in metron-alerts.
    
    ## Pull Request Checklist
    
    Thank you for submitting a contribution to Apache Metron.  
    Please refer to our [Development Guidelines](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=61332235) for the complete guide to follow for contributions.  
    Please refer also to our [Build Verification Guidelines](https://cwiki.apache.org/confluence/display/METRON/Verifying+Builds?show-miniview) 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](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel).
    - [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?
    - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
    - [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:
    - [ ] 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](https://travis-ci.org) is set up for your personal repository such that your branches are built there before submitting a pull request.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/tiborm/metron feature/METRON-1554-pcap-query-panel

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/metron/pull/1103.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1103
    
----

----


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by tiborm <gi...@git.apache.org>.
Github user tiborm commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1103#discussion_r205722021
  
    --- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-panel/pcap-panel.component.ts ---
    @@ -0,0 +1,65 @@
    +import { Component, OnInit, Input } from '@angular/core';
    --- End diff --
    
    Thanks, we added license headers.


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    @sardell Thanks for adding the tickets.
    
    +1, @tiborm Thanks for your patience during the whole review process.  I'm looking forward to getting the ball rolling on the rest of the contributions!


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1103#discussion_r204541945
  
    --- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-panel/pcap-panel.component.ts ---
    @@ -0,0 +1,78 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +import { Component, OnInit, Input } from '@angular/core';
    +
    +import { PcapService, PcapStatusResponse } from '../service/pcap.service'
    +import { PcapRequest } from '../model/pcap.request'
    +import { Pdml } from '../model/pdml'
    +import {Subscription} from "rxjs/Rx";
    +
    +@Component({
    +  selector: 'app-pcap-panel',
    +  templateUrl: './pcap-panel.component.html',
    +  styleUrls: ['./pcap-panel.component.scss']
    +})
    +export class PcapPanelComponent implements OnInit {
    +
    +  @Input() pdml: Pdml = null;
    +
    +  @Input() pcapRequest: PcapRequest;
    +
    +  statusSubscription: Subscription;
    +  queryRunning: boolean = false;
    +  progressWidth: number = 0;
    +  selectedPage: number = 1;
    +
    +  constructor(private pcapService: PcapService ) { }
    +
    +  ngOnInit() {
    +  }
    +
    +  onSearch(pcapRequest) {
    +    console.log(pcapRequest);
    +    this.pdml = null;
    +    this.progressWidth = 0;
    +    this.pcapService.submitRequest(pcapRequest).subscribe(id => {
    +      this.queryRunning = true;
    +      this.statusSubscription = this.pcapService.pollStatus(id).subscribe((statusResponse: PcapStatusResponse) => {
    +        if ('SUCCEEDED' === statusResponse.jobStatus) {
    +          this.statusSubscription.unsubscribe();
    +          this.queryRunning = false;
    +          this.pcapService.getPackets(id, this.selectedPage).toPromise().then(pdml => {
    +            this.pdml = pdml;
    +          });
    +        } else if (this.progressWidth < 100) {
    +          this.progressWidth = Math.trunc(statusResponse.percentComplete);
    +        }
    +      });
    +    });
    +
    +    // this.pcapService.getTestPackets(this.pcapRequest).subscribe(response => {
    --- End diff --
    
    Commented code.


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    There's a bug report that rat treats .ts files as binary and ignores them.
    https://issues.apache.org/jira/browse/RAT-234


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by tiborm <gi...@git.apache.org>.
Github user tiborm commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1103#discussion_r205715003
  
    --- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-list/pcap-list.component.spec.ts ---
    @@ -0,0 +1,53 @@
    +import { async, ComponentFixture, TestBed } from '@angular/core/testing';
    --- End diff --
    
    Thanks, we added license headers.


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by tiborm <gi...@git.apache.org>.
Github user tiborm commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1103#discussion_r205723106
  
    --- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-panel/pcap-panel.component.ts ---
    @@ -0,0 +1,78 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +import { Component, OnInit, Input } from '@angular/core';
    +
    +import { PcapService, PcapStatusResponse } from '../service/pcap.service'
    +import { PcapRequest } from '../model/pcap.request'
    +import { Pdml } from '../model/pdml'
    +import {Subscription} from "rxjs/Rx";
    +
    +@Component({
    +  selector: 'app-pcap-panel',
    +  templateUrl: './pcap-panel.component.html',
    +  styleUrls: ['./pcap-panel.component.scss']
    +})
    +export class PcapPanelComponent implements OnInit {
    +
    +  @Input() pdml: Pdml = null;
    +
    +  @Input() pcapRequest: PcapRequest;
    +
    +  statusSubscription: Subscription;
    +  queryRunning: boolean = false;
    +  progressWidth: number = 0;
    +  selectedPage: number = 1;
    +
    +  constructor(private pcapService: PcapService ) { }
    +
    +  ngOnInit() {
    +  }
    +
    +  onSearch(pcapRequest) {
    +    console.log(pcapRequest);
    +    this.pdml = null;
    +    this.progressWidth = 0;
    +    this.pcapService.submitRequest(pcapRequest).subscribe(id => {
    +      this.queryRunning = true;
    +      this.statusSubscription = this.pcapService.pollStatus(id).subscribe((statusResponse: PcapStatusResponse) => {
    +        if ('SUCCEEDED' === statusResponse.jobStatus) {
    +          this.statusSubscription.unsubscribe();
    +          this.queryRunning = false;
    +          this.pcapService.getPackets(id, this.selectedPage).toPromise().then(pdml => {
    +            this.pdml = pdml;
    +          });
    +        } else if (this.progressWidth < 100) {
    +          this.progressWidth = Math.trunc(statusResponse.percentComplete);
    +        }
    +      });
    +    });
    +
    +    // this.pcapService.getTestPackets(this.pcapRequest).subscribe(response => {
    --- End diff --
    
    Commented code removed.


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    @mmiklavc The job manager is asynchronous in that it accepts a Finalizer.  We have the polling loop in place but we would need to refactor the job manager to expose a callback function for getStatus.
    
    @justinleet I get where you're coming from, async communication would be ideal.  For this use case though, I don't see much of a benefit.  There is no reason we can't send status for all user jobs in response to a single polling request (a trivial change since there is already a getJobs method on the job manager).  Also, we are reporting percentage done so there would likely be multiple aysnc calls anyways as the job progresses. 
    
    This is definitely something we should add to our platform and I would be happy to work on it with you.  Sounds like you're ok with this being a follow on.  I'm sure we'll need this construct at some point in the near future.


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    @sardell I'm fine with this stuff being follow on.  Can we make sure we have tickets linked to https://issues.apache.org/jira/browse/METRON-1554?
    
    I believe the follow-ons are 
    
    - Input validation
    - Kill button for in progress jobs
    - Testing the backend for the null vs blank discrepancy.  I don't care that the front end gives either, I care about making sure there are tests such that we're sure being provided either works as expected.
    
    Is there anything else that tickets need to be created for (assuming I didn't just miss them)?
    
    Once we get the tickets made (and linked here, so it's easy for anyone following the discussion to find them), I'm good with this being the basis for the rest of the UI work.


---

[GitHub] metron pull request #1103: METRON-1554: Initial PCAP UI

Posted by tiborm <gi...@git.apache.org>.
GitHub user tiborm reopened a pull request:

    https://github.com/apache/metron/pull/1103

    METRON-1554: Initial PCAP UI

    ## Contributor Comments
    This PR contains the initial cut of PCAP UI.
    
    <img width="2029" alt="screen shot 2018-07-16 at 9 29 49 am" src="https://user-images.githubusercontent.com/2437400/42747095-28d510bc-88db-11e8-8501-98c82f6ec521.png">
    
    <img width="2028" alt="screen shot 2018-07-16 at 9 30 43 am" src="https://user-images.githubusercontent.com/2437400/42747099-2cbd9db6-88db-11e8-8be8-7f2bb2971fe3.png">
    
    
    
    ## Pull Request Checklist
    
    Thank you for submitting a contribution to Apache Metron.  
    Please refer to our [Development Guidelines](https://cwiki.apache.org/confluence/pages/viewpage.action?pageId=61332235) for the complete guide to follow for contributions.  
    Please refer also to our [Build Verification Guidelines](https://cwiki.apache.org/confluence/display/METRON/Verifying+Builds?show-miniview) 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](https://issues.apache.org/jira/browse/METRON/?selectedTab=com.atlassian.jira.jira-projects-plugin:summary-panel).
    - [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?
    - [ ] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under [ASF 2.0](http://www.apache.org/legal/resolved.html#category-a)?
    - [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:
    - [ ] 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](https://travis-ci.org) is set up for your personal repository such that your branches are built there before submitting a pull request.


You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/tiborm/metron feature/METRON-1554-pcap-query-panel

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/metron/pull/1103.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #1103
    
----
commit 872d1b1ee13e358c18956945d71d3667d19fca8a
Author: merrimanr <me...@...>
Date:   2018-04-12T14:57:48Z

    Merge branch 'pcap-front' of https://github.com/simonellistonball/metron into pcaprest
    
    Conflicts:
    	metron-interface/metron-alerts/src/app/app.module.ts

commit b1b6a7dabea1a1d0d132482c8d97af29c0ac2683
Author: merrimanr <me...@...>
Date:   2018-04-13T15:00:15Z

    initial commit
    
     Conflicts:
    	metron-interface/metron-rest/src/main/java/org/apache/metron/rest/MetronRestApplication.java
    	metron-interface/metron-rest/src/main/java/org/apache/metron/rest/controller/PcapQueryController.java
    	metron-interface/metron-rest/src/main/java/org/apache/metron/rest/util/pcapQueryThread.java

commit 55cf2d945a4fcff1e7e2e47a234037ed6f394b2e
Author: merrimanr <me...@...>
Date:   2018-04-18T15:52:56Z

    added license headers

----


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by mmiklavc <gi...@git.apache.org>.
Github user mmiklavc commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    >  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?
    
    Can you submit or link to the test script/instructions for this?


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by tiborm <gi...@git.apache.org>.
Github user tiborm commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1103#discussion_r205714876
  
    --- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-filters/pcap-filters.component.spec.ts ---
    @@ -0,0 +1,29 @@
    +import { async, ComponentFixture, TestBed } from '@angular/core/testing';
    --- End diff --
    
    Thanks, we added license headers.


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    @merrimanr Up front I want to say I agree async is follow-on.  I think polling is fine for right now, but there are potential scaling concerns and I wanted to get a sense of the reasoning.  In practice, it may not actually be pretty avoidable, but I'm honestly not sure.
    
    For how we'd do it, I'm not super familiar with the the job and status objects.  Having said that, I know a lot of the YARN stuff is already setup in an relatively async manner, it may simply be a matter of finding the right hook. 
    
    There are a benefit to doing it async, specifically that we could start having users being able to kick off multiple pcap queries simultaneously without having their browsers do many requests a second.
    
    I'd also expect similar types of jobs to potentially make their way into UIs that might also benefit from being async (or even potentially existing ones) that would benefit from having an async pattern available to model off of.


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    Several people have given feedback on this PR.  What do you think @mmiklavc, @justinleet and @cestella?  Is this good enough for a first pass at the UI?


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    I have submitted a PR to up the rat plugin version and have fixed it in master #1126, let's make sure to add license headers here for new files added in this branch though.


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by tiborm <gi...@git.apache.org>.
Github user tiborm closed the pull request at:

    https://github.com/apache/metron/pull/1103


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by tiborm <gi...@git.apache.org>.
Github user tiborm commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    @mmiklavc The 10k+ line spec file is a working unit test which contains mock data as well.
    Unfortunately, some other unit tests failing in this PR. We already to a separate ticket for fixing and enriching unit tests. 
    
    @justinleet managed to make UI unit tests part of our Travis build. He opens a PR for that soon.


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by tiborm <gi...@git.apache.org>.
Github user tiborm commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1103#discussion_r205723353
  
    --- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-panel/pcap-panel.component.ts ---
    @@ -0,0 +1,78 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +import { Component, OnInit, Input } from '@angular/core';
    +
    +import { PcapService, PcapStatusResponse } from '../service/pcap.service'
    +import { PcapRequest } from '../model/pcap.request'
    +import { Pdml } from '../model/pdml'
    +import {Subscription} from "rxjs/Rx";
    +
    +@Component({
    +  selector: 'app-pcap-panel',
    +  templateUrl: './pcap-panel.component.html',
    +  styleUrls: ['./pcap-panel.component.scss']
    +})
    +export class PcapPanelComponent implements OnInit {
    +
    +  @Input() pdml: Pdml = null;
    +
    +  @Input() pcapRequest: PcapRequest;
    +
    +  statusSubscription: Subscription;
    +  queryRunning: boolean = false;
    +  progressWidth: number = 0;
    +  selectedPage: number = 1;
    +
    +  constructor(private pcapService: PcapService ) { }
    +
    +  ngOnInit() {
    +  }
    +
    +  onSearch(pcapRequest) {
    +    console.log(pcapRequest);
    +    this.pdml = null;
    +    this.progressWidth = 0;
    +    this.pcapService.submitRequest(pcapRequest).subscribe(id => {
    +      this.queryRunning = true;
    +      this.statusSubscription = this.pcapService.pollStatus(id).subscribe((statusResponse: PcapStatusResponse) => {
    +        if ('SUCCEEDED' === statusResponse.jobStatus) {
    --- End diff --
    
    FAILED status handling added. KILLED status is out of scope of the current PR.


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1103#discussion_r204541854
  
    --- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-panel/pcap-panel.component.html ---
    @@ -0,0 +1,39 @@
    +<!--
    +  Licensed to the Apache Software
    +	Foundation (ASF) under one or more contributor license agreements. See the
    +	NOTICE file distributed with this work for additional information regarding
    +	copyright ownership. The ASF licenses this file to You under the Apache License,
    +	Version 2.0 (the "License"); you may not use this file except in compliance
    +	with the License. You may obtain a copy of the License at
    +  http://www.apache.org/licenses/LICENSE-2.0
    +  Unless required by applicable law or agreed to in writing, software distributed
    +	under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES
    +	OR CONDITIONS OF ANY KIND, either express or implied. See the License for
    +  the specific language governing permissions and limitations under the License.
    +  -->
    +<div class="panel my-4">
    +  <div class="panel-header">
    +    <app-pcap-filters [queryRunning]="queryRunning" (search)="onSearch($event)"></app-pcap-filters>
    +  </div>
    +    <!--<div *ngIf="queryRunning" style="text-align: center">-->
    --- End diff --
    
    Commented code should be removed.


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by sardell <gi...@git.apache.org>.
Github user sardell commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    @justinleet Here are the tickets for the follow-on items:
    
    * Input validation: https://issues.apache.org/jira/browse/METRON-1712
    * Kill button for in progress jobs: https://issues.apache.org/jira/browse/METRON-1713
    * Testing different parameter values: https://issues.apache.org/jira/browse/METRON-1718
    
    I believe that's all the tickets that need to be created stemming from the discussion above.


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    In the screenshots you provide, the timestamp field contains both the epoch timestamp and the date.  I'd have to dig in, but based on PDML files I've seen, it's displaying both the "show" and the "value".  Seems like we should either be splitting that display field, or just choosing one.


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1103#discussion_r204138476
  
    --- Diff: metron-interface/metron-alerts/src/app/pcap/service/pcap.service.ts ---
    @@ -0,0 +1,10186 @@
    +import {Injectable, NgZone} from '@angular/core';
    --- End diff --
    
    we need a license header here


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    The other feedback I have is that the unit tests need some work.  For example, when I updated `pcap.request.ts` but hadn't updated `pcap-filters.component.html`, the unit tests still pass.  That template directly depends on the property names in pcap.request.ts and should fail if they don't match.
    
    Also when I run `npm run lint` there was a long list of violations.


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    I have a couple comments after spinning up  #1122 (which should have all these changes, I believe).
    
    - Better input validation would be nice.  I can currently enter negative ports (which don't match anything) or ports beyond max int (which match everything!). Ideally this would also happen in the REST layer, because it kicks off jobs that have nonsense parameters.
    - The UI doesn't appear to allow users to kill jobs (although the REST API exists for it). Is it correct that this isn't supported?
    - The parameters differ if you've never entered a field vs. entered then deleted it.  For example, for ports (and possibly just the two port fields), it'll be empty string if never entered and null if entered then deleted. This appears to be benign, although surprising.
    - Kicking off a job, then navigating to the alerts view, then navigating back allows a user to kick off a second job.  I'm not sure what happens when as these jobs finish in potentially either order (or any order if you repeat this and kick off many jobs).
    - The UI polls for job status infinitely if the REST UI dies.
    - It's odd that the port fields have increment / decrement buttons. I would expect a user to manually enter that every time.
    - I get an NPE if there are no pcaps within the date range provided by the query. This is a backend query thing, so it might be worth creating a separate ticket for it?


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    Can I also get some insight into why the job status REST call and UI usage isn't an asynchronous call? Polling seems pretty avoidable here.


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by mmiklavc <gi...@git.apache.org>.
Github user mmiklavc commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    @tiborm @sardell Can one of you list out the UI PR's and indicate the order in which they should be tested and subsequently merged into the feature branch? Since this is the initial major ticket, you can probably add that as a comment here. In addition, each PR should clearly indicate which PR's it depends on and those that are depending on it.


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by mmiklavc <gi...@git.apache.org>.
Github user mmiklavc commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    >  I'm not sure the Hadoop Job object is asynchronous anyways.
    @merrimanr If I'm right on the context of the Q and A here, the PcapJob around the underlying MR job handles it async or sync. For REST, we expose it asynchronously via the job manager.


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1103#discussion_r204137868
  
    --- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-filters/pcap-filters.component.ts ---
    @@ -0,0 +1,24 @@
    +import { Component, OnInit, Input, Output, EventEmitter } from '@angular/core';
    --- End diff --
    
    we need a license header here


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    Given that unit tests will be addressed in a forthcoming PR, this is good enough for me as an initial UI.  +1 pending other commenters are satisfied.


---

[GitHub] metron issue #1103: Feature/metron 1554 pcap query panel

Posted by mmiklavc <gi...@git.apache.org>.
Github user mmiklavc commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    Some screenshots of your UI changes with accompanying explanation would also be useful so reviewers know what we're looking at.


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by tiborm <gi...@git.apache.org>.
Github user tiborm commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    @cestella We added the license headers for all new files. 


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    This has been merged into the feature branch.  Can you close it @tiborm?


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by tiborm <gi...@git.apache.org>.
Github user tiborm closed the pull request at:

    https://github.com/apache/metron/pull/1103


---

[GitHub] metron pull request #1103: METRON-1554: Initial PCAP UI

Posted by tiborm <gi...@git.apache.org>.
Github user tiborm closed the pull request at:

    https://github.com/apache/metron/pull/1103


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1103#discussion_r204137821
  
    --- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-filters/pcap-filters.component.spec.ts ---
    @@ -0,0 +1,29 @@
    +import { async, ComponentFixture, TestBed } from '@angular/core/testing';
    --- End diff --
    
    Some of these .ts files don't have license headers, how is this passing the rat check?  Do we have a rat exclusion that is too broad?


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1103#discussion_r204137964
  
    --- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-list/pcap-list.component.ts ---
    @@ -0,0 +1,22 @@
    +import { Component, OnInit, Input } from '@angular/core';
    --- End diff --
    
    we need a license header here


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1103#discussion_r204541762
  
    --- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-packet-line/pcap-packet-line.component.ts ---
    @@ -0,0 +1,55 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +import { Component, OnInit, Input } from '@angular/core';
    +import { PdmlPacket, PdmlProto, PdmlField } from '../model/pdml'
    +
    +@Component({
    +  selector: '[app-pcap-packet-line]',
    +  templateUrl: './pcap-packet-line.component.html',
    +  styleUrls: ['./pcap-packet-line.component.scss']
    +})
    +export class PcapPacketLineComponent implements OnInit {
    +
    +  @Input() packet: PdmlPacket
    +
    +  ip: {
    +    timestamp: PdmlField,
    +    ip_src_addr: PdmlField, ip_src_port: PdmlField,
    +    ip_dest_addr: PdmlField, ip_dest_port: PdmlField,
    +    protocol: PdmlField
    +  }
    +
    +  constructor() { }
    +
    +  ngOnInit() {
    +    let gen_proto: PdmlProto = this.packet.protos.filter(p => p.name == "geninfo")[0]
    --- End diff --
    
    These variables do not match the style used across Metron in Typescript files.  We generally follow the [Angular](https://angular.io/guide/styleguide) style guide because this is an Angular application but I'm not sure we have officially agreed on that as a community in a discussion thread.  I will get that going.
    
    I'm fairly confident that this won't match whatever style we choose so I would suggest we make it match our current codebase in the meantime.


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by tiborm <gi...@git.apache.org>.
Github user tiborm commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    @mmiklavc We had to update a package-lock.json file because of the original one contained a package collision. npm ci command just failed on that.


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by tiborm <gi...@git.apache.org>.
Github user tiborm commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    As part of the latest commits I removed commented code blocks, and fixed the variable naming issues in pcap-packet-line.component.ts.
    This PR and the followup ones are updated by the latest changes from the base branch.


---

[GitHub] metron issue #1103: METRON-1554: Initial PCAP UI

Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    I think we should rename from alert ui to investigate or something


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by merrimanr <gi...@git.apache.org>.
Github user merrimanr commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    I tested this in full dev and the fixed query request is incorrect.  Here's what is being sent:
    ```
    {
      "startTime": 0,
      "endTime": 150000000000000000,
      "srcIp": "",
      "srcPort": 22,
      "dstIp": "",
      "dstPort": "",
      "protocol": "",
      "packetFilter": "",
      "includeReverseTraffic": false
    }
    ```
    when it should look like:
    ```
    {
      "startTimeMs": 0,
      "endTimeMs": 150000000000000000,
      "ipSrcAddr": "",
      "ipSrcPort": 22,
      "ipDstAddr": "",
      "ipDstPort": "",
      "protocol": "",
      "packetFilter": "",
      "includeReverse": false
    }
    ```
    I've submitted a PR that fixes the issue.  The UI works as expected with those changes.  Let me know if there is anything missing in that.


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by tiborm <gi...@git.apache.org>.
Github user tiborm commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1103#discussion_r205721365
  
    --- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-packet/pcap-packet.component.ts ---
    @@ -0,0 +1,22 @@
    +import { Component, OnInit, Input } from '@angular/core';
    --- End diff --
    
    Thanks, we added license headers.


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1103#discussion_r204138341
  
    --- Diff: metron-interface/metron-alerts/src/app/pcap/service/pcap.service.spec.ts ---
    @@ -0,0 +1,1735 @@
    +import { TestBed, async, inject } from '@angular/core/testing';
    --- End diff --
    
    we need a license header here


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1103#discussion_r204138121
  
    --- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-packet-line/pcap-packet-line.component.spec.ts ---
    @@ -0,0 +1,1266 @@
    +import { async, ComponentFixture, TestBed } from '@angular/core/testing';
    --- End diff --
    
    we need a license header here


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by james-sirota <gi...@git.apache.org>.
Github user james-sirota commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    I reviewed the UI.  Everything comes up fine and looks good.  One assumption I am making is that protocol is numeric because the protocol field will not allow me to enter a character.  +1


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by mmiklavc <gi...@git.apache.org>.
Github user mmiklavc commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    @tiborm - I see a lot of recent changes to dependency versions in the package lock file - is that intentional? Just a short comment about what it's for would be helpful.


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by sardell <gi...@git.apache.org>.
Github user sardell commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    @justinleet In response to your UI-related questions:
    
    > Better input validation would be nice. I can currently enter negative ports (which don't match anything) or ports beyond max int (which match everything!). Ideally this would also happen in the REST layer, because it kicks off jobs that have nonsense parameters.
    
    It looks like @ruffle1986 added some validation [in this branch](https://github.com/ruffle1986/metron/commit/1c4bc69eb55d1ba96ce6d101516099d2a788ecf7). Are you fine with this being a follow up PR?
    
    > The UI doesn't appear to allow users to kill jobs (although the REST API exists for it). Is it correct that this isn't supported?
    
    In the current iteration of the UI, a button to kill jobs is not available. I agree that we should add this to the UI, but if it's okay with you I think this could be a follow on PR. 
    
    > The parameters differ if you've never entered a field vs. entered then deleted it. For example, for ports (and possibly just the two port fields), it'll be empty string if never entered and null if entered then deleted. This appears to be benign, although surprising.
    
    Nice catch. I think we should open a ticket to investigate this further.
    
    > Kicking off a job, then navigating to the alerts view, then navigating back allows a user to kick off a second job. I'm not sure what happens when as these jobs finish in potentially either order (or any order if you repeat this and kick off many jobs).
    
    If a job is now running, an error is returned from the API. The UI work to handle this is in [this commit](https://github.com/apache/metron/pull/1122/commits/60f5d76d62f9b23dd44d9890b05c0101ae2b2080).
    
    > It's odd that the port fields have increment / decrement buttons. I would expect a user to manually enter that every time.
    
    It looks like this was resolved by @ruffle1986 [in this branch](https://github.com/ruffle1986/metron/commit/1c4bc69eb55d1ba96ce6d101516099d2a788ecf7) by removing the type attribute on the input.
    
    



---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by tiborm <gi...@git.apache.org>.
Github user tiborm commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1103#discussion_r205714935
  
    --- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-filters/pcap-filters.component.ts ---
    @@ -0,0 +1,24 @@
    +import { Component, OnInit, Input, Output, EventEmitter } from '@angular/core';
    --- End diff --
    
    Thanks, we added license headers.


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by sardell <gi...@git.apache.org>.
Github user sardell commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1103#discussion_r205746588
  
    --- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-packet-line/pcap-packet-line.component.html ---
    @@ -0,0 +1,19 @@
    +<!--
    +  Licensed to the Apache Software
    +	Foundation (ASF) under one or more contributor license agreements. See the
    +	NOTICE file distributed with this work for additional information regarding
    +	copyright ownership. The ASF licenses this file to You under the Apache License,
    +	Version 2.0 (the "License"); you may not use this file except in compliance
    +	with the License. You may obtain a copy of the License at
    +  http://www.apache.org/licenses/LICENSE-2.0
    +  Unless required by applicable law or agreed to in writing, software distributed
    +	under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES
    +	OR CONDITIONS OF ANY KIND, either express or implied. See the License for
    +  the specific language governing permissions and limitations under the License.
    +  -->
    +<td class="timestamp">{{ip.timestamp.value}} <span class="date">{{ip.timestamp.show}}</span></td>
    --- End diff --
    
    @justinleet [I created a ticket](https://issues.apache.org/jira/browse/METRON-1698) to get some feedback on whether we should be displaying both field values as separate fields in the UI or if we should just choose one. I'm going to hold off on making changes to this code until we get some feedback from the community.


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by tiborm <gi...@git.apache.org>.
Github user tiborm commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    Thanks to everyone for the feedback and comments!


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1103#discussion_r204138244
  
    --- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-panel/pcap-panel.component.spec.ts ---
    @@ -0,0 +1,51 @@
    +import { async, ComponentFixture, TestBed } from '@angular/core/testing';
    --- End diff --
    
    we need a license header here


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by tiborm <gi...@git.apache.org>.
Github user tiborm commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1103#discussion_r205721981
  
    --- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-panel/pcap-panel.component.spec.ts ---
    @@ -0,0 +1,51 @@
    +import { async, ComponentFixture, TestBed } from '@angular/core/testing';
    --- End diff --
    
    Thanks, we added license headers.


---

[GitHub] metron issue #1103: METRON-1671: Initial PCAP UI

Posted by tiborm <gi...@git.apache.org>.
Github user tiborm commented on the issue:

    https://github.com/apache/metron/pull/1103
  
    @mmiklavc The list of the sequential PR's in order are the following:
    
    METRON-1671: Initial PCAP UI
    https://github.com/apache/metron/pull/1103
     
    METRON-1662: Adding download button
    https://github.com/apache/metron/pull/1118
     
    METRON-1676: Adding date range selector to PCAP filter bar
    https://github.com/apache/metron/pull/1119
     
    METRON-1675: Add Pagination to PCAP
    https://github.com/apache/metron/pull/1121
     
    METRON-1683: Fix the download progress bar in PCAP UI
    https://github.com/apache/metron/pull/1122


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by cestella <gi...@git.apache.org>.
Github user cestella commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1103#discussion_r204137924
  
    --- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-list/pcap-list.component.spec.ts ---
    @@ -0,0 +1,53 @@
    +import { async, ComponentFixture, TestBed } from '@angular/core/testing';
    --- End diff --
    
    we need a license header here


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1103#discussion_r205108455
  
    --- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-packet-line/pcap-packet-line.component.html ---
    @@ -0,0 +1,19 @@
    +<!--
    +  Licensed to the Apache Software
    +	Foundation (ASF) under one or more contributor license agreements. See the
    +	NOTICE file distributed with this work for additional information regarding
    +	copyright ownership. The ASF licenses this file to You under the Apache License,
    +	Version 2.0 (the "License"); you may not use this file except in compliance
    +	with the License. You may obtain a copy of the License at
    +  http://www.apache.org/licenses/LICENSE-2.0
    +  Unless required by applicable law or agreed to in writing, software distributed
    +	under the License is distributed on an "AS IS" BASIS, WITHOUT WARRANTIES
    +	OR CONDITIONS OF ANY KIND, either express or implied. See the License for
    +  the specific language governing permissions and limitations under the License.
    +  -->
    +<td class="timestamp">{{ip.timestamp.value}} <span class="date">{{ip.timestamp.show}}</span></td>
    --- End diff --
    
    This looks like the culprit for the double timestamp/data showing


---

[GitHub] metron pull request #1103: METRON-1671: Initial PCAP UI

Posted by justinleet <gi...@git.apache.org>.
Github user justinleet commented on a diff in the pull request:

    https://github.com/apache/metron/pull/1103#discussion_r204532402
  
    --- Diff: metron-interface/metron-alerts/src/app/pcap/pcap-panel/pcap-panel.component.ts ---
    @@ -0,0 +1,78 @@
    +/**
    + * Licensed to the Apache Software Foundation (ASF) under one
    + * or more contributor license agreements.  See the NOTICE file
    + * distributed with this work for additional information
    + * regarding copyright ownership.  The ASF licenses this file
    + * to you under the Apache License, Version 2.0 (the
    + * "License"); you may not use this file except in compliance
    + * with the License.  You may obtain a copy of the License at
    + *
    + *     http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +import { Component, OnInit, Input } from '@angular/core';
    +
    +import { PcapService, PcapStatusResponse } from '../service/pcap.service'
    +import { PcapRequest } from '../model/pcap.request'
    +import { Pdml } from '../model/pdml'
    +import {Subscription} from "rxjs/Rx";
    +
    +@Component({
    +  selector: 'app-pcap-panel',
    +  templateUrl: './pcap-panel.component.html',
    +  styleUrls: ['./pcap-panel.component.scss']
    +})
    +export class PcapPanelComponent implements OnInit {
    +
    +  @Input() pdml: Pdml = null;
    +
    +  @Input() pcapRequest: PcapRequest;
    +
    +  statusSubscription: Subscription;
    +  queryRunning: boolean = false;
    +  progressWidth: number = 0;
    +  selectedPage: number = 1;
    +
    +  constructor(private pcapService: PcapService ) { }
    +
    +  ngOnInit() {
    +  }
    +
    +  onSearch(pcapRequest) {
    +    console.log(pcapRequest);
    +    this.pdml = null;
    +    this.progressWidth = 0;
    +    this.pcapService.submitRequest(pcapRequest).subscribe(id => {
    +      this.queryRunning = true;
    +      this.statusSubscription = this.pcapService.pollStatus(id).subscribe((statusResponse: PcapStatusResponse) => {
    +        if ('SUCCEEDED' === statusResponse.jobStatus) {
    --- End diff --
    
    We also need to handle FAILED and KILLED states


---