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

Skip add_kubernetes_metadata processor if kubernetes metadata are aleady there #27689

Merged
merged 5 commits into from
Sep 3, 2021
Merged

Skip add_kubernetes_metadata processor if kubernetes metadata are aleady there #27689

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
5 commits
Select commit Hold shift + click to select a range
e1bde3a
Skip add_kubernetes_metadata processor if kubernetes metadata are alr…
MichaelKatsoulis Aug 31, 2021
4f75d6c
Add removed line
MichaelKatsoulis Sep 1, 2021
dfa3c64
Update changelog
MichaelKatsoulis Sep 1, 2021
b6d4d53
Add test for skipping of the annotator
MichaelKatsoulis Sep 2, 2021
3955e71
Trigger Build
MichaelKatsoulis Sep 3, 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 @@ -22,6 +22,7 @@ https://github.com/elastic/beats/compare/v7.0.0-alpha2...master[Check the HEAD d
- Add daemonset.name in pods controlled by DaemonSets {pull}26808[26808], {issue}25816[25816]
- Kubernetes autodiscover fails in node scope if node name cannot be discovered {pull}26947[26947]
- Loading Kibana assets (dashboards, index templates) rely on Saved Object API. So to provide a reliable service, Beats can only import and export dasbhboards using at least Kibana 7.15. {issue}20672[20672] {pull}27220[27220]
- Skip add_kubernetes_metadata processor when kubernetes metadata are already present {pull}27689[27689]

*Auditbeat*

Expand Down
12 changes: 12 additions & 0 deletions libbeat/processors/add_kubernetes_metadata/kubernetes.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,14 @@ func isKubernetesAvailableWithRetry(client k8sclient.Interface) bool {
}
}

// kubernetesMetadataExist checks whether an event is already enriched with kubernetes metadata
func kubernetesMetadataExist(event *beat.Event) bool {
if _, err := event.GetValue("kubernetes"); err != nil {
return false
}
return true
}

// New constructs a new add_kubernetes_metadata processor.
func New(cfg *common.Config) (processors.Processor, error) {
config, err := newProcessorConfig(cfg, Indexing)
Expand Down Expand Up @@ -251,6 +259,10 @@ func (k *kubernetesAnnotator) Run(event *beat.Event) (*beat.Event, error) {
if !k.kubernetesAvailable {
return event, nil
}
if kubernetesMetadataExist(event) {
k.log.Debug("Skipping add_kubernetes_metadata processor as kubernetes metadata already exist")
Copy link
Member

Choose a reason for hiding this comment

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

This will be reporting the log for every single event, means that even if it is in debug level it could become quite overwhelming.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Only in case user enables debug mode. In this Run method there are other debug messages as well. Do you think we shouldn't log this at all?

Copy link
Member

Choose a reason for hiding this comment

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

Ok, since we have already another debug message in the main flow we can preserve this one too. Maybe in the future we can revisit the logging in this method if we see it becomes noisy, but for now we can keep it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Also we log each published event in processing/processors.go:203. So tbh the logging of one line that the processor is skipped is nothing comparing to that

return event, nil
}
index := k.matchers.MetadataIndex(event.Fields)
if index == "" {
k.log.Debug("No container match string, not adding kubernetes data")
Expand Down
11 changes: 3 additions & 8 deletions libbeat/processors/add_kubernetes_metadata/kubernetes_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,8 @@ import (
"github.com/elastic/beats/v7/libbeat/logp"
)

// Test metadata updates don't replace existing pod metrics
func TestAnnotatorDeepUpdate(t *testing.T) {
// Test Annotator is skipped if kubernetes metadata already exist
func TestAnnotatorSkipped(t *testing.T) {
cfg := common.MustNewConfigFrom(map[string]interface{}{
"lookup_fields": []string{"kubernetes.pod.name"},
})
Expand All @@ -53,8 +53,7 @@ func TestAnnotatorDeepUpdate(t *testing.T) {
"kubernetes": common.MapStr{
"pod": common.MapStr{
"labels": common.MapStr{
"dont": "replace",
"original": "fields",
"added": "should not",
},
},
},
Expand Down Expand Up @@ -85,10 +84,6 @@ func TestAnnotatorDeepUpdate(t *testing.T) {
"a": 1,
"b": 2,
},
"labels": common.MapStr{
"dont": "replace",
"original": "fields",
},
},
},
}, event.Fields)
Expand Down