[GitHub] metron pull request #760: METRON-1188: Ambari global configuration managemen...

[mmiklavc]

Github user mmiklavc commented on a diff in the pull request:

https://github.com/apache/metron/pull/760#discussion_r139550653
  
--- Diff: 
metron-platform/metron-common/src/main/java/org/apache/metron/common/cli/ConfigurationManager.java
 ---
@@ -130,29 +201,39 @@ public void pull(CuratorFramework client, String 
outFileStr, final boolean force
   public void visit(ConfigurationType configurationType, String name, 
String data) {
 File out = getFile(outputDir, configurationType, name);
 if (!out.exists() || force) {
-  if(!out.exists()) {
+  if (!out.exists()) {
 out.getParentFile().mkdirs();
   }
   try {
 Files.write(data, out, Charset.defaultCharset());
   } catch (IOException e) {
 throw new RuntimeException("Sorry, something bad happened 
writing the config to " + out.getAbsolutePath() + ": " + e.getMessage(), e);
   }
-}
-else if(out.exists() && !force) {
+} else if (out.exists() && !force) {
   throw new IllegalStateException("Unable to overwrite existing 
file (" + out.getAbsolutePath() + ") without the force flag (-f or --force) 
being set.");
 }
   }
 });
   }
 
   public void push(String inputDirStr, CuratorFramework client) throws 
Exception {
-  final File inputDir = new File(inputDirStr);
+final File inputDir = new File(inputDirStr);
--- End diff --

Latest commit addresses the input dir null check.


---

[GitHub] metron pull request #760: METRON-1188: Ambari global configuration managemen...

[justinleet]

Github user justinleet commented on a diff in the pull request:

https://github.com/apache/metron/pull/760#discussion_r139547949
  
--- Diff: 
metron-platform/metron-common/src/main/java/org/apache/metron/common/cli/ConfigurationManager.java
 ---
@@ -130,29 +201,39 @@ public void pull(CuratorFramework client, String 
outFileStr, final boolean force
   public void visit(ConfigurationType configurationType, String name, 
String data) {
 File out = getFile(outputDir, configurationType, name);
 if (!out.exists() || force) {
-  if(!out.exists()) {
+  if (!out.exists()) {
 out.getParentFile().mkdirs();
   }
   try {
 Files.write(data, out, Charset.defaultCharset());
   } catch (IOException e) {
 throw new RuntimeException("Sorry, something bad happened 
writing the config to " + out.getAbsolutePath() + ": " + e.getMessage(), e);
   }
-}
-else if(out.exists() && !force) {
+} else if (out.exists() && !force) {
   throw new IllegalStateException("Unable to overwrite existing 
file (" + out.getAbsolutePath() + ") without the force flag (-f or --force) 
being set.");
 }
   }
 });
   }
 
   public void push(String inputDirStr, CuratorFramework client) throws 
Exception {
-  final File inputDir = new File(inputDirStr);
+final File inputDir = new File(inputDirStr);
--- End diff --

Yeah, I think we could definitely handle the scripts better in general.

I'm not opposed to just leaving this as-is, but it seems like a decent / 
small opportunity to make it more usable. Up to you.


---

[GitHub] metron issue #760: METRON-1188: Ambari global configuration management broke...

[mmiklavc]

Github user mmiklavc commented on the issue:

https://github.com/apache/metron/pull/760
  
@justinleet Looks like you found the missing -i option. Also, it's funny 
that I confirmed "pp" and still ended up typing "pf." While I'm in here, what 
do you think is better, pp or pf? I'm happy to change it.


---

[GitHub] metron pull request #760: METRON-1188: Ambari global configuration managemen...

[justinleet]

Github user justinleet commented on a diff in the pull request:

https://github.com/apache/metron/pull/760#discussion_r139526927
  
--- Diff: 
metron-platform/metron-common/src/main/java/org/apache/metron/common/cli/ConfigurationManager.java
 ---
@@ -130,29 +201,39 @@ public void pull(CuratorFramework client, String 
outFileStr, final boolean force
   public void visit(ConfigurationType configurationType, String name, 
String data) {
 File out = getFile(outputDir, configurationType, name);
 if (!out.exists() || force) {
-  if(!out.exists()) {
+  if (!out.exists()) {
 out.getParentFile().mkdirs();
   }
   try {
 Files.write(data, out, Charset.defaultCharset());
   } catch (IOException e) {
 throw new RuntimeException("Sorry, something bad happened 
writing the config to " + out.getAbsolutePath() + ": " + e.getMessage(), e);
   }
-}
-else if(out.exists() && !force) {
+} else if (out.exists() && !force) {
   throw new IllegalStateException("Unable to overwrite existing 
file (" + out.getAbsolutePath() + ") without the force flag (-f or --force) 
being set.");
 }
   }
 });
   }
 
   public void push(String inputDirStr, CuratorFramework client) throws 
Exception {
-  final File inputDir = new File(inputDirStr);
+final File inputDir = new File(inputDirStr);
--- End diff --

The examples in the test plan don't work directly because they NPE here.  I 
assume it's because inputDirStr is null here and we only check for validity 
after wrapping it in a `File`.  While you're in here, could you add a check 
(and meaningful exception) for the case where inputDirStr is null?

Error is:
```
[root@node1 zookeeper]# $METRON_HOME/bin/zk_load_configs.sh -z $ZOOKEEPER 
-m PUSH -c GLOBAL
Exception in thread "main" java.lang.NullPointerException
at java.io.File.(File.java:277)
at 
org.apache.metron.common.cli.ConfigurationManager.push(ConfigurationManager.java:230)
at 
org.apache.metron.common.cli.ConfigurationManager.run(ConfigurationManager.java:256)
at 
org.apache.metron.common.cli.ConfigurationManager.run(ConfigurationManager.java:242)
at 
org.apache.metron.common.cli.ConfigurationManager.main(ConfigurationManager.java:350)
```


---

[GitHub] metron issue #760: METRON-1188: Ambari global configuration management broke...

[mmiklavc]

Github user mmiklavc commented on the issue:

https://github.com/apache/metron/pull/760
  
I also ran this through testing with Kerberos enabled in full dev.


---

[GitHub] metron issue #760: METRON-1188: Ambari global configuration management broke...

[mmiklavc]

Github user mmiklavc commented on the issue:

https://github.com/apache/metron/pull/760
  
### Test Plan
1. Run up full dev and verify you have data flowing through to the indexes
2. Change global config options.
1. Go into the Ambari management UI and modify any of the following 
config options. e.g. set the es_clustername property to "ivebeenchanged". These 
are values that end up in global.json and will prompt for a component restart. 
Restart the components.
- es.clustername
- es.ip
- es.date.format
- parser.error.topic
- update.hbase.table
- update.hbase.cf
- profiler.client.period.duration
- profiler.client.period.duration.units
2. Verify $METRON_HOME/config/zookeeper/global.json now has the new 
value. For the example above, you would expect to see 
es.clustername=ivebeenchanged.
3. Verify Zookeeper contains the new config.
`$METRON_HOME/bin/zk_load_configs.sh -z $ZOOKEEPER -m DUMP -c 
GLOBAL`
3. Test changing configs from the command line. The zk config utils has 
been updated with new backwards compatible features, so we'll test that here.
1. Change one of the zookeeper configs locally and then try a vanilla 
config PUSH using the original method. Verify the values made it into zookeeper.
`${METRON_HOME}/bin/zk_load_configs.sh -i 
${METRON_HOME}/config/zookeeper -m PUSH -z $ZOOKEEPER`
2. Change a config locally and push JUST that config to Zookeeper by 
using one of the new config loader options. You can then perform a DUMP in a 
similar manner.
```
# global config only
$METRON_HOME/bin/zk_load_configs.sh -z $ZOOKEEPER -m PUSH -c GLOBAL
# bro config only
$METRON_HOME/bin/zk_load_configs.sh -z $ZOOKEEPER -m PUSH -c PARSER 
-n bro
# take a dump of config
$METRON_HOME/bin/zk_load_configs.sh -z $ZOOKEEPER -m DUMP -c GLOBAL
$METRON_HOME/bin/zk_load_configs.sh -z $ZOOKEEPER -m DUMP -c PARSER 
-n bro
```
3. Try the new JSON patch mechanism with key/value from the command 
line.  Below are some examples but please feel free to explore other 
combinations. **Note** - commons cli strips the first outermost pair of quotes 
from all arguments, so you have to provide escaped quotes twice. Neither 
\""bar"\" nor "\"bar\"" will work. You must provide both escaped quotes for 
string values. \"\"bar\"\". Complex objects are a bit easier, but you still 
have to escape quotes inside the string to get them to parse properly.
```
# global config only - add a new key "foo" with simple string value 
"bar"
$METRON_HOME/bin/zk_load_configs.sh -z $ZOOKEEPER -m PATCH -c 
GLOBAL -pm ADD -pk foo  -pv \"\"bar\"\"
# global config only - add a new key "foo" to the JSON doc root 
with complex value "{ \"bar\" : { \"baz\" : [ \"bazval1\", \"bazval2\" ] } }"
$METRON_HOME/bin/zk_load_configs.sh -z $ZOOKEEPER -m PATCH -c 
GLOBAL -pm ADD -pk "/foo"  -pv "{ \"bar\" : { \"baz\" : [ \"bazval1\", 
\"bazval2\" ] } }"
# global config only - remove a key altogether
$METRON_HOME/bin/zk_load_configs.sh -z $ZOOKEEPER -m PATCH -c 
GLOBAL -pm REMOVE -pk "/foo"
```
4. Try the new JSON patch mechanism with a patch file.  Below are some 
examples but please feel free to explore other combinations. Note that a patch 
file is a JSON array of individual patches.
1. Try a single patch from file
```
# Create a single patch that adds a new node. Create file 
"/tmp/mypatch.txt" and add the following JSON array.
[
{
"op": "add",
"path": "/you",
"value": { "cannot" : { "handle" : [ "the", "flow" ] } }
}
]

   # Perform the patch
   $METRON_HOME/bin/zk_load_configs.sh -z $ZOOKEEPER -m PATCH -c GLOBAL 
-pf /tmp/mypatch.txt

   # Should result in adding the following to global.json
"you" : {
  "cannot" : {
"handle" : [ "the", "flow" ]
  }
}
   ```

   2. Try multiple patches in a patch file
   ```
   Create a single patch file that adds multiple nodes. Create file 
"/tmp/mygrimespatch.txt" and add the following JSON array.
   # 2 patches in a single patch file
[
{
"op": "add",
"path": "/la",
"value": { "jiggy" : { "jar" : { "jar" : "do" } } }
},
{
"op": "add",
"path": "/carl",
"value": [ "poppa", "rhymes" ]
}
]

# Perform the patch

[GitHub] metron pull request #765: METRON-1194 Add Profiler Debug Functions to Profil...

[nickwallen]

GitHub user nickwallen opened a pull request:

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

METRON-1194 Add Profiler Debug Functions to Profiler README

This is a doc-only change that introduces new users to the Profiler using 
the Profiler Debug functions.  This is a nice way to get up and running with 
the Profiler.

## 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 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/nickwallen/metron METRON-1194

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

https://github.com/apache/metron/pull/765.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 #765


commit d4379cc58956be8b8af43989655a15b655b23741
Author: Nick Allen 
Date:   2017-09-18T17:47:58Z

METRON-1194 Add Profiler Debug Functions to Profiler README




---

[GitHub] metron issue #760: METRON-1188: Ambari global configuration management broke...

[nickwallen]

Github user nickwallen commented on the issue:

https://github.com/apache/metron/pull/760
  
@mmiklavc I will spin this up and get cozy with it. 

> A friendlier, detailed testing plan for this is coming very soon. 

Have you been able to get a test plan together?  That would help me 
understand what I should expect from the PR.

As you know tackling configuration is a big thing and will take a few 
iterations.  We know this is good step in the right direction, but  I want to 
make sure that I understand exactly what this PR does and (more importantly) 
what it does not do.




---

Re: Committing to the metron-bro-plugin-kafka repo

[Nick Allen]

Nice!  Looks good to me.






On Mon, Sep 18, 2017 at 11:35 AM zeo...@gmail.com  wrote:

> Okay, I took a stab at it this morning, can I get a double check before
> pushing it out?  The latest commit would be opened as a PR.
>
> https://github.com/JonZeolla/metron-bro-plugin-kafka/tree/dev
>
> Jon
>
> On Fri, Sep 15, 2017 at 12:54 PM zeo...@gmail.com 
> wrote:
>
> > Good point, I can take that task re migrating the revision history of the
> > folder.
> >
> > Jon
> >
> > On Fri, Sep 15, 2017, 12:14 Nick Allen  wrote:
> >
> >> Hi Jon -
> >>
> >> I agree with you on the approach.  We should first copy everything as it
> >> is
> >> to the new repo.  We should maintain the revision history too.  I'm sure
> >> there is a way to do it, but would have to research a bit.  Then we
> apply
> >> your changes on top of that.
> >>
> >> Thanks
> >>
> >> On Thu, Sep 14, 2017 at 1:36 AM, zeo...@gmail.com 
> >> wrote:
> >>
> >> > So, I've been working on METRON-813
> >> >  lately and I have
> an
> >> > initial run at it ready to go here
> >> >  (squashed
> >> history,
> >> > see a better history there
> >> >  >> >).
> >> > Since the metron-bro-plugin-kafka repo is empty, I can't open a PR
> >> against
> >> > it on GitHub for review.  Does anybody have a suggestion regarding how
> >> to
> >> > move forward?  I see two options:
> >> > 1. I make the initial commit a direct copy of the bro-plugin-kafka
> >> folder
> >> >  >> > sensors/bro-plugin-kafka>
> >> > (I believe this would require a new JIRA for a direct copy), and then
> >> open
> >> > a PR for the METRON-813 changes to get reviewed via the normal
> process.
> >> > 2. I make the initial commit the result of METRON-813, but review
> occurs
> >> > via the mailing list and using my fork.
> >> >
> >> > I prefer 1, but wanted to put it up for discussion.  Once we decide on
> >> the
> >> > correct approach then I would be happy to put together a testing plan
> >> for
> >> > the PR as well.
> >> >
> >> > Just to clarify, the general roadmap for getting this used in
> >> apache/metron
> >> > is:
> >> > 1.  Create a bro package in apache/metron-bro-plugin-kafka
> >> > 2.  Update the ansible bro setup
> >> >  >> > deployment/roles/bro/tasks>
> >> > to install/configure bro-pkg (`pip install bro-pkg && bro-pkg
> >> autoconfig`)
> >> > and use it to install the apache/metron-bro-plugin-kafka package.
> >> >
> >> > I will also be adding this to the official bro package manager
> >> > , but out of an abundance of
> caution I
> >> > plan to setup ansible to pull the package directly from the
> >> > apache/metron-bro-plugin-kafka using bro-pkg instead of going through
> >> the
> >> > bro/packages package source (which removes the bro/packages
> dependency).
> >> >
> >> > Feedback on all of the above is welcome.
> >> >
> >> > Jon
> >> > --
> >> >
> >> > Jon
> >> >
> >>
> > --
> >
> > Jon
> >
> --
>
> Jon
>

RE: [DISCUSS] metron-config build failure on Centos 7

[Ian Abreu]

Sorry for bringing this back from the dead, but the problem was that NPM was 
failing silently due to me running the build as root.

Cheers!
-Original Message-
From: Nick Allen [mailto:n...@nickallen.org] 
Sent: Wednesday, September 6, 2017 5:32 PM
To: dev@metron.apache.org
Subject: Re: [DISCUSS] metron-config build failure on Centos 7

Actually, ignore my previous advice.  That was completely wrong.

I think you are running an old version of Node.  Per our docs, you need 6.9+ 
[1].  It appears you are running 6.2.

[1]
https://github.com/apache/metron/tree/master/metron-interface/metron-config#prerequisites



On Wed, Sep 6, 2017 at 5:29 PM Nick Allen  wrote:

> What version of Metron are you running?
>
> This error seems a bit different, but do you have the patch for this 
> issue that was fixed a while back?
> https://github.com/apache/metron/pull/691
> https://issues.apache.org/jira/browse/METRON-1104
>
>
> On Wed, Sep 6, 2017 at 3:41 PM Ian Abreu  wrote:
>
>> Hello all,
>>
>> After spending a few days on it now, I'm stuck. I can't figure out 
>> how to build metron-config on Centos 7.
>>
>> I've gone down the rabbit hole so far as to believe that it's a 
>> problem with npm and dependencies for whatever reason not installing 
>> properly, but I'm posting here in the hopes that someone else knows what I'm 
>> doing wrong.
>>
>> I've posted most of my progress here:
>> https://issues.apache.org/jira/browse/METRON-914
>>
>> But here are the highlights..
>>
>> Trying to build on centos 7, after adhering to the advice here:
>> https://issues.apache.org/jira/browse/METRON-1112
>> I attempt to build and receive the following errors...
>>
>>
>> [ERROR] npm ERR! Linux 3.10.0-514.26.2.el7.x86_64 [ERROR] npm ERR! 
>> argv "/root/metron/metron-interface/metron-config/node/node"
>> "/root/metron/metron-interface/metron-config/node/node_modules/npm/bin/npm-cli.js"
>> "install"
>> [ERROR] npm ERR! node v6.2.0
>> [ERROR] npm ERR! npm  v3.8.9
>> [ERROR] npm ERR! code ELIFECYCLE
>> [ERROR]
>> [ERROR] npm ERR! execSync@1.0.2 install: `node install.js` [ERROR] 
>> npm ERR! Exit status 1 [ERROR] npm ERR!
>> [ERROR] npm ERR! Failed at the execSync@1.0.2 install script 'node 
>> install.js'.
>> [ERROR] npm ERR! Make sure you have the latest version of node.js and 
>> npm installed.
>> [ERROR] npm ERR! If you do, this is most likely a problem with the 
>> execSync package, [ERROR] npm ERR! not with npm itself.
>> [ERROR] npm ERR! Tell the author that this fails on your system:
>> [ERROR] npm ERR! node install.js
>> [ERROR] npm ERR! You can get information on how to open an issue for 
>> this project with:
>> [ERROR] npm ERR! npm bugs execSync
>> [ERROR] npm ERR! Or if that isn't available, you can get their info via:
>> [ERROR] npm ERR! npm owner ls execSync
>> [ERROR] npm ERR! There is likely additional logging output above.
>>
>> Which appears to be an NPM problem. I've gone ahead and tried to 
>> troubleshoot by setting my $node_path to the local modules dir, and a 
>> few other things, but the only thing that seemed to change any sort 
>> of state was when I setup an inotify on a module's folder as hinted 
>> at by one of my logs.
>>
>> Error: Cannot find module
>> '/root/metron/metron-interface/metron-config/node_modules/codecov/node_modules/execSync/install.js'
>>
>> So I setup an inotify trigger, and pulled in that module as soon as 
>> the parent directory was created, and I received the following:
>>
>> Error: Cannot find module
>> '/root/metron/metron-interface/metron-config/node_modules/codecov/node_modules/execSync/install.js'
>>
>> Notice the path change, after copying the same module to that path, 
>> it changed to yet another module:
>>
>> [ERROR] Error: Cannot find module
>> '/root/metron/metron-interface/metron-config/node_modules/angular2-template-loader/node_modules/execSync/install.js'
>>
>> I'm beginning to think that there's something wrong with the locally 
>> instantiated npm binary on centos 7 for the build process.
>>
>> It should be noted that I'm building using 'mvn clean package -Dskiptests'
>>
>> Anyone else got any thoughts on the matter?
>>
>> Cheers,
>> Ian
>>
>