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 process monitor enhancements #7135

Merged
merged 1 commit into from
May 26, 2018

Conversation

adriansr
Copy link
Contributor

This patch extends the functionality of the processes monitor in packetbeat:

  • report the full command-line for all processes (not only those configured as monitored).
  • Add support for Windows process monitoring.
  • Disable the monitor when using file input.
  • Get rid of polling for processes and sockets.

Closes #541

Updated functionality

The original monitor will only track those processes that match a pattern specified in the configuration:

packetbeat.procs:
  enabled: true
  monitored:
    - process: mysqld
      cmdline_grep: mysqld

    - process: pgsql
      cmdline_grep: postgres

    - process: nginx
      cmdline_grep: nginx

    - process: app
      cmdline_grep: gunicorn

Will result in the following fields being added to generated events:

"proc": "mysqld" when the process is detected to be on the server-side of a stream, and
"client_proc": "mysqld" when the process is detected on the client-side.

While maintaining this behavior, this patch will add:

"cmdline": "/usr/bin/mysqld --bind-address=0.0.0.0 --data-dir=/var/lib/mysql"

and the same for client_cmdline when the process is on the client-side of a connection.

The cmdline and client_cmdline fields will be added to events generated by any running process. The proc and client_proc fields will be added only to those processes in the monitored section, so the original behavior is maintained.

Windows support

Added support for Windows using IP Helper functions (see issue #541).

Gosigar dependency

The original feature was tailored to Linux. To make it multiplatform, gosigar is used to get process information. This also means that it is no longer required to run packetbeat as root to access the command-line from processes belonging to other users.

Get rid of processes/sockets polling

The original implementation would refresh the process list with a specified frequency (refreshPidsFreq). The mapping of TCP ports to processes would also be updated after a specified interval (maxReadFreq). This caused the feature to frequently miss process information for an event, specially with short-lived processes and connections.

In this implementation, such delays are removed. The list of processes and active sockets is updated as necessary. While this is a somewhat expensive operation, ranging from ~10ms under Linux and ~100ms in Windows, it is only performed when an event is being populated, and the source or destination of this traffic is a local port for which we don't have any information yet.

As the processes monitor is an opt-in feature, disabled by default, I find it more important to aim for more reliable behavior in the expense of some delays.

Missing features

Some limitations of the original implementation have not been addressed by this PR:

  • Support UDP sockets.
  • Handle processes bound to a subset of the local network addresses, and by extension two or more processes using the same port on different local addresses.
  • Fetch Linux sockets using elastic/gosigar/sys/linux/inetdiag (faster than the current proc-based access).
  • Move Windows socket querying to gosigar/sys/windows and use it in metricbeat's system/socket module.
  • Support macOS.

@adriansr adriansr requested a review from andrewkroh May 18, 2018 11:26
@adriansr adriansr added Packetbeat discuss Issue needs further discussion. labels May 18, 2018
procGetExtendedTcpTable = modiphlpapi.NewProc("GetExtendedTcpTable")
)

