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

Add IP-addresses and MAC-addresses to event #6878

Merged
merged 20 commits into from
May 4, 2018
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.asciidoc
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@ https://github.com/elastic/beats/compare/v6.0.0-beta2...master[Check the HEAD di
- Added logging of system info at Beat startup. {issue}5946[5946]
- Do not log errors if X-Pack Monitoring is enabled but Elastisearch X-Pack is not. {pull}6627[6627]
- Add rename processor. {pull}6292[6292]
- Add IP-addresses and MAC-addresses to add_host_metadata. {pull}6878[6878]

*Auditbeat*

Expand Down
9 changes: 9 additions & 0 deletions libbeat/processors/add_host_metadata/_meta/fields.yml
Original file line number Diff line number Diff line change
Expand Up @@ -31,3 +31,12 @@
type: keyword
description: >
OS family (e.g. redhat, debian, freebsd, windows).
- name: ip
type: ip
description: >
List of IP-addresses.
Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, of course! I'll fix that too.

- name: mac
type: keyword
description: >
List of hardware-addresses, usually MAC-addresses.
Copy link
Contributor

Choose a reason for hiding this comment

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

I assume the type here would be keyword.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll fix that


64 changes: 61 additions & 3 deletions libbeat/processors/add_host_metadata/add_host_metadata.go
Original file line number Diff line number Diff line change
@@ -1,13 +1,16 @@
package add_host_metadata

import (
"fmt"
"net"
"time"

"github.com/elastic/beats/libbeat/beat"
"github.com/elastic/beats/libbeat/common"
"github.com/elastic/beats/libbeat/processors"
"github.com/elastic/go-sysinfo"
"github.com/elastic/go-sysinfo/types"
"github.com/pkg/errors"
)

func init() {
Expand All @@ -18,19 +21,27 @@ type addHostMetadata struct {
info types.HostInfo
lastUpdate time.Time
data common.MapStr
config Config
}

const (
processorName = "add_host_metadata"
cacheExpiration = time.Minute * 5
)

func newHostMetadataProcessor(_ *common.Config) (processors.Processor, error) {
func newHostMetadataProcessor(cfg *common.Config) (processors.Processor, error) {
config := defaultConfig()
if err := cfg.Unpack(&config); err != nil {
return nil, errors.Wrapf(err, "fail to unpack the %v configuration", processorName)
}

h, err := sysinfo.Host()
if err != nil {
return nil, err
}
p := &addHostMetadata{
info: h.Info(),
info: h.Info(),
config: config,
}
return p, nil
}
Expand Down Expand Up @@ -71,10 +82,57 @@ func (p *addHostMetadata) loadData() {
if p.info.OS.Build != "" {
p.data.Put("host.os.build", p.info.OS.Build)
}

if p.config.NetInfoEnabled {
// IP-address and MAC-address
var ipList, hwList = p.getNetInfo()
p.data.Put("host.ip", ipList)
Copy link
Contributor

Choose a reason for hiding this comment

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

Reviewing the code below and the possibility that ipList and hwList can be empty, we should check for len(x) == 0 and in case of 0 not add the field.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Ok, I'll fix that

p.data.Put("host.mac", hwList)
}

p.lastUpdate = time.Now()
}
}

func (p addHostMetadata) getNetInfo() ([]string, []string) {
var ipList []string
var hwList []string

// Get all interfaces and loop through them
ifaces, err := net.Interfaces()
if err != nil {
return ipList, hwList
Copy link
Contributor

Choose a reason for hiding this comment

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

Instead or returning the empty lists could we return here the error? Meaning it would return nil, nil, err. On line 89 we could then check for the error and would not add the fields in case of an error and could also log the error.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, do you have a code example of logging, that I could look at?

}
for _, i := range ifaces {
// Skip loopback interfaces
if i.Flags&net.FlagLoopback == net.FlagLoopback {
continue
}

hw := i.HardwareAddr.String()
// Skip empty hardware addresses
if hw != "" {
hwList = append(hwList, hw)
}

addrs, err := i.Addrs()
if err != nil {
return ipList, hwList
Copy link
Contributor

Choose a reason for hiding this comment

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

Similar question as above. I wonder if in case of an error we should just log it but continue trying to collect the addresses from the interfaces instead of returning.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It is a matter of taste, I prefer to fail fast. You decide.

}
for _, addr := range addrs {
switch v := addr.(type) {
case *net.IPNet:
ipList = append(ipList, v.IP.String())
case *net.IPAddr:
ipList = append(ipList, v.IP.String())
}
}
}

return ipList, hwList
}

func (p addHostMetadata) String() string {
return "add_host_metadata=[]"
return fmt.Sprintf("%v=[netinfo.enabled=[%v]]",
processorName, p.config.NetInfoEnabled)
}
40 changes: 39 additions & 1 deletion libbeat/processors/add_host_metadata/add_host_metadata_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,10 @@ func TestRun(t *testing.T) {
Fields: common.MapStr{},
Timestamp: time.Now(),
}
p, err := newHostMetadataProcessor(nil)
testConfig, err := common.NewConfigFrom(map[string]interface{}{})
assert.NoError(t, err)

p, err := newHostMetadataProcessor(testConfig)
if runtime.GOOS != "windows" && runtime.GOOS != "darwin" && runtime.GOOS != "linux" {
assert.IsType(t, types.ErrNotImplemented, err)
return
Expand All @@ -31,4 +34,39 @@ func TestRun(t *testing.T) {
v, err := newEvent.GetValue("host.os.family")
assert.NoError(t, err)
assert.NotNil(t, v)

v, err = newEvent.GetValue("host.ip")
assert.Error(t, err)
assert.Nil(t, v)

v, err = newEvent.GetValue("host.mac")
assert.Error(t, err)
assert.Nil(t, v)

event = &beat.Event{
Copy link
Contributor

Choose a reason for hiding this comment

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

Could you split this up into 2 tests? Like this if the tests fail we can already tell based on the name which part is probably not working.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@ruflin Is it really enough to make update in the metricbeat directory? Or should I do it for every beat?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry, my mistake. You have to run it in the top level beats directory. Same for make fmt.

Fields: common.MapStr{},
Timestamp: time.Now(),
}
testConfig, err = common.NewConfigFrom(map[string]interface{}{
"netinfo.enabled": true,
})
assert.NoError(t, err)

p, err = newHostMetadataProcessor(testConfig)
if runtime.GOOS != "windows" && runtime.GOOS != "darwin" && runtime.GOOS != "linux" {
assert.IsType(t, types.ErrNotImplemented, err)
return
}
assert.NoError(t, err)

newEvent, err = p.Run(event)
assert.NoError(t, err)

v, err = newEvent.GetValue("host.ip")
assert.NoError(t, err)
assert.NotNil(t, v)

v, err = newEvent.GetValue("host.mac")
assert.NoError(t, err)
assert.NotNil(t, v)
}
16 changes: 16 additions & 0 deletions libbeat/processors/add_host_metadata/config.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
package add_host_metadata

Choose a reason for hiding this comment

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

don't use an underscore in package name


//import (
Copy link
Contributor

Choose a reason for hiding this comment

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

leftover?

// "github.com/elastic/beats/libbeat/processors"
//)

// Config for add_host_metadata processor.
type Config struct {
NetInfoEnabled bool `config:"netinfo.enabled"` // Add IP and MAC to event
}

func defaultConfig() Config {
return Config{
// netInfoEnabled: "false",
}
}