You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@metron.apache.org by JonZeolla <gi...@git.apache.org> on 2017/11/07 13:17:54 UTC
[GitHub] metron-bro-plugin-kafka pull request #2: DO NOT MERGE METRON-1304: Allow met...
GitHub user JonZeolla opened a pull request:
https://github.com/apache/metron-bro-plugin-kafka/pull/2
DO NOT MERGE METRON-1304: Allow metron-bro-plugin-kafka to include or exclude logs
This PR assumes [METRON-1303](https://issues.apache.org/jira/browse/METRON-1303).
This allows us the option to specify the inclusion and exclusion of logs to send to kafka. If you were to unset `Kafka::logs_to_send` and specify logs in `Kafka::logs_to_exclude`, all currently active logs, except for the ones you've excluded, should be sent to kafka.
You can merge this pull request into a Git repository by running:
$ git pull https://github.com/JonZeolla/metron-bro-plugin-kafka METRON-1304
Alternatively you can review and apply these changes as the patch at:
https://github.com/apache/metron-bro-plugin-kafka/pull/2.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 #2
----
commit f21e51f4f91452d66b644b1c041e9a3ae3b39bd7
Author: Jon Zeolla <ze...@gmail.com>
Date: 2017-11-07T12:12:53Z
METRON-1303: Reorganize the metron-bro-plugin-kafka
commit c2f8b2c347f647076c1d0ba17dad5b3794d7957d
Author: Jon Zeolla <ze...@gmail.com>
Date: 2017-11-07T12:22:39Z
Fix broken link
commit 3dcecec4faf2119174123ea8a934a83e469080e2
Author: Jon Zeolla <ze...@gmail.com>
Date: 2017-11-07T13:12:37Z
METRON-1304: Allow metron-bro-plugin-kafka to include or exclude logs
----
---
[GitHub] metron-bro-plugin-kafka pull request #2: DO NOT MERGE METRON-1304: Allow met...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:
https://github.com/apache/metron-bro-plugin-kafka/pull/2#discussion_r151793309
--- Diff: scripts/Bro/Kafka/logs-to-kafka.bro ---
@@ -14,32 +14,37 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
-##! load this script to enable log output to kafka
+
+##! Load this script to enable log output to kafka
module Kafka;
export {
+ ## Specify which :bro:type:`Log::ID` to exclude from being sent to kafka.
##
- ## which log streams should be sent to kafka?
- ## example:
- ## redef Kafka::logs_to_send = set(Conn::Log, HTTP::LOG, DNS::LOG);
+ ## Example: redef Kafka::logs_to_exclude = set(SSH::LOG);
+ const logs_to_exclude: set[Log::ID] &redef;
+
+ ## Specify which :bro:type:`Log::ID` to send to kafka.
##
+ ## Example: redef Kafka::logs_to_send = set(Conn::Log, DNS::LOG);
const logs_to_send: set[Log::ID] &redef;
}
event bro_init() &priority=-5
{
for (stream_id in Log::active_streams)
{
- if (stream_id in Kafka::logs_to_send)
- {
- local filter: Log::Filter = [
- $name = fmt("kafka-%s", stream_id),
- $writer = Log::WRITER_KAFKAWRITER,
- $config = table(["stream_id"] = fmt("%s", stream_id))
- ];
+ if ( stream_id in Kafka::logs_to_exclude ||
+ (|Kafka::logs_to_send| > 0 && stream_id !in Kafka::logs_to_send) )
--- End diff --
If a user configures nothing, neither defines `logs_to_send` nor `logs_to_exclude`, then we expect ALL logs to go to Kafka?
---
[GitHub] metron-bro-plugin-kafka pull request #2: DO NOT MERGE METRON-1304: Allow met...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:
https://github.com/apache/metron-bro-plugin-kafka/pull/2#discussion_r152848070
--- Diff: scripts/Bro/Kafka/logs-to-kafka.bro ---
@@ -14,32 +14,37 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
-##! load this script to enable log output to kafka
+
+##! Load this script to enable log output to kafka
module Kafka;
export {
+ ## Specify which :bro:type:`Log::ID` to exclude from being sent to kafka.
##
- ## which log streams should be sent to kafka?
- ## example:
- ## redef Kafka::logs_to_send = set(Conn::Log, HTTP::LOG, DNS::LOG);
+ ## Example: redef Kafka::logs_to_exclude = set(SSH::LOG);
+ const logs_to_exclude: set[Log::ID] &redef;
+
+ ## Specify which :bro:type:`Log::ID` to send to kafka.
##
+ ## Example: redef Kafka::logs_to_send = set(Conn::Log, DNS::LOG);
const logs_to_send: set[Log::ID] &redef;
}
event bro_init() &priority=-5
{
for (stream_id in Log::active_streams)
{
- if (stream_id in Kafka::logs_to_send)
- {
- local filter: Log::Filter = [
- $name = fmt("kafka-%s", stream_id),
- $writer = Log::WRITER_KAFKAWRITER,
- $config = table(["stream_id"] = fmt("%s", stream_id))
- ];
+ if ( stream_id in Kafka::logs_to_exclude ||
+ (|Kafka::logs_to_send| > 0 && stream_id !in Kafka::logs_to_send) )
--- End diff --
I too would rather an unset `logs_to_send` to send nothing. I'd rather not 'surprise' the user with more logs. Especially with how difficult it can be to truly 'see' the variety of logs that are actually landing in a topic.
---
[GitHub] metron-bro-plugin-kafka pull request #2: METRON-1304: Allow metron-bro-plugi...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on a diff in the pull request:
https://github.com/apache/metron-bro-plugin-kafka/pull/2#discussion_r232405844
--- Diff: README.md ---
@@ -11,6 +11,32 @@ This software is a part of the [Apache Metron](http://metron.apache.org/) projec
## Installation
+### `bro-pkg` Installation
+
--- End diff --
As a part of the review process, this improvement was split into #17
---
[GitHub] metron-bro-plugin-kafka pull request #2: METRON-1304: Allow metron-bro-plugi...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on a diff in the pull request:
https://github.com/apache/metron-bro-plugin-kafka/pull/2#discussion_r232405772
--- Diff: README.md ---
@@ -42,22 +68,47 @@ This software is a part of the [Apache Metron](http://metron.apache.org/) projec
The following examples highlight different ways that the plugin can be used. Simply add the Bro script language to your `local.bro` file (for example, `/usr/share/bro/site/local.bro`) as shown to demonstrate the example.
-### Example 1
+### Example 1 - Send a list of logs to kafka
The goal in this example is to send all HTTP and DNS records to a Kafka topic named `bro`.
* Any configuration value accepted by librdkafka can be added to the `kafka_conf` configuration table.
- * By defining `topic_name` all records will be sent to the same Kafka topic.
- * Defining `logs_to_send` will ensure that only HTTP and DNS records are sent.
+ * The `topic_name` will default to send all records to a single Kafka topic called 'bro'.
+ * Defining `logs_to_send` will send the HTTP and DNS records to the brokers specified in your `Kafka::kafka_conf`.
```
@load packages/metron-bro-plugin-kafka/Apache/Kafka
redef Kafka::logs_to_send = set(HTTP::LOG, DNS::LOG);
--- End diff --
As a part of the review process, this improvement was split into #17
---
[GitHub] metron-bro-plugin-kafka issue #2: DO NOT MERGE METRON-1304: Allow metron-bro...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on the issue:
https://github.com/apache/metron-bro-plugin-kafka/pull/2
Yup, I'm typing that up as we speak.
---
[GitHub] metron-bro-plugin-kafka pull request #2: METRON-1304: Allow metron-bro-plugi...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on a diff in the pull request:
https://github.com/apache/metron-bro-plugin-kafka/pull/2#discussion_r225018294
--- Diff: README.md ---
@@ -42,22 +68,47 @@ This software is a part of the [Apache Metron](http://metron.apache.org/) projec
The following examples highlight different ways that the plugin can be used. Simply add the Bro script language to your `local.bro` file (for example, `/usr/share/bro/site/local.bro`) as shown to demonstrate the example.
-### Example 1
+### Example 1 - Send a list of logs to kafka
The goal in this example is to send all HTTP and DNS records to a Kafka topic named `bro`.
* Any configuration value accepted by librdkafka can be added to the `kafka_conf` configuration table.
- * By defining `topic_name` all records will be sent to the same Kafka topic.
- * Defining `logs_to_send` will ensure that only HTTP and DNS records are sent.
+ * The `topic_name` will default to send all records to a single Kafka topic called 'bro'.
+ * Defining `logs_to_send` will send the HTTP and DNS records to the brokers specified in your `Kafka::kafka_conf`.
```
@load packages/metron-bro-plugin-kafka/Apache/Kafka
redef Kafka::logs_to_send = set(HTTP::LOG, DNS::LOG);
--- End diff --
Sure, I added multiple brokers to example 1.
---
[GitHub] metron-bro-plugin-kafka pull request #2: METRON-1304: Allow metron-bro-plugi...
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on a diff in the pull request:
https://github.com/apache/metron-bro-plugin-kafka/pull/2#discussion_r224997103
--- Diff: README.md ---
@@ -144,23 +194,35 @@ event bro_init() &priority=-5
#### Notes
* `logs_to_send` is mutually exclusive with `$pred`, thus for each log you want to set `$pred` on, you must individually setup a `Log::add_filter` and refrain from including that log in `logs_to_send`.
+ * In Bro 2.5.x the bro project introduced a [logger function](https://www.bro.org/sphinx/cluster/index.html#logger) which removes the logging functions from the manager thread, and taking advantage of that is highly recommended. If you are running this plugin on Bro 2.4.x, you may encounter issues where the manager thread is taking on too much responsibility and pinning a single CPU core without the ability to spread the load across additional cores. In this case, it may be in your best interest to prefer using a bro logging predicate over filtering in your Metron cluster [using Stellar](https://github.com/apache/metron/tree/master/metron-stellar/stellar-common) in order to lesson the load of that thread.
--- End diff --
s/lesson/lessen/
---
[GitHub] metron-bro-plugin-kafka pull request #2: METRON-1304: Allow metron-bro-plugi...
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on a diff in the pull request:
https://github.com/apache/metron-bro-plugin-kafka/pull/2#discussion_r224996952
--- Diff: README.md ---
@@ -11,6 +11,32 @@ This software is a part of the [Apache Metron](http://metron.apache.org/) projec
## Installation
+### `bro-pkg` Installation
+
--- End diff --
What is the case for having more than one way to install? When would you not want to use bro-pkg?
---
[GitHub] metron-bro-plugin-kafka issue #2: METRON-1304: Allow metron-bro-plugin-kafka...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/metron-bro-plugin-kafka/pull/2
I am getting a test failure.
```
[root@localhost ~]# bro-pkg install http://github.com/jonzeolla/metron-bro-plugin-kafka --version=METRON-1304
The following packages will be INSTALLED:
http://github.com/jonzeolla/metron-bro-plugin-kafka (METRON-1304)
Verify the following REQUIRED external dependencies:
(Ensure their installation on all relevant systems before proceeding):
from http://github.com/jonzeolla/metron-bro-plugin-kafka (METRON-1304):
librdkafka ~0.9.4
Proceed? [Y/n] Y
http://github.com/jonzeolla/metron-bro-plugin-kafka asks for LIBRDKAFKA_ROOT (Path to librdkafka installation tree) ? [/usr/local/lib]
Saved answers to config file: /root/.bro-pkg/config
Running unit tests for "http://github.com/jonzeolla/metron-bro-plugin-kafka"
[ 90%] kafka.show-plugin ... failed
% 'btest-diff output' failed unexpectedly (exit code 1)
% cat .diag
== File ===============================
Apache::Kafka - Writes logs to Kafka (dynamic, version 0.3)
[Writer] KafkaWriter (Log::WRITER_KAFKAWRITER)
[Constant] Kafka::kafka_conf
[Constant] Kafka::topic_name
[Constant] Kafka::max_wait_on_shutdown
[Constant] Kafka::tag_json
[Constant] Kafka::json_timestamps
[Constant] Kafka::debug
== Diff ===============================
--- /tmp/test-diff.22832.output.baseline.tmp 2018-11-09 22:51:27.714922560 +0000
+++ /tmp/test-diff.22832.output.tmp 2018-11-09 22:51:27.719922348 +0000
@@ -4,5 +4,6 @@
[Constant] Kafka::topic_name
[Constant] Kafka::max_wait_on_shutdown
[Constant] Kafka::tag_json
+ [Constant] Kafka::json_timestamps
[Constant] Kafka::debug
=======================================
% cat .stderr
1 of 10 tests failed
error: http://github.com/jonzeolla/metron-bro-plugin-kafka tests failed, inspect contents of /root/.bro-pkg/testing/metron-bro-plugin-kafka for details
Proceed to install anyway? [N/y]
```
---
[GitHub] metron-bro-plugin-kafka issue #2: METRON-1304: Allow metron-bro-plugin-kafka...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on the issue:
https://github.com/apache/metron-bro-plugin-kafka/pull/2
@ottobackwards That last commit should have addressed all of your feedback.
---
[GitHub] metron-bro-plugin-kafka issue #2: METRON-1304: Allow metron-bro-plugin-kafka...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on the issue:
https://github.com/apache/metron-bro-plugin-kafka/pull/2
Suggested order of review: #16 -> #2 -> #17 -> #13
---
[GitHub] metron-bro-plugin-kafka pull request #2: METRON-1304: Allow metron-bro-plugi...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on a diff in the pull request:
https://github.com/apache/metron-bro-plugin-kafka/pull/2#discussion_r232405800
--- Diff: README.md ---
@@ -144,23 +194,35 @@ event bro_init() &priority=-5
#### Notes
* `logs_to_send` is mutually exclusive with `$pred`, thus for each log you want to set `$pred` on, you must individually setup a `Log::add_filter` and refrain from including that log in `logs_to_send`.
+ * In Bro 2.5.x the bro project introduced a [logger function](https://www.bro.org/sphinx/cluster/index.html#logger) which removes the logging functions from the manager thread, and taking advantage of that is highly recommended. If you are running this plugin on Bro 2.4.x, you may encounter issues where the manager thread is taking on too much responsibility and pinning a single CPU core without the ability to spread the load across additional cores. In this case, it may be in your best interest to prefer using a bro logging predicate over filtering in your Metron cluster [using Stellar](https://github.com/apache/metron/tree/master/metron-stellar/stellar-common) in order to lesson the load of that thread.
--- End diff --
As a part of the review process, this improvement was split into #17
---
[GitHub] metron-bro-plugin-kafka pull request #2: DO NOT MERGE METRON-1304: Allow met...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:
https://github.com/apache/metron-bro-plugin-kafka/pull/2#discussion_r151791732
--- Diff: scripts/Bro/Kafka/logs-to-kafka.bro ---
@@ -14,32 +14,37 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
-##! load this script to enable log output to kafka
+
+##! Load this script to enable log output to kafka
module Kafka;
export {
+ ## Specify which :bro:type:`Log::ID` to exclude from being sent to kafka.
##
- ## which log streams should be sent to kafka?
- ## example:
- ## redef Kafka::logs_to_send = set(Conn::Log, HTTP::LOG, DNS::LOG);
+ ## Example: redef Kafka::logs_to_exclude = set(SSH::LOG);
+ const logs_to_exclude: set[Log::ID] &redef;
+
+ ## Specify which :bro:type:`Log::ID` to send to kafka.
##
+ ## Example: redef Kafka::logs_to_send = set(Conn::Log, DNS::LOG);
const logs_to_send: set[Log::ID] &redef;
}
event bro_init() &priority=-5
{
for (stream_id in Log::active_streams)
{
- if (stream_id in Kafka::logs_to_send)
- {
- local filter: Log::Filter = [
- $name = fmt("kafka-%s", stream_id),
- $writer = Log::WRITER_KAFKAWRITER,
- $config = table(["stream_id"] = fmt("%s", stream_id))
- ];
+ if ( stream_id in Kafka::logs_to_exclude ||
+ (|Kafka::logs_to_send| > 0 && stream_id !in Kafka::logs_to_send) )
--- End diff --
Why do we have to check that `logs_to_send` > 0 ? Is this necessary before doing a 'contains' (`in`)?
If it is necessary then we should do the same for `logs_to_exclude`. If it is NOT necessary, then let's just get rid of it to simplify the logic.
---
[GitHub] metron-bro-plugin-kafka issue #2: METRON-1304: Allow metron-bro-plugin-kafka...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/metron-bro-plugin-kafka/pull/2
@JonZeolla Yes, I would definitely appreciate if you could break this into separate, focused PRs.
This PR itself has been open since Nov 2017 and it seems like it has been accumulating changes. Would be better to just open a PR when you're ready for a review.
You might consider closing some of the other ones that you have open (#12, #8, ?), since those don't seem to be ready for review. It makes it difficult to track what needs reviewed if we have PRs just hanging out there.
---
[GitHub] metron-bro-plugin-kafka pull request #2: METRON-1304: Allow metron-bro-plugi...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on a diff in the pull request:
https://github.com/apache/metron-bro-plugin-kafka/pull/2#discussion_r225629223
--- Diff: README.md ---
@@ -11,6 +11,32 @@ This software is a part of the [Apache Metron](http://metron.apache.org/) projec
## Installation
+### `bro-pkg` Installation
+
+`bro-pkg` is the preferred mechanism for installing this plugin, as it will dynamically retrieve, build, test, and load the plugin. Note, that you will still need to [activate](#activation) and configure the plugin after your installation.
+
--- End diff --
The libs don't need to be installed anywhere in particular, I could put them in my home directory.
---
[GitHub] metron-bro-plugin-kafka pull request #2: METRON-1304: Allow metron-bro-plugi...
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on a diff in the pull request:
https://github.com/apache/metron-bro-plugin-kafka/pull/2#discussion_r225631931
--- Diff: README.md ---
@@ -11,6 +11,32 @@ This software is a part of the [Apache Metron](http://metron.apache.org/) projec
## Installation
+### `bro-pkg` Installation
+
+`bro-pkg` is the preferred mechanism for installing this plugin, as it will dynamically retrieve, build, test, and load the plugin. Note, that you will still need to [activate](#activation) and configure the plugin after your installation.
+
--- End diff --
ok
---
[GitHub] metron-bro-plugin-kafka issue #2: METRON-1304: Allow metron-bro-plugin-kafka...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on the issue:
https://github.com/apache/metron-bro-plugin-kafka/pull/2
Note that this one now depends on apache/metron-bro-plugin-kafka#16 to work properly because I split out the btest bugfix.
---
[GitHub] metron-bro-plugin-kafka issue #2: METRON-1304: Allow metron-bro-plugin-kafka...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on the issue:
https://github.com/apache/metron-bro-plugin-kafka/pull/2
Yes that's why I said it depends on #16 and mentioned the order of review should be #16 -> #2 -> #17 -> #13
---
[GitHub] metron-bro-plugin-kafka pull request #2: METRON-1304: Allow metron-bro-plugi...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on a diff in the pull request:
https://github.com/apache/metron-bro-plugin-kafka/pull/2#discussion_r225018235
--- Diff: README.md ---
@@ -11,6 +11,32 @@ This software is a part of the [Apache Metron](http://metron.apache.org/) projec
## Installation
+### `bro-pkg` Installation
+
--- End diff --
Added some context to the README.
---
[GitHub] metron-bro-plugin-kafka issue #2: METRON-1304: Allow metron-bro-plugin-kafka...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on the issue:
https://github.com/apache/metron-bro-plugin-kafka/pull/2
https://github.com/apache/metron-bro-plugin-kafka/pull/2#issuecomment-429502420 is still accurate for testing. I'll edit the initial comment to add it.
---
[GitHub] metron-bro-plugin-kafka issue #2: METRON-1304: Allow metron-bro-plugin-kafka...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/metron-bro-plugin-kafka/pull/2
+1 Looks good @JonZeolla Thanks
---
[GitHub] metron-bro-plugin-kafka pull request #2: METRON-1304: Allow metron-bro-plugi...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on a diff in the pull request:
https://github.com/apache/metron-bro-plugin-kafka/pull/2#discussion_r232405751
--- Diff: README.md ---
@@ -11,6 +11,32 @@ This software is a part of the [Apache Metron](http://metron.apache.org/) projec
## Installation
+### `bro-pkg` Installation
+
+`bro-pkg` is the preferred mechanism for installing this plugin, as it will dynamically retrieve, build, test, and load the plugin. Note, that you will still need to [activate](#activation) and configure the plugin after your installation.
+
--- End diff --
As a part of the review process, this improvement was split into #17
---
[GitHub] metron-bro-plugin-kafka issue #2: METRON-1304: Allow metron-bro-plugin-kafka...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on the issue:
https://github.com/apache/metron-bro-plugin-kafka/pull/2
This is ready for review. I refactored and added tests for the new include/exclude functionality and also found out that the unit tests are broken for `0.2`, which this fixes.
---
[GitHub] metron-bro-plugin-kafka pull request #2: DO NOT MERGE METRON-1304: Allow met...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on a diff in the pull request:
https://github.com/apache/metron-bro-plugin-kafka/pull/2#discussion_r151794206
--- Diff: scripts/init.bro ---
@@ -18,11 +18,20 @@
module Kafka;
export {
- const topic_name: string = "bro" &redef;
- const max_wait_on_shutdown: count = 3000 &redef;
- const tag_json: bool = F &redef;
- const kafka_conf: table[string] of string = table(
- ["metadata.broker.list"] = "localhost:9092"
- ) &redef;
- const debug: string = "" &redef;
+ ## Destination kafka topic name
+ const topic_name: string = "bro" &redef;
+
+ ## Maximum wait on shutdown in milliseconds
+ const max_wait_on_shutdown: count = 3000 &redef;
+
+ ## Boolean to JSON with a log stream identifier
--- End diff --
Thanks for adding all the comments. But this one doesn't read so well. Or maybe I just don't read so well. :) Read this one over once more. If it still makes sense to you, then I'm good with it.
---
[GitHub] metron-bro-plugin-kafka issue #2: DO NOT MERGE METRON-1304: Allow metron-bro...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/metron-bro-plugin-kafka/pull/2
My bad, you did. You're totally on top of it. Ignore me. :)
---
[GitHub] metron-bro-plugin-kafka issue #2: METRON-1304: Allow metron-bro-plugin-kafka...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on the issue:
https://github.com/apache/metron-bro-plugin-kafka/pull/2
Okay, pushed that fix and merged in master.
---
[GitHub] metron-bro-plugin-kafka pull request #2: METRON-1304: Allow metron-bro-plugi...
Posted by asfgit <gi...@git.apache.org>.
Github user asfgit closed the pull request at:
https://github.com/apache/metron-bro-plugin-kafka/pull/2
---
[GitHub] metron-bro-plugin-kafka issue #2: METRON-1304: Allow metron-bro-plugin-kafka...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/metron-bro-plugin-kafka/pull/2
@JonZeolla I just want to confirm, all of this is related only to include/exclude functionality? There seems to be more here, but maybe I am misunderstanding on first glance.
---
[GitHub] metron-bro-plugin-kafka issue #2: METRON-1304: Allow metron-bro-plugin-kafka...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on the issue:
https://github.com/apache/metron-bro-plugin-kafka/pull/2
If you're looking to test this by spinning up full-dev in apache/metron without skipping the sensors tag (a non-default setting), you would need to merge in apache/metron#1238. There are other ways to test this without doing that, but I assume that's a typical way people may want to look at this.
---
[GitHub] metron-bro-plugin-kafka issue #2: METRON-1304: Allow metron-bro-plugin-kafka...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/metron-bro-plugin-kafka/pull/2
@JonZeolla Can you add the latest testing instructions in the description? This seems to have evolved over time and am not sure what is needed at this point.
---
[GitHub] metron-bro-plugin-kafka pull request #2: METRON-1304: Allow metron-bro-plugi...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on a diff in the pull request:
https://github.com/apache/metron-bro-plugin-kafka/pull/2#discussion_r225017094
--- Diff: README.md ---
@@ -11,6 +11,32 @@ This software is a part of the [Apache Metron](http://metron.apache.org/) projec
## Installation
+### `bro-pkg` Installation
+
+`bro-pkg` is the preferred mechanism for installing this plugin, as it will dynamically retrieve, build, test, and load the plugin. Note, that you will still need to [activate](#activation) and configure the plugin after your installation.
+
--- End diff --
There is a suggestion via a [user var](https://bro-package-manager.readthedocs.io/en/stable/package.html#user-vars-field) defined [here](https://github.com/apache/metron-bro-plugin-kafka/blob/d1a09b6a50f20e5fa6cf5c758eea8b0d39ce65be/bro-pkg.meta#L16) but it can be overridden during install. Whatever you provide gets passed into the [build_command](https://github.com/apache/metron-bro-plugin-kafka/blob/d1a09b6a50f20e5fa6cf5c758eea8b0d39ce65be/bro-pkg.meta#L5).
---
[GitHub] metron-bro-plugin-kafka pull request #2: METRON-1304: Allow metron-bro-plugi...
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on a diff in the pull request:
https://github.com/apache/metron-bro-plugin-kafka/pull/2#discussion_r224996917
--- Diff: README.md ---
@@ -11,6 +11,32 @@ This software is a part of the [Apache Metron](http://metron.apache.org/) projec
## Installation
+### `bro-pkg` Installation
+
+`bro-pkg` is the preferred mechanism for installing this plugin, as it will dynamically retrieve, build, test, and load the plugin. Note, that you will still need to [activate](#activation) and configure the plugin after your installation.
+
--- End diff --
Do these libs have to be installed in any certain place? Relative to the Bro Install?
---
[GitHub] metron-bro-plugin-kafka pull request #2: DO NOT MERGE METRON-1304: Allow met...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on a diff in the pull request:
https://github.com/apache/metron-bro-plugin-kafka/pull/2#discussion_r152085762
--- Diff: scripts/Bro/Kafka/logs-to-kafka.bro ---
@@ -14,32 +14,37 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
-##! load this script to enable log output to kafka
+
+##! Load this script to enable log output to kafka
module Kafka;
export {
+ ## Specify which :bro:type:`Log::ID` to exclude from being sent to kafka.
##
- ## which log streams should be sent to kafka?
- ## example:
- ## redef Kafka::logs_to_send = set(Conn::Log, HTTP::LOG, DNS::LOG);
+ ## Example: redef Kafka::logs_to_exclude = set(SSH::LOG);
+ const logs_to_exclude: set[Log::ID] &redef;
+
+ ## Specify which :bro:type:`Log::ID` to send to kafka.
##
+ ## Example: redef Kafka::logs_to_send = set(Conn::Log, DNS::LOG);
const logs_to_send: set[Log::ID] &redef;
}
event bro_init() &priority=-5
{
for (stream_id in Log::active_streams)
{
- if (stream_id in Kafka::logs_to_send)
- {
- local filter: Log::Filter = [
- $name = fmt("kafka-%s", stream_id),
- $writer = Log::WRITER_KAFKAWRITER,
- $config = table(["stream_id"] = fmt("%s", stream_id))
- ];
+ if ( stream_id in Kafka::logs_to_exclude ||
+ (|Kafka::logs_to_send| > 0 && stream_id !in Kafka::logs_to_send) )
--- End diff --
Actually, wait, sorry. If `|Kafka::logs_to_send| > 0` is removed, this doesn't send when `logs_to_send` is unset. Re-adding this.
---
[GitHub] metron-bro-plugin-kafka issue #2: METRON-1304: Allow metron-bro-plugin-kafka...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on the issue:
https://github.com/apache/metron-bro-plugin-kafka/pull/2
Good to go on this one @nickwallen
---
[GitHub] metron-bro-plugin-kafka pull request #2: METRON-1304: Allow metron-bro-plugi...
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on a diff in the pull request:
https://github.com/apache/metron-bro-plugin-kafka/pull/2#discussion_r224996990
--- Diff: README.md ---
@@ -42,22 +68,47 @@ This software is a part of the [Apache Metron](http://metron.apache.org/) projec
The following examples highlight different ways that the plugin can be used. Simply add the Bro script language to your `local.bro` file (for example, `/usr/share/bro/site/local.bro`) as shown to demonstrate the example.
-### Example 1
+### Example 1 - Send a list of logs to kafka
The goal in this example is to send all HTTP and DNS records to a Kafka topic named `bro`.
* Any configuration value accepted by librdkafka can be added to the `kafka_conf` configuration table.
- * By defining `topic_name` all records will be sent to the same Kafka topic.
- * Defining `logs_to_send` will ensure that only HTTP and DNS records are sent.
+ * The `topic_name` will default to send all records to a single Kafka topic called 'bro'.
+ * Defining `logs_to_send` will send the HTTP and DNS records to the brokers specified in your `Kafka::kafka_conf`.
```
@load packages/metron-bro-plugin-kafka/Apache/Kafka
redef Kafka::logs_to_send = set(HTTP::LOG, DNS::LOG);
--- End diff --
The example should be for multiple brokers, not just one since that will be the most common case won't it?
---
[GitHub] metron-bro-plugin-kafka issue #2: DO NOT MERGE METRON-1304: Allow metron-bro...
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on the issue:
https://github.com/apache/metron-bro-plugin-kafka/pull/2
logs_to_exclude should be in the readme, and not just the bro file
---
[GitHub] metron-bro-plugin-kafka issue #2: METRON-1304: Allow metron-bro-plugin-kafka...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on the issue:
https://github.com/apache/metron-bro-plugin-kafka/pull/2
In looking through the PR, this is mostly related to the include/exclude functionality yes, but you are right that it may look like there are some other things included because:
* As a part of updating the documentation for these new features I attempted to make things more uniform in format (which included some text reordering), and I added details regarding what a successful `bro-pkg` install looks like (before this PR that was broken/undocumented)
* This also fixes the previously broken btest which was a 1 line change to `tests/Baseline/kafka.show-plugin/output`
* There are some minor documentation changes such as changing suggested priority to -10, or adding multiple brokers to the docs in response to a [review comment](https://github.com/apache/metron-bro-plugin-kafka/pull/2/files#r224996990).
---
[GitHub] metron-bro-plugin-kafka pull request #2: METRON-1304: Allow metron-bro-plugi...
Posted by ottobackwards <gi...@git.apache.org>.
Github user ottobackwards commented on a diff in the pull request:
https://github.com/apache/metron-bro-plugin-kafka/pull/2#discussion_r225028659
--- Diff: README.md ---
@@ -11,6 +11,32 @@ This software is a part of the [Apache Metron](http://metron.apache.org/) projec
## Installation
+### `bro-pkg` Installation
+
+`bro-pkg` is the preferred mechanism for installing this plugin, as it will dynamically retrieve, build, test, and load the plugin. Note, that you will still need to [activate](#activation) and configure the plugin after your installation.
+
--- End diff --
I mean do we have to document that the libs have to be installed to where bro looks for libs? Like /usr/local vs /opt
---
[GitHub] metron-bro-plugin-kafka issue #2: DO NOT MERGE METRON-1304: Allow metron-bro...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on the issue:
https://github.com/apache/metron-bro-plugin-kafka/pull/2
Yeah, I did send an initial email out [last night](https://lists.apache.org/thread.html/621cbeca432fef0170836e07036e309d943068c5d6a544c1ef2136f9@%3Cdev.metron.apache.org%3E), and in a previous email in that thread I outlined a suggested roadmap, but I will bring it up and clarify things so we can discuss on the list instead of here. Thanks
---
[GitHub] metron-bro-plugin-kafka issue #2: DO NOT MERGE METRON-1304: Allow metron-bro...
Posted by nickwallen <gi...@git.apache.org>.
Github user nickwallen commented on the issue:
https://github.com/apache/metron-bro-plugin-kafka/pull/2
@JonZeolla Can you send an update to the Dev list on where we are at in migrating the Bro plugin? I had not even realized that you had added the code to this repo.
We also still have the Bro plugin code in the main Metron repository. Don't we have some clean-up to do before we start merging new changes?
---
[GitHub] metron-bro-plugin-kafka pull request #2: DO NOT MERGE METRON-1304: Allow met...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on a diff in the pull request:
https://github.com/apache/metron-bro-plugin-kafka/pull/2#discussion_r152674806
--- Diff: scripts/Bro/Kafka/logs-to-kafka.bro ---
@@ -14,32 +14,37 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
-##! load this script to enable log output to kafka
+
+##! Load this script to enable log output to kafka
module Kafka;
export {
+ ## Specify which :bro:type:`Log::ID` to exclude from being sent to kafka.
##
- ## which log streams should be sent to kafka?
- ## example:
- ## redef Kafka::logs_to_send = set(Conn::Log, HTTP::LOG, DNS::LOG);
+ ## Example: redef Kafka::logs_to_exclude = set(SSH::LOG);
+ const logs_to_exclude: set[Log::ID] &redef;
+
+ ## Specify which :bro:type:`Log::ID` to send to kafka.
##
+ ## Example: redef Kafka::logs_to_send = set(Conn::Log, DNS::LOG);
const logs_to_send: set[Log::ID] &redef;
}
event bro_init() &priority=-5
{
for (stream_id in Log::active_streams)
{
- if (stream_id in Kafka::logs_to_send)
- {
- local filter: Log::Filter = [
- $name = fmt("kafka-%s", stream_id),
- $writer = Log::WRITER_KAFKAWRITER,
- $config = table(["stream_id"] = fmt("%s", stream_id))
- ];
+ if ( stream_id in Kafka::logs_to_exclude ||
+ (|Kafka::logs_to_send| > 0 && stream_id !in Kafka::logs_to_send) )
--- End diff --
I was talking to a few people in the bro community about this and I'm hearing that people mostly prefer an unset send_logs to send nothing. What are your thoughts on that? I would prefer to send all by default, but it's not a huge deal to go either way for me.
---
[GitHub] metron-bro-plugin-kafka issue #2: METRON-1304: Allow metron-bro-plugin-kafka...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on the issue:
https://github.com/apache/metron-bro-plugin-kafka/pull/2
An easy way to test this is to use bro-pkg to install the plugin from my branch, or you can go on a properly configured bro box to the `tests` directory and run `btest -d`.
---
[GitHub] metron-bro-plugin-kafka pull request #2: DO NOT MERGE METRON-1304: Allow met...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on a diff in the pull request:
https://github.com/apache/metron-bro-plugin-kafka/pull/2#discussion_r151825744
--- Diff: scripts/init.bro ---
@@ -18,11 +18,20 @@
module Kafka;
export {
- const topic_name: string = "bro" &redef;
- const max_wait_on_shutdown: count = 3000 &redef;
- const tag_json: bool = F &redef;
- const kafka_conf: table[string] of string = table(
- ["metadata.broker.list"] = "localhost:9092"
- ) &redef;
- const debug: string = "" &redef;
+ ## Destination kafka topic name
+ const topic_name: string = "bro" &redef;
+
+ ## Maximum wait on shutdown in milliseconds
+ const max_wait_on_shutdown: count = 3000 &redef;
+
+ ## Boolean to JSON with a log stream identifier
--- End diff --
Lol. I will update, thanks
---
[GitHub] metron-bro-plugin-kafka issue #2: METRON-1304: Allow metron-bro-plugin-kafka...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on the issue:
https://github.com/apache/metron-bro-plugin-kafka/pull/2
Based on the recent release discussions I'd like to get this in. @nickwallen do you have any additional feedback?
---
[GitHub] metron-bro-plugin-kafka pull request #2: DO NOT MERGE METRON-1304: Allow met...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on a diff in the pull request:
https://github.com/apache/metron-bro-plugin-kafka/pull/2#discussion_r152850661
--- Diff: scripts/Bro/Kafka/logs-to-kafka.bro ---
@@ -14,32 +14,37 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
-##! load this script to enable log output to kafka
+
+##! Load this script to enable log output to kafka
module Kafka;
export {
+ ## Specify which :bro:type:`Log::ID` to exclude from being sent to kafka.
##
- ## which log streams should be sent to kafka?
- ## example:
- ## redef Kafka::logs_to_send = set(Conn::Log, HTTP::LOG, DNS::LOG);
+ ## Example: redef Kafka::logs_to_exclude = set(SSH::LOG);
+ const logs_to_exclude: set[Log::ID] &redef;
+
+ ## Specify which :bro:type:`Log::ID` to send to kafka.
##
+ ## Example: redef Kafka::logs_to_send = set(Conn::Log, DNS::LOG);
const logs_to_send: set[Log::ID] &redef;
}
event bro_init() &priority=-5
{
for (stream_id in Log::active_streams)
{
- if (stream_id in Kafka::logs_to_send)
- {
- local filter: Log::Filter = [
- $name = fmt("kafka-%s", stream_id),
- $writer = Log::WRITER_KAFKAWRITER,
- $config = table(["stream_id"] = fmt("%s", stream_id))
- ];
+ if ( stream_id in Kafka::logs_to_exclude ||
+ (|Kafka::logs_to_send| > 0 && stream_id !in Kafka::logs_to_send) )
--- End diff --
Ok I'm convinced, I guess my posture on this is more aggressive than most. I will adjust
---
[GitHub] metron-bro-plugin-kafka pull request #2: DO NOT MERGE METRON-1304: Allow met...
Posted by JonZeolla <gi...@git.apache.org>.
Github user JonZeolla commented on a diff in the pull request:
https://github.com/apache/metron-bro-plugin-kafka/pull/2#discussion_r152075056
--- Diff: scripts/Bro/Kafka/logs-to-kafka.bro ---
@@ -14,32 +14,37 @@
# See the License for the specific language governing permissions and
# limitations under the License.
#
-##! load this script to enable log output to kafka
+
+##! Load this script to enable log output to kafka
module Kafka;
export {
+ ## Specify which :bro:type:`Log::ID` to exclude from being sent to kafka.
##
- ## which log streams should be sent to kafka?
- ## example:
- ## redef Kafka::logs_to_send = set(Conn::Log, HTTP::LOG, DNS::LOG);
+ ## Example: redef Kafka::logs_to_exclude = set(SSH::LOG);
+ const logs_to_exclude: set[Log::ID] &redef;
+
+ ## Specify which :bro:type:`Log::ID` to send to kafka.
##
+ ## Example: redef Kafka::logs_to_send = set(Conn::Log, DNS::LOG);
const logs_to_send: set[Log::ID] &redef;
}
event bro_init() &priority=-5
{
for (stream_id in Log::active_streams)
{
- if (stream_id in Kafka::logs_to_send)
- {
- local filter: Log::Filter = [
- $name = fmt("kafka-%s", stream_id),
- $writer = Log::WRITER_KAFKAWRITER,
- $config = table(["stream_id"] = fmt("%s", stream_id))
- ];
+ if ( stream_id in Kafka::logs_to_exclude ||
+ (|Kafka::logs_to_send| > 0 && stream_id !in Kafka::logs_to_send) )
--- End diff --
Yeah, that's valid, I have removed the check and simplify.
Yeah, I would prefer a default 'send everything' policy when someone loads the package, as long as it's otherwise configured. That said, it will require a bit of Metron testing to make sure that it can handle that. We don't currently handle some of the less interesting logs that are on by default, like packet filter or loaded scripts.
---