[GitHub] metron issue #481: METRON-322 Global Batching and Flushing
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/481 +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #481: METRON-322 Global Batching and Flushing
Github user ottobackwards commented on a diff in the pull request: https://github.com/apache/metron/pull/481#discussion_r127087711 --- Diff: metron-platform/metron-writer/src/main/java/org/apache/metron/writer/bolt/BulkMessageWriterBolt.java --- @@ -92,24 +177,58 @@ public void prepare(Map stormConf, TopologyContext context, OutputCollector coll configurationTransformation = x -> x; } try { - bulkMessageWriter.init(stormConf, - context, - configurationTransformation.apply(new IndexingWriterConfiguration(bulkMessageWriter.getName(), - getConfigurations())) -); + WriterConfiguration writerconf = configurationTransformation.apply( + new IndexingWriterConfiguration(bulkMessageWriter.getName(), getConfigurations())); + if (defaultBatchTimeout == 0) { +//This means getComponentConfiguration was never called to initialize defaultBatchTimeout, +//probably because we are in a unit test scenario. So calculate it here. +BatchTimeoutHelper timeoutHelper = new BatchTimeoutHelper(writerconf::getAllConfiguredTimeouts, batchTimeoutDivisor); +defaultBatchTimeout = timeoutHelper.getDefaultBatchTimeout(); + } + writerComponent.setDefaultBatchTimeout(defaultBatchTimeout); + bulkMessageWriter.init(stormConf, context, writerconf); } catch (Exception e) { throw new RuntimeException(e); } } + /** + * Used only for unit testing. + */ --- End diff -- let the intellisense fall where it may then --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #649: METRON-1035 Added SUM to the rules triage aggregat...
GitHub user simonellistonball opened a pull request: https://github.com/apache/metron/pull/649 METRON-1035 Added SUM to the rules triage aggregation docs ## Contributor Comments Quick doc fix verified against code. ## Pull Request Checklist ### 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- where 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 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](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/simonellistonball/incubator-metron METRON-1035 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/metron/pull/649.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 #649 commit d2604086c52f00656ffab23b8b4eab1d30aeecee Author: Simon Elliston Ball Date: 2017-07-12T22:12:31Z Added SUM to the rules triage aggregation docs --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron issue #481: METRON-322 Global Batching and Flushing
Github user mattf-horton commented on the issue: https://github.com/apache/metron/pull/481 @ottobackwards , thanks for looking at this. I did not see those error behaviors while running integration tests. Please tell me the environment and the exact invocation you used. Thanks. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #481: METRON-322 Global Batching and Flushing
Github user mattf-horton commented on a diff in the pull request: https://github.com/apache/metron/pull/481#discussion_r127081873 --- Diff: metron-platform/metron-writer/src/main/java/org/apache/metron/writer/bolt/BulkMessageWriterBolt.java --- @@ -92,24 +177,58 @@ public void prepare(Map stormConf, TopologyContext context, OutputCollector coll configurationTransformation = x -> x; } try { - bulkMessageWriter.init(stormConf, - context, - configurationTransformation.apply(new IndexingWriterConfiguration(bulkMessageWriter.getName(), - getConfigurations())) -); + WriterConfiguration writerconf = configurationTransformation.apply( + new IndexingWriterConfiguration(bulkMessageWriter.getName(), getConfigurations())); + if (defaultBatchTimeout == 0) { +//This means getComponentConfiguration was never called to initialize defaultBatchTimeout, +//probably because we are in a unit test scenario. So calculate it here. +BatchTimeoutHelper timeoutHelper = new BatchTimeoutHelper(writerconf::getAllConfiguredTimeouts, batchTimeoutDivisor); +defaultBatchTimeout = timeoutHelper.getDefaultBatchTimeout(); + } + writerComponent.setDefaultBatchTimeout(defaultBatchTimeout); + bulkMessageWriter.init(stormConf, context, writerconf); } catch (Exception e) { throw new RuntimeException(e); } } + /** + * Used only for unit testing. + */ --- End diff -- Hm. With respect, this one I don't really agree with. It's just a polymorphism of the regular prepare() method. Since its signature differs from the "regular" prepare() method, system code will never call it. It is documented, at javadoc level, as being only for use in unit testing, which is by definition code-level. Those who aren't willing to look at code shouldn't be using it, and indeed will have a difficult time finding it at all. I'd rather let the polymorphism be evident. I will, however, add further comment that this is "used only for unit testing, with injection of a FakeClock for mocking passage of time in a controlled way, to test queue timeout behavior in the WriterComponent." Would that be okay? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #481: METRON-322 Global Batching and Flushing
Github user mattf-horton commented on a diff in the pull request: https://github.com/apache/metron/pull/481#discussion_r127077516 --- Diff: metron-platform/metron-enrichment/src/test/java/org/apache/metron/enrichment/bolt/BulkMessageWriterBoltTest.java --- @@ -164,4 +174,100 @@ public void test() throws Exception { verify(outputCollector, times(1)).emit(eq(Constants.ERROR_STREAM), any(Values.class)); verify(outputCollector, times(1)).reportError(any(Throwable.class)); } + + @Test --- End diff -- @ottobackwards , how about "Mocking sucks"? :-) Or at least "Mocking complex objects is excruciating." But seriously, yes I'll add some comments. Not planning on a line-by-line walk-thru, tho. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #481: METRON-322 Global Batching and Flushing
Github user mattf-horton commented on a diff in the pull request: https://github.com/apache/metron/pull/481#discussion_r127076248 --- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/configuration/writer/SingleBatchConfigurationFacade.java --- @@ -18,6 +18,8 @@ package org.apache.metron.common.configuration.writer; +import java.util.ArrayList; +import java.util.List; import java.util.Map; --- End diff -- Well, there's some weirdness in this stuff, and I didn't try to get rid of it lest I break something I didn't understand. In this case (and other places), it appears that non-batched writers are essentially faked by using batched writers with a batchSize of "1". Doesn't seem real efficient to me, but obviously decreases the overall quantity of code and complexity of testing. Anyway, since I only did a trivial change to this file, I didn't feel obligated to try to explain it. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[REQUEST] Contributor rights in Jira
Hello, Could a PMC member please grant my Jira account contributor rights? I'd like to start helping out with various smaller tasks. I promise I won't mess stuff up and go to IRC first for any questions/comments/additions. Otto has been extremely helpful there already :) Thanks, Laurens
[GitHub] metron issue #481: METRON-322 Global Batching and Flushing
Github user mattf-horton commented on the issue: https://github.com/apache/metron/pull/481 Comment on testing: There are so many permutations it only seemed reasonable to automate them in unit test, and so I did. As part of code review, please provide your opinion on whether the provided unit tests are adequate, or what additional test cases should be added. Manual end-to-end testing, if you are so moved, consists of six scenarios for a given sensor queue: 1. Under **heavy continuous load** the batchSize still controls flushing behavior, because the queue size always exceeds batchSize before queue age exceeds batchTimeout. 2. Under **light continuous load**, where each queue continues to receive at least one message per second, and batchSize is large enough it is never exceeded, then the batchTimeout for each queue should control flushing behavior within +/- 1 sec, because each new message triggers a check of the queue age and potential timeout flush. - NOTE: If the configured batchTimeout is set to a large number, bigger than `1/2 topology.message.timeout.secs - 1` (which equals **14 sec** by default), then it will be replaced by an effective value equal to `1/2 topology.message.timeout.secs - 1`. Flushing will occur within +/- 1 sec of each _effective_ batchTimeout interval, rather than the _configured_ batchTimeout interval. 3. Under **light intermittent load**, where less than batchSize messages queue up, and gaps between messages may exceed the timeout interval, then age checks and potential flush events may be triggered by _either_ incoming messages or TickTuple events, depending on the phase relationship between intermittent bursts of messages, and the TickTuple system tick. The TickTuple interval is guaranteed to be < `1/2 topology.message.timeout.secs`, hence the default TickTuple interval is 14 seconds. But if the smallest batchTimeout configured for any sensor queue on the Bolt is < the default TickTuple interval, then that smallest value becomes the actual TickTuple interval. This produces three sub-cases, all of which guarantee a flush event before any message gets recycled due to aging past `topology.message.timeout.secs`: - If the queue's configured batchTimeout is the smallest (or only) such on this Bolt, and that number is smaller than the default TickTuple interval, then it _becomes_ the actual TickTuple interval. The queue is guaranteed to flush between 1x and 2x this interval. - If the queue's configured batchTimeout is not the smallest such, but still is < the default TickTuple interval, then the queue is guaranteed to flush between its own `configured batchTimeout` and its `configured batchTimeout + actual TickTuple interval` (which is less than 2x its own `configured batchTimeout`). - If the queue's configured batchTimeout is > the default TickTuple interval (14 sec default), then its effective batchTimeout is set to the default TickTuple interval. The queue is guaranteed to flush between this `effective batchTimeout` and its `effective batchTimeout + actual TickTuple interval`. The upshot is that: * "Configured batchTimeout" should be thought of as "minimum age before you'll allow a time-based flush" (capped by default TickTuple interval, aka 1/2 `topology.message.timeout.secs`) * "Actual TickTuple interval" is the "maximum time between age checks". It will be <= all the configured batchTimeouts for the various sensors on the Bolt. * When a flush actually happens may be up to "effective batchTimeout" + "actual TickTuple interval", depending on exactly when intermittent message events and periodic Tick events happen. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[REQUEST] Wiki Edit Rights
Could a PMC member please grant my account edit rights to the Metron space in Confluence? I'd like to start helping with the clean up efforts there. Thanks, Kyle
[GitHub] metron issue #481: METRON-322 Global Batching and Flushing
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/481 I am seeing errors like ```bash --- T E S T S --- Running org.apache.metron.solr.integration.SolrIndexingIntegrationTest Waiting for zookeeper... Found index config in zookeeper... 0 vs 10 vs 0 0 vs 10 vs 0 0 vs 10 vs 0 0 vs 10 vs 0 0 vs 10 vs 0 0 vs 10 vs 0 Processed target/indexingIntegrationTest/hdfs/test/enrichment-hdfsIndexingBolt-1-0-1499888936956.json 10 vs 10 vs 10 Processed target/indexingIntegrationTest/hdfs/test/enrichment-hdfsIndexingBolt-1-0-1499888936956.json 2017-07-12 15:48:57 ERROR ClientCnxn:524 - Error while calling watcher java.util.concurrent.RejectedExecutionException: Task org.apache.solr.common.util.ExecutorUtil$MDCAwareThreadPoolExecutor$1@9651f04 rejected from org.apache.solr.common.util.ExecutorUtil$MDCAwareThreadPoolExecutor@d5a9af0[Terminated, pool size = 0, active threads = 0, queued tasks = 0, completed tasks = 16] at java.util.concurrent.ThreadPoolExecutor$AbortPolicy.rejectedExecution(ThreadPoolExecutor.java:2047) at java.util.concurrent.ThreadPoolExecutor.reject(ThreadPoolExecutor.java:823) at java.util.concurrent.ThreadPoolExecutor.execute(ThreadPoolExecutor.java:1369) at org.apache.solr.common.util.ExecutorUtil$MDCAwareThreadPoolExecutor.execute(ExecutorUtil.java:135) at java.util.concurrent.AbstractExecutorService.submit(AbstractExecutorService.java:112) at org.apache.solr.common.cloud.SolrZkClient$3.process(SolrZkClient.java:261) at org.apache.zookeeper.ClientCnxn$EventThread.processEvent(ClientCnxn.java:522) at org.apache.zookeeper.ClientCnxn$EventThread.run(ClientCnxn.java:498) 2017-07-12 15:49:01 ERROR ReadClusterState:181 - Failed to Sync Supervisor java.lang.RuntimeException: java.lang.InterruptedException at org.apache.storm.utils.Utils.wrapInRuntime(Utils.java:1507) at org.apache.storm.zookeeper.Zookeeper.getChildren(Zookeeper.java:260) at org.apache.storm.cluster.ZKStateStorage.get_children(ZKStateStorage.java:174) at org.apache.storm.cluster.StormClusterStateImpl.assignments(StormClusterStateImpl.java:153) at org.apache.storm.daemon.supervisor.ReadClusterState.run(ReadClusterState.java:126) at org.apache.storm.event.EventManagerImp$1.run(EventManagerImp.java:54) Caused by: java.lang.InterruptedException at java.lang.Object.wait(Native Method) at java.lang.Object.wait(Object.java:502) at org.apache.storm.shade.org.apache.zookeeper.ClientCnxn.submitRequest(ClientCnxn.java:1342) at org.apache.storm.shade.org.apache.zookeeper.ZooKeeper.getChildren(ZooKeeper.java:1588) at org.apache.storm.shade.org.apache.zookeeper.ZooKeeper.getChildren(ZooKeeper.java:1625) at org.apache.storm.shade.org.apache.curator.framework.imps.GetChildrenBuilderImpl$3.call(GetChildrenBuilderImpl.java:210) at org.apache.storm.shade.org.apache.curator.framework.imps.GetChildrenBuilderImpl$3.call(GetChildrenBuilderImpl.java:203) at org.apache.storm.shade.org.apache.curator.RetryLoop.callWithRetry(RetryLoop.java:108) at org.apache.storm.shade.org.apache.curator.framework.imps.GetChildrenBuilderImpl.pathInForeground(GetChildrenBuilderImpl.java:200) at org.apache.storm.shade.org.apache.curator.framework.imps.GetChildrenBuilderImpl.forPath(GetChildrenBuilderImpl.java:191) at org.apache.storm.shade.org.apache.curator.framework.imps.GetChildrenBuilderImpl.forPath(GetChildrenBuilderImpl.java:38) at org.apache.storm.zookeeper.Zookeeper.getChildren(Zookeeper.java:255) ... 4 more Tests run: 1, Failures: 0, Errors: 0, Skipped: 0, Time elapsed: 45.675 sec - in org.apache.metron.solr.integration.SolrIndexingIntegrationTest Results : ``` Have you seen these? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron issue #481: METRON-322 Global Batching and Flushing
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/481 This is a lot of work, and a great job @mattf-horton. A couple of nit comments I would like to get response to before a +1 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #481: METRON-322 Global Batching and Flushing
Github user ottobackwards commented on a diff in the pull request: https://github.com/apache/metron/pull/481#discussion_r127053228 --- Diff: metron-platform/metron-writer/src/main/java/org/apache/metron/writer/bolt/BulkMessageWriterBolt.java --- @@ -92,24 +177,58 @@ public void prepare(Map stormConf, TopologyContext context, OutputCollector coll configurationTransformation = x -> x; } try { - bulkMessageWriter.init(stormConf, - context, - configurationTransformation.apply(new IndexingWriterConfiguration(bulkMessageWriter.getName(), - getConfigurations())) -); + WriterConfiguration writerconf = configurationTransformation.apply( + new IndexingWriterConfiguration(bulkMessageWriter.getName(), getConfigurations())); + if (defaultBatchTimeout == 0) { +//This means getComponentConfiguration was never called to initialize defaultBatchTimeout, +//probably because we are in a unit test scenario. So calculate it here. +BatchTimeoutHelper timeoutHelper = new BatchTimeoutHelper(writerconf::getAllConfiguredTimeouts, batchTimeoutDivisor); +defaultBatchTimeout = timeoutHelper.getDefaultBatchTimeout(); + } + writerComponent.setDefaultBatchTimeout(defaultBatchTimeout); + bulkMessageWriter.init(stormConf, context, writerconf); } catch (Exception e) { throw new RuntimeException(e); } } + /** + * Used only for unit testing. + */ --- End diff -- Maybe we can name this testPrepare? So it is explicit to caller, who may not 'peek' inside the source --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #481: METRON-322 Global Batching and Flushing
Github user ottobackwards commented on a diff in the pull request: https://github.com/apache/metron/pull/481#discussion_r127052398 --- Diff: metron-platform/metron-enrichment/src/test/java/org/apache/metron/enrichment/bolt/BulkMessageWriterBoltTest.java --- @@ -164,4 +174,100 @@ public void test() throws Exception { verify(outputCollector, times(1)).emit(eq(Constants.ERROR_STREAM), any(Values.class)); verify(outputCollector, times(1)).reportError(any(Throwable.class)); } + + @Test --- End diff -- Maintainers will thank you for throwing some comments here --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #481: METRON-322 Global Batching and Flushing
Github user ottobackwards commented on a diff in the pull request: https://github.com/apache/metron/pull/481#discussion_r127051759 --- Diff: metron-platform/metron-common/src/main/java/org/apache/metron/common/configuration/writer/SingleBatchConfigurationFacade.java --- @@ -18,6 +18,8 @@ package org.apache.metron.common.configuration.writer; +import java.util.ArrayList; +import java.util.List; import java.util.Map; --- End diff -- It would be nice if there was some javadoc on this class as to it's purpose and usage --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
code review: adding time-based flushing to BulkMessageWriterBolt
I think PR#481, for the first two-thirds of METRON322 (sub-tasks 2, 3, 4, and 7), is ready for review and inclusion. If folks have time to review it would be appreciated. Thanks, --Matt
[GitHub] metron pull request #648: METRON-1033 Corrected profiler docs units on expir...
GitHub user simonellistonball opened a pull request: https://github.com/apache/metron/pull/648 METRON-1033 Corrected profiler docs units on expires field Minor change to update profiler docs ## Contributor Comments [Please place any comments here. A description of the problem/enhancement, how to reproduce the issue, your testing methodology, etc.] ## 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- where 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 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`: You can merge this pull request into a Git repository by running: $ git pull https://github.com/simonellistonball/incubator-metron METRON-1033 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/metron/pull/648.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 #648 commit 880e10cfc31f49cb11a3f5639c2c0a217743c044 Author: Simon Elliston Ball Date: 2017-07-12T16:30:25Z Corrected profiler docs units on expires field --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #631: METRON-1019: Metron 0.4.0 manual installation guid...
Github user asfgit closed the pull request at: https://github.com/apache/metron/pull/631 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #639: METRON-1013 add command line verification to stell...
Github user asfgit closed the pull request at: https://github.com/apache/metron/pull/639 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #647: METRON-1031: Management UI Cannot Start Topologies...
GitHub user nickwallen opened a pull request: https://github.com/apache/metron/pull/647 METRON-1031: Management UI Cannot Start Topologies in Kerberized Environment The Metron Management UI cannot start topologies in a kerberized environment. This includes the Parser, Enrichment, and Indexing topologies. It seems that Metron REST does not pass "-ksp" to the start topology commands. As a result, topologies that are started with the Management UI cannot start a producer against a kerberized Kafka. ### Changes * Altered the `StormCLIWrapper` to add logging of the actual CLI commands that are executed. * The `-ksp` argument is always used (with Kerberos or not) when launching the Parser, Enrichment, and Indexing topologies. * Added a Spring property `kafka.security.protocol` that serves as the `-ksp` argument when launching the topologies. * Ambari sets this Spring property based on Kafka's own configuration in Ambari. ### Testing To manually test this, validate that all of the topologies can be stopped and started using the Management UI and Swagger UI both before and after kerberization. 1. Launch the Full Dev environment. 1. Stop then start the Parser topologies using the Management UI. 1. Stop then start the Enrichment and Indexing topologies using the Swagger UI. 1. Kerberize the Full Dev environment. 1. Stop then start the Parser topologies using the Management UI. 1. Stop then start the Enrichment and Indexing topologies using the Swagger UI. ### Pull Request Checklist - [ ] 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- where 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)? - [ ] 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](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? You can merge this pull request into a Git repository by running: $ git pull https://github.com/nickwallen/metron METRON-1031 Alternatively you can review and apply these changes as the patch at: https://github.com/apache/metron/pull/647.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 #647 commit 6cff444a1120ca356aa4e39f54981f2d43b5 Author: Nick Allen Date: 2017-07-11T23:05:26Z METRON-1031: Management UI Cannot Start Topologies in Kerberized Environment commit a70ad836f9a08e58dc97c0cfc4d5dcaabf5c818b Author: Nick Allen Date: 2017-07-12T13:52:04Z Configuration fixup --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #620: Metron-988: UI for viewing alerts generated by Met...
Github user iraghumitra commented on a diff in the pull request: https://github.com/apache/metron/pull/620#discussion_r126957989 --- Diff: metron-interface/metron-alerts/src/app/service/data-source.ts --- @@ -0,0 +1,62 @@ +/** + * 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 {Observable} from 'rxjs/Rx'; +import {Injectable} from '@angular/core'; +import {Http} from '@angular/http'; + +import {Alert} from '../model/alert'; +import {ColumnMetadata} from '../model/column-metadata'; +import {ColumnNames} from '../model/column-names'; +import {TableMetadata} from '../model/table-metadata'; +import {SaveSearch} from '../model/save-search'; +import {AlertsSearchResponse} from '../model/alerts-search-response'; +import {SearchRequest} from '../model/search-request'; + +@Injectable() +export abstract class DataSource { + defaultHeaders: {'Content-Type': 'application/json', 'X-Requested-With': 'XMLHttpRequest'}; + + constructor(protected http: Http) {} + + // Calls to fetch alerts + abstract getAlerts(searchRequest: SearchRequest): Observable + abstract getAlert(index: string, type: string, alertId: string): Observable + abstract updateAlertState(request: any): Observable<{}> + + // Calls to fetch default alert table column names and all the field names across all indexes + abstract getDefaultAlertTableColumnNames(): Observable + abstract getAllFieldNames(): Observable + + // Calls to rename field names and to fetch the renamed field names + abstract getAlertTableColumnNames(): Observable + abstract saveAlertTableColumnNames(columns: ColumnNames[]): Observable<{}> + + // Calls to fetch and save alerts table settings like refresh interval, page size, default selected table column names + abstract getAlertTableSettings(): Observable --- End diff -- The idea here is to provide an abstraction for all the API's not just the ES API's. In theory, if we want GUI to use rest API instead of directly querying ES and storing data in the local browser cache the API provider should support all the API's defined in data-source.ts. The API's can be written in phases we would create a separate Impl class that extends Datasource and use the API's that are available. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron issue #620: Metron-988: UI for viewing alerts generated by Metron
Github user mraliagha commented on the issue: https://github.com/apache/metron/pull/620 @iraghumitra We are using ASA and CEF parsers. Can't you get the field names dynamically from Elasticsearch? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron issue #639: METRON-1013 add command line verification to stellar shel...
Github user cestella commented on the issue: https://github.com/apache/metron/pull/639 +1 by inspection --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #634: METRON-1017: Ambari components should be separate
Github user asfgit closed the pull request at: https://github.com/apache/metron/pull/634 --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #620: Metron-988: UI for viewing alerts generated by Met...
Github user iraghumitra commented on a diff in the pull request: https://github.com/apache/metron/pull/620#discussion_r126949623 --- Diff: metron-interface/metron-alerts/src/app/model/alert.ts --- @@ -0,0 +1,44 @@ +export class Alert { --- End diff -- I agree the name alert might be a misnomer the accurate name should reflect that this class holds all the data in the ES indexes. Any suggestions? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron issue #620: Metron-988: UI for viewing alerts generated by Metron
Github user iraghumitra commented on the issue: https://github.com/apache/metron/pull/620 @mraliagha I guess I found the issue. I was fetching the column names only from bro indexes. I fixed it in the latest commit. Do you have bro indexes ? --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #620: Metron-988: UI for viewing alerts generated by Met...
Github user iraghumitra commented on a diff in the pull request: https://github.com/apache/metron/pull/620#discussion_r126948453 --- Diff: metron-interface/metron-alerts/src/app/utils/elasticsearch-utils.ts --- @@ -0,0 +1,72 @@ +/** + * 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 {ColumnMetadata} from '../model/column-metadata'; +import {AlertsSearchResponse} from '../model/alerts-search-response'; + +export class ElasticsearchUtils { + + private static createColumMetaData(properties: any, columnMetadata: ColumnMetadata[], seen: string[]) { + try { + let columnNames = Object.keys(properties); + for (let columnName of columnNames) { + if (seen.indexOf(columnName) === -1) { + seen.push(columnName); + columnMetadata.push( + new ColumnMetadata(columnName, (properties[columnName].type ? properties[columnName].type.toUpperCase() : '')) + ); + } + } + } catch (e) {} + } + + public static extractColumnNameData(res: Response): ColumnMetadata[] { +let response: any = res || {}; +let columnMetadata: ColumnMetadata[] = []; +let seen: string[] = []; + +for (let index in response.metadata.indices) { + if (index.startsWith('bro') || index.startsWith('bro') || index.startsWith('bro')) { --- End diff -- Yup, good catch replaced the condition with much better one. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #620: Metron-988: UI for viewing alerts generated by Met...
Github user iraghumitra commented on a diff in the pull request: https://github.com/apache/metron/pull/620#discussion_r126948282 --- Diff: metron-interface/metron-alerts/src/app/service/elasticsearch-localstorage-impl.ts --- @@ -0,0 +1,291 @@ +/** + * 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 {Observable} from 'rxjs/Rx'; +import {Headers, RequestOptions} from '@angular/http'; + +import {HttpUtil} from '../utils/httpUtil'; +import {DataSource} from './data-source'; +import {Alert} from '../model/alert'; +import {ColumnMetadata} from '../model/column-metadata'; +import {ElasticsearchUtils} from '../utils/elasticsearch-utils'; +import { + ALERTS_COLUMN_NAMES, ALERTS_TABLE_METADATA, ALERTS_RECENT_SEARCH, + ALERTS_SAVED_SEARCH, NUM_SAVED_SEARCH +} from '../utils/constants'; +import {ColumnNames} from '../model/column-names'; +import {ColumnNamesService} from './column-names.service'; +import {TableMetadata} from '../model/table-metadata'; +import {SaveSearch} from '../model/save-search'; +import {AlertsSearchResponse} from '../model/alerts-search-response'; +import {SearchRequest} from '../model/search-request'; + +export class ElasticSearchLocalstorageImpl extends DataSource { + + private defaultColumnMetadata = [ +new ColumnMetadata('_id', 'string'), +new ColumnMetadata('timestamp', 'date'), +new ColumnMetadata('source:type', 'string'), +new ColumnMetadata('ip_src_addr', 'ip'), +new ColumnMetadata('enrichments:geo:ip_dst_addr:country', 'string'), +new ColumnMetadata('ip_dst_addr', 'ip'), +new ColumnMetadata('host', 'string'), +new ColumnMetadata('alert_status', 'string') + ]; + + getAlerts(searchRequest: SearchRequest): Observable { +let url = '/search/*,-*kibana/_search'; +return this.http.post(url, searchRequest, new RequestOptions({headers: new Headers(this.defaultHeaders)})) --- End diff -- Fixed --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #620: Metron-988: UI for viewing alerts generated by Met...
Github user iraghumitra commented on a diff in the pull request: https://github.com/apache/metron/pull/620#discussion_r126948073 --- Diff: metron-interface/metron-alerts/src/app/model/search-request.ts --- @@ -0,0 +1,7 @@ +export class SearchRequest { + _source: string[]; + query = { query_string: { query: '' } }; --- End diff -- Fixed the type to string --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #620: Metron-988: UI for viewing alerts generated by Met...
Github user iraghumitra commented on a diff in the pull request: https://github.com/apache/metron/pull/620#discussion_r126947986 --- Diff: metron-interface/metron-alerts/src/app/alerts/saved-searches/saved-searches.component.ts --- @@ -0,0 +1,124 @@ +import { Component, OnInit } from '@angular/core'; +import {Router} from '@angular/router'; +import {Observable} from 'rxjs/Rx'; + +import {SaveSearchService} from '../../service/save-search.service'; +import {SaveSearch} from '../../model/save-search'; +import {MetronDialogBox} from '../../shared/metron-dialog-box'; +import {NUM_SAVED_SEARCH} from '../../utils/constants'; + +@Component({ + selector: 'app-saved-searches', + templateUrl: './saved-searches.component.html', + styleUrls: ['./saved-searches.component.scss'] +}) +export class SavedSearchesComponent implements OnInit { + + searches: SaveSearch[]; + recentSearcheObj: SaveSearch[]; + savedSearches: any = {}; --- End diff -- There was a separate model to pass data to collapse component the type declaration for it was missing here. Since i was looking at the code after a while i got a chance to improve now :). The code looks much better now. In gist I fixed this --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #620: Metron-988: UI for viewing alerts generated by Met...
Github user iraghumitra commented on a diff in the pull request: https://github.com/apache/metron/pull/620#discussion_r126947375 --- Diff: metron-interface/metron-alerts/src/app/alerts/alerts-list/query-builder.ts --- @@ -0,0 +1,139 @@ +/** + * 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 {Filter} from '../../model/filter'; +import {ColumnNamesService} from '../../service/column-names.service'; +import {SearchRequest} from '../../model/search-request'; + +export class QueryBuilder { + private _searchRequest = new SearchRequest(); + private _query = '*'; + private _displayQuery = this._query; + private _filters: Filter[] = []; + + set query(value: string) { +value = value.replace(/\\:/g, ':'); +this._query = value; +this.updateFilters(this._query, false); +this.onSearchChange(); + } + + get query(): string { +return this._query; + } + + set displayQuery(value: string) { +this._displayQuery = value; +this.updateFilters(this._displayQuery, true); +this.onSearchChange(); + } + + get displayQuery(): string { +return this._displayQuery; + } + + get filters(): Filter[] { +return this._filters; + } + + + get searchRequest(): SearchRequest { +this._searchRequest.query = { query_string: { query: this.generateSelect() } }; +return this._searchRequest; + } + + set searchRequest(value: SearchRequest) { +this._searchRequest = value; +this.query = this._searchRequest.query.query_string.query; --- End diff -- Fixed --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #620: Metron-988: UI for viewing alerts generated by Met...
Github user iraghumitra commented on a diff in the pull request: https://github.com/apache/metron/pull/620#discussion_r126947344 --- Diff: metron-interface/metron-alerts/src/app/alerts/alerts-list/query-builder.ts --- @@ -0,0 +1,139 @@ +/** + * 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 {Filter} from '../../model/filter'; +import {ColumnNamesService} from '../../service/column-names.service'; +import {SearchRequest} from '../../model/search-request'; + +export class QueryBuilder { + private _searchRequest = new SearchRequest(); + private _query = '*'; + private _displayQuery = this._query; + private _filters: Filter[] = []; + + set query(value: string) { +value = value.replace(/\\:/g, ':'); +this._query = value; +this.updateFilters(this._query, false); +this.onSearchChange(); + } + + get query(): string { +return this._query; + } + + set displayQuery(value: string) { +this._displayQuery = value; +this.updateFilters(this._displayQuery, true); +this.onSearchChange(); + } + + get displayQuery(): string { +return this._displayQuery; + } + + get filters(): Filter[] { +return this._filters; + } + + + get searchRequest(): SearchRequest { +this._searchRequest.query = { query_string: { query: this.generateSelect() } }; --- End diff -- Fixed. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron issue #636: METRON-1022: Elasticsearch REST endpoint
Github user cestella commented on the issue: https://github.com/apache/metron/pull/636 @ottobackwards agreed, we should work to that goal. I think we might want to make baby steps, though, and create the abstractions first and move to making it pluggable later. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron issue #636: METRON-1022: Elasticsearch REST endpoint
Github user ottobackwards commented on the issue: https://github.com/apache/metron/pull/636 @cestella https://issues.apache.org/jira/browse/METRON-956 Is the jira I had created on this topic. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron pull request #641: METRON-539: added HASH function for stellar.
Github user jjmeyer0 commented on a diff in the pull request: https://github.com/apache/metron/pull/641#discussion_r126926090 --- Diff: metron-stellar/stellar-common/src/test/java/org/apache/metron/stellar/dsl/functions/HashFunctionsTest.java --- @@ -0,0 +1,169 @@ +/* + * 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. + */ +package org.apache.metron.stellar.dsl.functions; + +import com.google.common.io.BaseEncoding; +import org.apache.commons.lang.SerializationUtils; +import org.junit.Test; + +import java.io.Serializable; +import java.nio.charset.StandardCharsets; +import java.security.MessageDigest; +import java.security.NoSuchAlgorithmException; +import java.security.Security; +import java.util.Arrays; +import java.util.Collections; +import java.util.HashMap; +import java.util.Map; +import java.util.Set; + +import static org.apache.metron.stellar.common.utils.StellarProcessorUtils.run; +import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertNull; + +public class HashFunctionsTest { + final HashFunctions.Hash hash = new HashFunctions.Hash(); + + @Test(expected = IllegalArgumentException.class) + public void nullArgumentListShouldThrowException() throws Exception { +hash.apply(null); + } + + @Test(expected = IllegalArgumentException.class) + public void emptyArgumentListShouldThrowException() throws Exception { +hash.apply(Collections.emptyList()); + } + + @Test(expected = IllegalArgumentException.class) + public void singleArgumentListShouldThrowException() throws Exception { +hash.apply(Collections.singletonList("some value.")); + } + + @Test(expected = IllegalArgumentException.class) + public void argumentListWithMoreThanTwoValuesShouldThrowException3() throws Exception { +hash.apply(Arrays.asList("1", "2", "3")); + } + + @Test(expected = IllegalArgumentException.class) + public void argumentListWithMoreThanTwoValuesShouldThrowException4() throws Exception { +hash.apply(Arrays.asList("1", "2", "3", "4")); + } + + @Test(expected = IllegalArgumentException.class) + public void invalidAlgorithmArgumentShouldThrowException() throws Exception { +hash.apply(Arrays.asList("value to hash", "invalidAlgorithm")); + } + + @Test + public void invalidNullAlgorithmArgumentShouldThrowException() throws Exception { +assertNull(hash.apply(Arrays.asList("value to hash", null))); + } + + @Test + public void nullInputForValueToHashShouldProperlyThrowException() throws Exception { +assertNull(hash.apply(Arrays.asList(null, "md5"))); + } --- End diff -- oh boy, @mattf-horton that makes much more sense. forgive me i had a long day yesterday. I had a moment haha --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron issue #636: METRON-1022: Elasticsearch REST endpoint
Github user cestella commented on the issue: https://github.com/apache/metron/pull/636 I just want to follow-up with something a bit more concrete suggestions. I think the beginnings of an abstraction are there. You pulled out a bunch of utility methods from `ElasticsearchWriter` which is good. I'm looking for a DAO-like abstraction in `metron-elasticsearch` and the `ServiceImpl` in the REST layer uses the generic DAO implementation (which would not live in the REST layer so it's usable for lower level access) and the index impl is specified by at runtime. The impl for the DAO would live in one of `metron-elasticsearch` or `metron-solr`. Again, these are my $0.02 and just suggestions. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---
[GitHub] metron issue #636: METRON-1022: Elasticsearch REST endpoint
Github user cestella commented on the issue: https://github.com/apache/metron/pull/636 Looking at this implementation and working a bit on the PoC for index data mutation, I think the abstraction here isn't in the right place. It's too bound-up in the REST layer whereas data access has traditionally existed further down in the abstraction layer. It's a smell that metron-rest has to depend on ES at all. That dependency should be provided at runtime entirely. Also, we're going to need data access in batch as well and I'd like a sensible abstraction in place to enable that (e.g. merging modified records when reading as part of a Spark or MR job). It seems to me that we need a DAO abstraction living in `metron-writer` or even `metron-indexing` for interacting with indices with the concrete implementations existing in `metron-elasticsearch` and `metron-solr` for ES and Solr respectively. I think `metron-rest` should work entirely on abstract classes. The REST layer should choose the DAO implementation via reflection and we should adjust the classpath in the shell script to include one of `metron-elasticsearch` or `metron-solr`. In essence, I'm recommending something similar to how JDBC works where you choose your driver implementation via reflection (often) and work with base JDBC classes from there. --- If your project is set up for it, you can reply to this email and have your reply appear on GitHub as well. If your project does not have this feature enabled and wishes so, or if the feature is enabled but not working, please contact infrastructure at infrastruct...@apache.org or file a JIRA ticket with INFRA. ---