Skip to content

Commit

Permalink
Change add_cloud_metadata to not overwrite cloud field (#11612)
Browse files Browse the repository at this point in the history 
* Change add_cloud_metadata to not overwrite cloud field

* Add overwrite flag for add_cloud_metadata

* Add test for cloud.provider: ec2

* Update processors-using.asciidoc
  • Loading branch information
@kaiyan-sheng
kaiyan-sheng authored Apr 9, 2019
1 parent 14aa522 commit 17edc90
Show file tree
Hide file tree
Showing 4 changed files with 203 additions and 24 deletions.
3 changes: 3 additions & 0 deletions CHANGELOG.next.asciidoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,9 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d

- Add _bucket to histogram metrics in Prometheus Collector {pull}11578[11578]
- Prevent the docker/memory metricset from processing invalid events before container start {pull}11676[11676]
- Stop overwriting cloud.* fields from add_cloud_metadata if they are not empty. {pull}11612[11612] {issue}11305[11305]
- Change `add_cloud_metadata` processor to not overwrite `cloud` field when it's already exist in the event. {pull}11612[11612] {issue}11305[11305]
- Change `add_cloud_metadata` processor to not overwrite `cloud` field when it already exist in the event. {pull}11612[11612] {issue}11305[11305]

*Packetbeat*

Expand Down
12 changes: 8 additions & 4 deletions libbeat/docs/processors-using.asciidoc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -511,15 +511,19 @@ processors:
- add_cloud_metadata: ~
-------------------------------------------------------------------------------

The `add_cloud_metadata` processor has one optional configuration setting named
`timeout` that specifies the maximum amount of time to wait for a successful
response when detecting the hosting provider. The default timeout value is
`3s`.
The `add_cloud_metadata` processor has two optional configuration settings.
The first one is `timeout` which specifies the maximum amount of time to wait
for a successful response when detecting the hosting provider. The default
timeout value is `3s`.

If a timeout occurs then no instance metadata will be added to the events. This
makes it possible to enable this processor for all your deployments (in the
cloud or on-premise).

The second optional configuration setting is `overwrite`. When `overwrite` is
`true`, `add_cloud_metadata` overwrites existing `cloud.*` fields (`false` by
default).

The metadata that is added to events varies by hosting provider. Below are
examples for each of the supported providers.

Expand Down
29 changes: 21 additions & 8 deletions libbeat/processors/add_cloud_metadata/add_cloud_metadata.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,9 @@ const (

// Default config
defaultTimeOut = 3 * time.Second

// Default overwrite
defaultOverwrite = false
)

var debugf = logp.MakeDebug("filters")
Expand Down Expand Up @@ -302,9 +305,11 @@ func setupFetchers(c *common.Config) ([]*metadataFetcher, error) {
// New constructs a new add_cloud_metadata processor.
func New(c *common.Config) (processors.Processor, error) {
config := struct {
Timeout time.Duration `config:"timeout"` // Amount of time to wait for responses from the metadata services.
Timeout time.Duration `config:"timeout"` // Amount of time to wait for responses from the metadata services.
Overwrite bool `config:"overwrite"` // Overwrite if cloud.* fields already exist.
}{
Timeout: defaultTimeOut,
Timeout: defaultTimeOut,
Overwrite: defaultOverwrite,
}
err := c.Unpack(&config)
if err != nil {
Expand All @@ -317,16 +322,17 @@ func New(c *common.Config) (processors.Processor, error) {
}

p := &addCloudMetadata{
initData: &initData{fetchers, config.Timeout},
initData: &initData{fetchers, config.Timeout, config.Overwrite},
}

go p.initOnce.Do(p.init)
return p, nil
}

type initData struct {
fetchers []*metadataFetcher
timeout time.Duration
fetchers []*metadataFetcher
timeout time.Duration
overwrite bool
}

type addCloudMetadata struct {
Expand All @@ -342,7 +348,6 @@ func (p *addCloudMetadata) init() {
return
}
p.metadata = result.metadata
p.initData = nil
logp.Info("add_cloud_metadata: hosting provider type detected as %v, metadata=%v",
result.provider, result.metadata.String())
}
Expand All @@ -358,8 +363,16 @@ func (p *addCloudMetadata) Run(event *beat.Event) (*beat.Event, error) {
return event, nil
}

// This overwrites the meta.cloud if it exists. But the cloud key should be
// reserved for this processor so this should happen.
// If cloud key exists in event already and overwrite flag is set to false, this processor will not overwrite the
// cloud fields. For example aws module writes cloud.instance.* to events already, with overwrite=false,
// add_cloud_metadata should not overwrite these fields with new values.
if !p.initData.overwrite {
cloudValue, _ := event.GetValue("cloud")
if cloudValue != nil {
return event, nil
}
}

_, err := event.PutValue("cloud", meta)

return event, err
Expand Down
183 changes: 171 additions & 12 deletions libbeat/processors/add_cloud_metadata/provider_aws_ec2_test.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -64,7 +64,8 @@ func TestRetrieveAWSMetadata(t *testing.T) {
defer server.Close()

config, err := common.NewConfigFrom(map[string]interface{}{
"host": server.Listener.Addr().String(),
"host": server.Listener.Addr().String(),
"overwrite": false,
})
if err != nil {
t.Fatal(err)
Expand All @@ -75,23 +76,181 @@ func TestRetrieveAWSMetadata(t *testing.T) {
t.Fatal(err)
}

actual, err := p.Run(&beat.Event{Fields: common.MapStr{}})
cases := []struct {
fields common.MapStr
expectedResults common.MapStr
}{
{
common.MapStr{},
common.MapStr{
"cloud": common.MapStr{
"provider": "ec2",
"instance": common.MapStr{
"id": "i-11111111",
},
"machine": common.MapStr{
"type": "t2.medium",
},
"region": "us-east-1",
"availability_zone": "us-east-1c",
},
},
},
{
common.MapStr{
"cloud": common.MapStr{
"instance": common.MapStr{
"id": "i-000",
},
},
},
common.MapStr{
"cloud": common.MapStr{
"instance": common.MapStr{
"id": "i-000",
},
},
},
},
{
common.MapStr{
"provider": "ec2",
},
common.MapStr{
"provider": "ec2",
"cloud": common.MapStr{
"provider": "ec2",
"instance": common.MapStr{
"id": "i-11111111",
},
"machine": common.MapStr{
"type": "t2.medium",
},
"region": "us-east-1",
"availability_zone": "us-east-1c",
},
},
},
{
common.MapStr{
"cloud.provider": "ec2",
},
// NOTE: In this case, add_cloud_metadata will overwrite cloud fields because
// it won't detect cloud.provider as a cloud field. This is not the behavior we
// expect and will find a better solution later in issue 11697.
common.MapStr{
"cloud.provider": "ec2",
"cloud": common.MapStr{
"provider": "ec2",
"instance": common.MapStr{
"id": "i-11111111",
},
"machine": common.MapStr{
"type": "t2.medium",
},
"region": "us-east-1",
"availability_zone": "us-east-1c",
},
},
},
}

for _, c := range cases {
actual, err := p.Run(&beat.Event{Fields: c.fields})
if err != nil {
t.Fatal(err)
}
assert.Equal(t, c.expectedResults, actual.Fields)
}
}

func TestRetrieveAWSMetadataOverwriteTrue(t *testing.T) {
logp.TestingSetup()

server := initEC2TestServer()
defer server.Close()

config, err := common.NewConfigFrom(map[string]interface{}{
"host": server.Listener.Addr().String(),
"overwrite": true,
})
if err != nil {
t.Fatal(err)
}

p, err := New(config)
if err != nil {
t.Fatal(err)
}

expected := common.MapStr{
"cloud": common.MapStr{
"provider": "ec2",
"instance": common.MapStr{
"id": "i-11111111",
cases := []struct {
fields common.MapStr
expectedResults common.MapStr
}{
{
common.MapStr{},
common.MapStr{
"cloud": common.MapStr{
"provider": "ec2",
"instance": common.MapStr{
"id": "i-11111111",
},
"machine": common.MapStr{
"type": "t2.medium",
},
"region": "us-east-1",
"availability_zone": "us-east-1c",
},
},
},
{
common.MapStr{
"cloud": common.MapStr{
"instance": common.MapStr{
"id": "i-000",
},
},
},
"machine": common.MapStr{
"type": "t2.medium",
common.MapStr{
"cloud": common.MapStr{
"provider": "ec2",
"instance": common.MapStr{
"id": "i-11111111",
},
"machine": common.MapStr{
"type": "t2.medium",
},
"region": "us-east-1",
"availability_zone": "us-east-1c",
},
},
"region": "us-east-1",
"availability_zone": "us-east-1c",
},
{
common.MapStr{
"cloud.provider": "ec2",
},
common.MapStr{
"cloud.provider": "ec2",
"cloud": common.MapStr{
"provider": "ec2",
"instance": common.MapStr{
"id": "i-11111111",
},
"machine": common.MapStr{
"type": "t2.medium",
},
"region": "us-east-1",
"availability_zone": "us-east-1c",
},
},
},
}

for _, c := range cases {
actual, err := p.Run(&beat.Event{Fields: c.fields})
if err != nil {
t.Fatal(err)
}
assert.Equal(t, c.expectedResults, actual.Fields)
}
assert.Equal(t, expected, actual.Fields)
}

0 comments on commit 17edc90

Please sign in to comment.