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

allows disable pod events enrichment with deployment name #28521

Merged
merged 9 commits into from
Nov 8, 2021
Merged

allows disable pod events enrichment with deployment name #28521

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
ef84c87
allows disable pod events enrichment with deployment name
newly12 Oct 19, 2021
859f9aa
Merge remote-tracking branch 'upstream/master' into deployment_name
newly12 Oct 28, 2021
fb2e693
update doc and regression fix
newly12 Oct 28, 2021
b8f5d63
fix changelog
newly12 Oct 28, 2021
9556f83
Merge remote-tracking branch 'upstream/master' into deployment_name
newly12 Nov 4, 2021
3a1a5ac
fix merge conflict issue
newly12 Nov 4, 2021
9c023bb
incorporate suggested changes
newly12 Nov 4, 2021
357bb69
fix syntax error
newly12 Nov 5, 2021
821f06f
Merge remote-tracking branch 'upstream/master' into deployment_name
newly12 Nov 5, 2021
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.next.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ for a few releases. Please use other tools provided by Elastic to fetch data fro
- Fix handling of float data types within processors. {issue}28279[28279] {pull}28280[28280]
- Allow `clone3` syscall in seccomp filters. {pull}28117[28117]
- Remove unnecessary escaping step in dashboard loading, so they can be displayed in Kibana. {pull}28395[28395]
- Allows disable pod events enrichment with deployment name {pull}28521[28521]

*Auditbeat*

Expand Down
2 changes: 1 addition & 1 deletion libbeat/autodiscover/providers/kubernetes/pod_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2023,7 +2023,7 @@ func TestPod_EmitEvent(t *testing.T) {
t.Fatal(err)
}