func _GetExtendedTcpTable(pTcpTable uintptr, pdwSize *uint32, bOrder bool, ulAf uint32, tableClass uint32, reserved uint32) (code syscall.Errno, err error) {

This comment was marked as resolved.

var (
modiphlpapi = windows.NewLazySystemDLL("iphlpapi.dll")

procGetExtendedTcpTable = modiphlpapi.NewProc("GetExtendedTcpTable")

This comment was marked as outdated.

)

var (
errERROR_IO_PENDING error = syscall.Errno(errnoERROR_IO_PENDING)

This comment was marked as resolved.

// Do the interface allocations only once for common
// Errno values.
const (
errnoERROR_IO_PENDING = 997

This comment was marked as resolved.

localScopeId uint32
localPort uint32
remoteAddr [16]byte
remoteScopeId uint32

Choose a reason for hiding this comment

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

struct field remoteScopeId should be remoteScopeID

e(uint32FieldToPort(row.localPort), int(row.owningPID))
}

func (_ TCPRowOwnerPIDExtractor) Size() int {

Choose a reason for hiding this comment

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

exported method TCPRowOwnerPIDExtractor.Size should have comment or be unexported
receiver name should not be an underscore, omit the name if it is unused

return TCP6RowOwnerPIDExtractor(fn)
}

func (e TCPRowOwnerPIDExtractor) Extract(ptr unsafe.Pointer) {

Choose a reason for hiding this comment

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

exported method TCPRowOwnerPIDExtractor.Extract should have comment or be unexported

{windows.AF_INET6, _GetExtendedTcpTable, TCP_TABLE_OWNER_PID_ALL, extractTCP6RowOwnerPID},
}

func (proc *ProcessesWatcher) GetLocalPortToPIDMapping() (ports map[uint16]int, err error) {

Choose a reason for hiding this comment

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

exported method ProcessesWatcher.GetLocalPortToPIDMapping should have comment or be unexported

type extractorFactory func(fn callbackFn) extractor

type TCPRowOwnerPIDExtractor callbackFn
type TCP6RowOwnerPIDExtractor callbackFn

Choose a reason for hiding this comment

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

exported type TCP6RowOwnerPIDExtractor should have comment or be unexported

type callbackFn func(uint16, int)
type extractorFactory func(fn callbackFn) extractor

type TCPRowOwnerPIDExtractor callbackFn

Choose a reason for hiding this comment

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

exported type TCPRowOwnerPIDExtractor should have comment or be unexported

@adriansr adriansr force-pushed the packetbeat/feature/procs branch 3 times, most recently from 070f64b to 8a519bf Compare May 22, 2018 03:54

package procs

func (proc *ProcessesWatcher) GetLocalPortToPIDMapping() (ports map[uint16]int, err error) {

Choose a reason for hiding this comment

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

exported method ProcessesWatcher.GetLocalPortToPIDMapping should have comment or be unexported

inode uint64
}

func (proc *ProcessesWatcher) GetLocalPortToPIDMapping() (ports map[uint16]int, err error) {

Choose a reason for hiding this comment

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

exported method ProcessesWatcher.GetLocalPortToPIDMapping should have comment or be unexported


return false
func (proc *ProcessesWatcher) GetLocalIPs() ([]net.IP, error) {

Choose a reason for hiding this comment

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

exported method ProcessesWatcher.GetLocalIPs should have comment or be unexported

if ip.Equal(addr) {
return true
}
func (proc *ProcessesWatcher) GetProcessCommandLine(pid int) (cmdLine string) {

Choose a reason for hiding this comment

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

exported method ProcessesWatcher.GetProcessCommandLine should have comment or be unexported

SrcIP, DstIP net.IP
SrcPort, DstPort uint16
}

type IPPortTuple struct {

This comment was marked as resolved.

@@ -15,10 +15,15 @@ const MaxIPPortTupleRawSize = 16 + 16 + 2 + 2

type HashableIPPortTuple [MaxIPPortTupleRawSize]byte

type IPPortTuple struct {
IPLength int
type BaseTuple struct {

This comment was marked as resolved.


logp.Debug("procsdetailed", "UpdateMappingEntry(): port=%d pid=%d", port, p.name)
logp.Debug("procsdetailed", "UpdateMappingEntry(): port=%d pid=%d process='%s' name=%s",
Copy link
Member

Choose a reason for hiding this comment

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

Looks like that should “updateMappingEntry()”.

@ruflin
Copy link
Member

ruflin commented May 22, 2018

I wonder if we should make this an autodiscovery provider instead so we can reuse it in all Beats?

Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

LGTM. The PR description is very helpful. I'm looking forward to the future enhancements too.

class uint32
extractor extractorFactory
}{
{windows.AF_INET, _GetExtendedTcpTable, TCP_TABLE_OWNER_PID_ALL, extractTCPRowOwnerPID},
Copy link
Member

Choose a reason for hiding this comment

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

These TCP table structures look fun to work with [read with sarcastic tone].

This patch extends the functionality of the processes monitor in
packetbeat:

- report the full command-line for all processes (not only those
  configured as `monitored`.
- Add support for Windows process monitoring.
- Disable the monitor when using file input.
- Get rid of refresh delays.

Closes elastic#541
@adriansr
Copy link
Contributor Author

Updated CHANGELOG

@andrewkroh
Copy link
Member

jenkins, test it

@andrewkroh andrewkroh merged commit cf19da7 into elastic:master May 26, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss Issue needs further discussion. enhancement Packetbeat
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants