Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Packetbeat protocol analyzer configuration enhancements #3518

Merged
merged 1 commit into from
Feb 22, 2017

Conversation

urso
Copy link

@urso urso commented Feb 2, 2017

  • Optionally configure protocol analyzers using dictionary and/or list
  • Add 'fields', 'fields_under_root' and 'tags' settings to every
    protocol analyzer

This change allows for configuring packetbeat protocols in 2 different styles... both styles can be used at the same time.

1.) dictionary style:

packetbeat.protocols.http:
  ...
packetbeat.protocols.dns:
  ...

2.) array style:

packetbeat.protocols:
- type: http
  ...
- type: dns
  ...

Examples (1) and (2) are equivalent. But array style allows to configure a protocol analyzer multiple times: e.g.

(3) array style with multiple instances of http protocol analyzer:

packetbeat.protocols:
- type: http
  ports: [80]
  fields.service: nginx
- type: http
  ports: [9200]
  fields.service: elasticsearch
  1. mixed style:
packetbeat.protocols.http:
  ...

packetbeat.protocols:
- type: dns
  ...

Limitations:

a) due to limitations in yaml parser, only capturing the last 'name' in a dictionary the key name packetbeat.protocols must not be used multiple times. e.g. this will result in an incompletely processed config (only DNS will be configured):

packetbeat.protocols:
  http:
    ...

packetbeat.protocols:
- type: dns

b) Reusing port numbers (overlapping) might result in one module not seeing any packets (this is already the case if any 2 protocols shall listen on same port number). e.g.:

@urso urso added discuss Issue needs further discussion. in progress Pull request is currently in progress. labels Feb 2, 2017
@urso urso force-pushed the enh/packetbeat-modules-config branch from 17ff13f to ae6c95f Compare February 2, 2017 14:00
@ruflin
Copy link
Contributor

ruflin commented Feb 3, 2017

  • Can you provide an example on how to config could look afterwards?
  • As usual, -1 on fields_under_root ;-)

@urso
Copy link
Author

urso commented Feb 16, 2017

As usual, -1 on fields_under_root ;-)

I just reused already available functionality...

@ruflin
Copy link
Contributor

ruflin commented Feb 17, 2017

@urso I suggest we change our current default config to the array style and deprecate the old style. This will mean we have the same structure across all beats.

@urso urso force-pushed the enh/packetbeat-modules-config branch from ae6c95f to b76b503 Compare February 21, 2017 15:43
@urso urso added docs review and removed discuss Issue needs further discussion. in progress Pull request is currently in progress. labels Feb 21, 2017
Copy link
Contributor

@dedemorton dedemorton left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

small comment...otherwise docs LGTM.

@@ -92,6 +92,8 @@ https://github.com/elastic/beats/compare/v5.1.1...master[Check the HEAD diff]
- The HAProxy module is now GA, instead of experimental. {pull}3525[3525]

*Packetbeat*
- Add `fields` and `fields_under_root` to packetbeat protocols configurations. {pull}3518[3518]
- Add list style packetbeat protocols configurations. This change support different configurations of same protocol analyzers to be active. {pull]3518[3518]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would change this to say, "Add list style Packetbeat protocol configurations. This change supports different configurations of the same protocol analyzer to be active." The second sentence is a bit confusing even with the edit. How about this instead? "This change supports specifying multiple configurations of the same protocol analyzer." [saying "to be active" makes the sentence harder to parse and isn't IMO necessary.]

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

- Optionally configure protocol analyzers using dictionary and/or list
- Add 'fields', 'fields_under_root' and 'tags' settings to every
  protocol analyzer
- update sample config file to use list style configuration
- add deprecated warning if dictionary style configuration is used
- update docs

This change allows for configuring packetbeat protocols in 2 different
styles... both styles can be used at the same time,.

1.) (deprecated) dictionary style:

```
packetbeat.protocols.http:
  ...
packetbeat.protocols.dns:
  ...
```

2.) array style:

```
packetbeat.protocols:
- type: http
  ...
- type: dns
  ...
```

Examples (1) and (2) are equivalent. But array style allows to configure a
protocol analyzer multiple times: e.g.

(3) array style with multiple instances of http protocol analyzer:

```
packetbeat.protocols:
- type: http
  ports: [80]
  fields.service: nginx
- type: http
  ports: [9200]
  fields.service: elasticsearch
```

4) mixed style:

```
packetbeat.protocols.http:
  ...

packetbeat.protocols:
- type: dns
  ...
```

Limitations:

a) due to limitations in yaml parser, only capturing the last 'name' in a
dictionary the key name `packetbeat.protocols` must not be used multiple times.
e.g. this will result in an incompletely processed config (only DNS will be
configured):

```
packetbeat.protocols:
  http:
    ...

packetbeat.protocols:
- type: dns
```

b) Reusing port numbers (overlapping) might result in one module not seeing any
packets (this is already the case if any 2 protocols shall listen on same port
number).
@urso urso force-pushed the enh/packetbeat-modules-config branch from d78e421 to 7dfa8f0 Compare February 22, 2017 00:00
@ruflin ruflin merged commit bae62de into elastic:master Feb 22, 2017
@ruflin
Copy link
Contributor

ruflin commented Feb 22, 2017

One step closer to have beats configs unified 🎉

About defining a port multiple times. This is kind of similar to defining a file multiple times in filebeat, just easier to detect. If there would be a registry of ports, we could return an error in case this happens.

Having protocols as list will also make it easier to implement config reloading.

@monicasarbu monicasarbu changed the title Packetbeat protocol analyzer enhancements Packetbeat protocol analyzer configuration enhancements Feb 27, 2017
@urso urso deleted the enh/packetbeat-modules-config branch February 19, 2019 18:46
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants