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.

Sign up for GitHub

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

Jump to bottom

Change add_cloud_metadata to not overwrite cloud field #11612

Merged
merged 9 commits into from
Apr 9, 2019
Merged

Change add_cloud_metadata to not overwrite cloud field #11612

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
6bc48c2
Change add_cloud_metadata to not overwrite cloud field
kaiyan-sheng Apr 2, 2019
d755b88
Update changelog
kaiyan-sheng Apr 2, 2019
faabe4a
Only check cloudValue for overwriting
kaiyan-sheng Apr 2, 2019
c6862b4
Add overwrite flag for add_cloud_metadata
kaiyan-sheng Apr 3, 2019
9dc80c3
Add test for cloud.provider: ec2
kaiyan-sheng Apr 4, 2019
fff9fd3
Check if event field key starts with cloud. case
kaiyan-sheng Apr 4, 2019
6a3e60b
Remove code to check dots in field keys
kaiyan-sheng Apr 8, 2019
7329339
Update processors-using.asciidoc
kaiyan-sheng Apr 9, 2019
551cc93
Update doc
kaiyan-sheng Apr 9, 2019
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
3 changes: 3 additions & 0 deletions CHANGELOG.next.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -77,6 +77,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]
kaiyan-sheng marked this conversation as resolved.
Show resolved Hide resolved
- Change `add_cloud_metadata` processor to not overwrite `cloud` field when it's already exist in the event. {pull}11612[11612] {issue}11305[11305]
kaiyan-sheng marked this conversation as resolved.
Show resolved Hide resolved
- 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
Open in desktop
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
Open in desktop
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
Open in desktop
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{
kaiyan-sheng marked this conversation as resolved.
Show resolved Hide resolved
"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",
Copy link
Member

Choose a reason for hiding this comment

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

Really expected? Not even sure what ES will do with such a document.

"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",
kaiyan-sheng marked this conversation as resolved.
Show resolved Hide resolved
"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)
}