METRON-858 bro-plugin-kafka is throwing segfaults #547
Conversation
| $pred(rec: Conn::Info) = { return ! (( |rec$id$orig_h| == 128 || |rec$id$resp_h| == 128 )); }, | ||
| $config = table(["stream_id"] = fmt("%s", Conn::LOG)) | ||
| ]); | ||
| } |
There was a problem hiding this comment.
With this script, I would expect to find a total of 6 log filters having been created. The first 3 created by Bro/Kafka/logs-to-kafka.bro and then the last 3 created by your bro_init() function. To avoid this, I think what you want to do something more like this...
@load Bro/Kafka/logs-to-kafka.bro
redef Kafka::topic_name = "";
redef Kafka::tag_json = T;
event bro_init() &priority=-5
{
# handles HTTP
Log::add_filter(HTTP::LOG, [
$name = "kafka-http",
$writer = Log::WRITER_KAFKAWRITER,
$pred(rec: HTTP::Info) = { return ! (( |rec$id$orig_h| == 128 || |rec$id$resp_h| == 128 )); },
$config = table(
["stream_id"] = fmt("%s", HTTP::LOG),
["metadata.broker.list"] = "localhost:9092"
)
]);
# handles DNS
Log::add_filter(DNS::LOG, [
$name = "kafka-dns",
$writer = Log::WRITER_KAFKAWRITER,
$pred(rec: DNS::Info) = { return ! (( |rec$id$orig_h| == 128 || |rec$id$resp_h| == 128 )); },
$config = table(
["stream_id"] = fmt("%s", DNS::LOG),
["metadata.broker.list"] = "localhost:9092"
)
]);
}
| 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. | ||
| * By providing a set of logs via `logs_to_send`. |
There was a problem hiding this comment.
This doesn't sound like a complete thought to me. Maybe this?
"Defining logs_to_send will ensure that only HTTP and DNS records are sent."
| As documented in [METRON-285](https://issues.apache.org/jira/browse/METRON-285) and [METRON-286](https://issues.apache.org/jira/browse/METRON-286), various components in Metron do not currently support IPv6. Because of this, you may not want to send bro logs that contain IPv6 source or destination IPs into Metron. In this example, we are assuming a somewhat standard bro configuration for sending logs into a Metron cluster, such that: | ||
| * Each type of bro log is sent to the `bro` topic, but is tagged with the appropriate log type (such as `http`, `dns`, or `conn`). This is done by setting `topic_name` to `bro`, setting `$path` to an empty string (or leaving it unset), and by setting `tag_json` to true. | ||
| * The Kafka writer is set appropriately to send logs to the `bro` Kafka topic being used in your Metron cluster. This requires that your `kafka_conf` and `$config` tables are appropriately configured. | ||
|
|
There was a problem hiding this comment.
The effect of this paragraph is saying, "this is like example 1, but excludes IPv6", right?
There was a problem hiding this comment.
Yes. Do you think it's too wordy?
There was a problem hiding this comment.
In my humble opinion yes, but it is subjective. If you like what you have, we can keep it. I think its a great example to add.
There was a problem hiding this comment.
My goal was just to be explicit. I can take another stab at it tomorrow.
There was a problem hiding this comment.
I took a stab at being more concise. Take a look and let me know, I think I like it more this way than before so thanks for the critique.
| case "$1" in | ||
| --with-librdkafka=*) | ||
| append_cache_entry LibRdKafka_ROOT_DIR PATH $optarg | ||
| append_cache_entry LibRDKafka_ROOT_DIR PATH $optarg |
|
|
||
| ``` | ||
| @load Bro/Kafka/logs-to-kafka.bro | ||
| redef Kafka::topic_name = "bro"; |
There was a problem hiding this comment.
We have to set topic_name to empty string otherwise logs-to-kafka.bro will create its own filters.
redef Kafka::topic_name = "";
There was a problem hiding this comment.
Not sure I follow. If Kafka::logs_to_send is empty why would logs-to-kafka.bro make its own filters?
There was a problem hiding this comment.
Yes, exactly. It needs to be an empty string. I am seeing Example 3 in your README setting it to 'bro'. It needs to be set to empty string.
There was a problem hiding this comment.
I'm obviously missing something. logs_to_send is not topic_name? In my example logs_to_send is not set. I also note below that "logs_to_send is mutually exclusive with $pred, thus you must individually add a filter per log that you would like to send into Metron if you want to configure a predicate."
There was a problem hiding this comment.
Sorry, Jon. You are 100% right. I am the one confused. I had misunderstood what you are showing with the example. Ignore me. This looks good.
|
@JonZeolla Let me know when you have your "Outstanding Items" complete. Once you're happy, I'll run it through some testing. It is looking real good and seems ready to go. |
|
Will do, thanks. |
|
@nickwallen This should be ready to test now. Sorry about the delay |
|
ok, I tried this and was flumoxed by nonexistent directories or files:
Are the instructions up to date? |
|
Just ran through the instructions from scratch and it works for me, can you give it another shot from the beginning? |
|
+1 - although I can't do the stress testing part. Steps followed correctly work perfectly. |
|
+1 Works like a charm. Tested basic functioning on a multi-node cluster against 1 gbps of canned traffic. Thanks for the contribution @JonZeolla ! |
3 participants
Contributor Comments
This PR is a follow-on of #545.
The primary change here resolves a thread safety issue that is only seen when under load. It has been reported in numerous places, but I've seen it best documented here.
Testing
The following steps can be used to validate the PR.
HDP.repoYum repository; this will allow us to install Kafka./usr/share/bro/site/local.bro:tap0to listen on.bro.tap0. Keep this running in a separate session.brotopic in Kafka.Pull Request Checklist
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:
For code changes:
[N/A] Have you included steps to reproduce the behavior or problem that is being changed or addressed? (See Contributor Comments)
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 incubating-metron folder via:
[N/A] Have you written or updated unit tests and or integration tests to verify your changes?
[N/A] If adding new dependencies to the code, are these dependencies licensed in a way that is compatible for inclusion under ASF 2.0?
Have you verified the basic functionality of the build by building and running locally with Vagrant full-dev environment or the equivalent?
For documentation related changes:
Have you ensured that format looks appropriate for the output in which it is rendered by building and verifying the site-book? If not then run the following commands and the verify changes via
site-book/target/site/index.html:Note:
Please ensure that once the PR is submitted, you check travis-ci for build issues and submit an update to your PR as soon as possible.
It is also recommended that travis-ci is set up for your personal repository such that your branches are built there before submitting a pull request.