metaGen := metadata.NewPodMetadataGenerator(common.NewConfig(), nil, client, nil, nil)
metaGen := metadata.NewPodMetadataGenerator(common.NewConfig(), nil, client, nil, nil, nil)
p := &Provider{
config: defaultConfig(),
bus: bus.New(logp.NewLogger("bus"), "test"),
Expand Down
10 changes: 6 additions & 4 deletions libbeat/common/kubernetes/metadata/config.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,8 +32,9 @@ type Config struct {

// AddResourceMetadataConfig allows adding config for enriching additional resources
type AddResourceMetadataConfig struct {
Node *common.Config `config:"node"`
Namespace *common.Config `config:"namespace"`
Node *common.Config `config:"node"`
Namespace *common.Config `config:"namespace"`
Deployment bool `config:"deployment"`
}

// InitDefaults initializes the defaults for the config.
Expand All @@ -52,7 +53,8 @@ func GetDefaultResourceMetadataConfig() *AddResourceMetadataConfig {
metaConfig.InitDefaults()
metaCfg, _ := common.NewConfigFrom(&metaConfig)
return &AddResourceMetadataConfig{
Node: metaCfg,
Namespace: metaCfg,
Node: metaCfg,
Namespace: metaCfg,
Deployment: true,
}
}
2 changes: 1 addition & 1 deletion libbeat/common/kubernetes/metadata/metadata.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ func GetPodMetaGen(
if namespaceWatcher != nil && metaConf.Namespace.Enabled() {
namespaceMetaGen = NewNamespaceMetadataGenerator(metaConf.Namespace, namespaceWatcher.Store(), namespaceWatcher.Client())
}
metaGen := NewPodMetadataGenerator(cfg, podWatcher.Store(), podWatcher.Client(), nodeMetaGen, namespaceMetaGen)
metaGen := NewPodMetadataGenerator(cfg, podWatcher.Store(), podWatcher.Client(), nodeMetaGen, namespaceMetaGen, metaConf)
Copy link
Member

Choose a reason for hiding this comment

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

Can't you just pass the value of metaConf.Deployment as bool?


return metaGen
}
Expand Down
43 changes: 26 additions & 17 deletions libbeat/common/kubernetes/metadata/pod.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,12 @@ import (
)

type pod struct {
store cache.Store
client k8s.Interface
node MetaGen
namespace MetaGen
resource *Resource
store cache.Store
client k8s.Interface
node MetaGen
namespace MetaGen
resource *Resource
addDeployment bool
}

// NewPodMetadataGenerator creates a metagen for pod resources
Expand All @@ -42,13 +43,19 @@ func NewPodMetadataGenerator(
pods cache.Store,
client k8s.Interface,
node MetaGen,
namespace MetaGen) MetaGen {
namespace MetaGen,
metaCfg *AddResourceMetadataConfig) MetaGen {
addDeploymentMeta := true
Copy link
Member

Choose a reason for hiding this comment

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

In combination with the previous suggestion you might can move this one layer up to GetPodMetaGen function.

if metaCfg != nil {
addDeploymentMeta = metaCfg.Deployment
}
return &pod{
resource: NewResourceMetadataGenerator(cfg, client),
store: pods,
node: node,
namespace: namespace,
client: client,
resource: NewResourceMetadataGenerator(cfg, client),
store: pods,
node: node,
namespace: namespace,
client: client,
addDeployment: addDeploymentMeta,
}
}

Expand Down Expand Up @@ -84,11 +91,13 @@ func (p *pod) GenerateK8s(obj kubernetes.Resource, opts ...FieldOptions) common.
out := p.resource.GenerateK8s("pod", obj, opts...)

// check if Pod is handled by a ReplicaSet which is controlled by a Deployment
rsName, _ := out.GetValue("replicaset.name")
if rsName, ok := rsName.(string); ok {
dep := p.getRSDeployment(rsName, po.GetNamespace())
if dep != "" {
out.Put("deployment.name", dep)
if p.addDeployment {
rsName, _ := out.GetValue("replicaset.name")
if rsName, ok := rsName.(string); ok {
dep := p.getRSDeployment(rsName, po.GetNamespace())
if dep != "" {
out.Put("deployment.name", dep)
}
}
}

Expand All @@ -107,7 +116,7 @@ func (p *pod) GenerateK8s(obj kubernetes.Resource, opts ...FieldOptions) common.
meta := p.namespace.GenerateFromName(po.GetNamespace())
if meta != nil {
// Use this in 8.0
//out.Put("namespace", meta["namespace"])
// out.Put("namespace", meta["namespace"])
Copy link
Member

Choose a reason for hiding this comment

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

@MichaelKatsoulis this line looks like a leftover from other PR, shall we open a fixup PR to remove it?

out.DeepUpdate(meta)
}
}
Expand Down
6 changes: 3 additions & 3 deletions libbeat/common/kubernetes/metadata/pod_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ func TestPod_Generate(t *testing.T) {
})
assert.NoError(t, err)

metagen := NewPodMetadataGenerator(config, nil, client, nil, nil)
metagen := NewPodMetadataGenerator(config, nil, client, nil, nil, nil)
for _, test := range tests {
t.Run(test.name, func(t *testing.T) {
assert.Equal(t, test.output, metagen.Generate(test.input))
Expand Down Expand Up @@ -510,7 +510,7 @@ func TestPod_GenerateFromName(t *testing.T) {
assert.NoError(t, err)
pods := cache.NewStore(cache.MetaNamespaceKeyFunc)
pods.Add(test.input)
metagen := NewPodMetadataGenerator(config, pods, client, nil, nil)
metagen := NewPodMetadataGenerator(config, pods, client, nil, nil, nil)

accessor, err := meta.Accessor(test.input)
require.NoError(t, err)
Expand Down Expand Up @@ -634,7 +634,7 @@ func TestPod_GenerateWithNodeNamespace(t *testing.T) {
namespaces.Add(test.namespace)
nsMeta := NewNamespaceMetadataGenerator(config, namespaces, client)

metagen := NewPodMetadataGenerator(config, pods, client, nodeMeta, nsMeta)
metagen := NewPodMetadataGenerator(config, pods, client, nodeMeta, nsMeta, nil)
t.Run(test.name, func(t *testing.T) {
assert.Equal(t, test.output, metagen.Generate(test.input))
})
Expand Down
4 changes: 3 additions & 1 deletion libbeat/docs/shared-autodiscover.asciidoc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -245,7 +245,8 @@ endif::[]
`add_resource_metadata` can be done for `node` or `namespace`. By default all labels will be included
while annotations are not added by default. This settings are useful when labels' and annotations'
storing requires special handling to avoid overloading the storage output. The enrichment of `node` or `namespace` metadata
can be individually disabled by setting `enabled: false`.
can be individually disabled by setting `enabled: false`. If resource is `pod` and it is created from a `deployment`, by default
the deployment name is added, this can be disabled by set to `false`.
Example:

["source","yaml",subs="attributes"]
Expand All @@ -256,6 +257,7 @@ endif::[]
node:
include_labels: ["nodelabel2"]
include_annotations: ["nodeannotation1"]
deployment: false
-------------------------------------------------------------------------------------

`unique`:: (Optional) Defaults to `false`. Marking an autodiscover provider as unique results into
Expand Down
8 changes: 4 additions & 4 deletions libbeat/processors/add_kubernetes_metadata/indexers_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ import (
"github.com/elastic/beats/v7/libbeat/common/kubernetes"
)

var metagen = metadata.NewPodMetadataGenerator(common.NewConfig(), nil, nil, nil, nil)
var metagen = metadata.NewPodMetadataGenerator(common.NewConfig(), nil, nil, nil, nil, nil)

func TestPodIndexer(t *testing.T) {
var testConfig = common.NewConfig()
Expand Down Expand Up @@ -92,7 +92,7 @@ func TestPodIndexer(t *testing.T) {
func TestPodUIDIndexer(t *testing.T) {
var testConfig = common.NewConfig()

metaGenWithPodUID := metadata.NewPodMetadataGenerator(common.NewConfig(), nil, nil, nil, nil)
metaGenWithPodUID := metadata.NewPodMetadataGenerator(common.NewConfig(), nil, nil, nil, nil, nil)

podUIDIndexer, err := NewPodUIDIndexer(*testConfig, metaGenWithPodUID)
assert.NoError(t, err)
Expand Down Expand Up @@ -307,7 +307,7 @@ func TestFilteredGenMeta(t *testing.T) {
})
assert.NoError(t, err)

filteredGen := metadata.NewPodMetadataGenerator(config, nil, nil, nil, nil)
filteredGen := metadata.NewPodMetadataGenerator(config, nil, nil, nil, nil, nil)

podIndexer, err = NewPodNameIndexer(*testConfig, filteredGen)
assert.NoError(t, err)
Expand Down Expand Up @@ -344,7 +344,7 @@ func TestFilteredGenMetaExclusion(t *testing.T) {
})
assert.NoError(t, err)

filteredGen := metadata.NewPodMetadataGenerator(config, nil, nil, nil, nil)
filteredGen := metadata.NewPodMetadataGenerator(config, nil, nil, nil, nil, nil)

podIndexer, err := NewPodNameIndexer(*testConfig, filteredGen)
assert.NoError(t, err)
Expand Down
4 changes: 2 additions & 2 deletions metricbeat/module/kubernetes/util/kubernetes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -139,7 +139,7 @@ func NewResourceMetadataEnricher(
cfg, _ := common.NewConfigFrom(&metaConfig)

metaGen := metadata.NewResourceMetadataGenerator(cfg, watcher.Client())
podMetaGen := metadata.NewPodMetadataGenerator(cfg, nil, watcher.Client(), nil, nil)
podMetaGen := metadata.NewPodMetadataGenerator(cfg, nil, watcher.Client(), nil, nil, &metadata.AddResourceMetadataConfig{Deployment: true})
Copy link
Member

Choose a reason for hiding this comment

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

Can't we leverage the value from the configuration here instead of setting always to true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

lol I was simply following same like the other arguments and meantime keep the same behavior like previous version. so current configuration doesn't have a section for namespace nor node, wondering if deployment should be added first there..

Copy link
Contributor Author

Choose a reason for hiding this comment

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

in the later commit I still passed a true to this function for the comment. we can discuss more on how this one should be handled properly.

Copy link
Member

Choose a reason for hiding this comment

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

Ah ok, I see! I think we can leave it as is for now and try to address it in general in another round. I will open an issue for this :).

serviceMetaGen := metadata.NewServiceMetadataGenerator(cfg, nil, nil, watcher.Client())
enricher := buildMetadataEnricher(watcher,
// update
Expand Down Expand Up @@ -228,7 +228,7 @@ func NewContainerMetadataEnricher(

cfg, _ := common.NewConfigFrom(&metaConfig)

metaGen := metadata.NewPodMetadataGenerator(cfg, nil, watcher.Client(), nil, nil)
metaGen := metadata.NewPodMetadataGenerator(cfg, nil, watcher.Client(), nil, nil, &metadata.AddResourceMetadataConfig{Deployment: true})
enricher := buildMetadataEnricher(watcher,
// update
func(m map[string]common.MapStr, r kubernetes.Resource) {
Expand Down