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

Adding support for include_labels and include_annotations in kubernetes processor #4043

Merged
merged 1 commit into from
Apr 19, 2017

Conversation

vjsamuel
Copy link
Contributor

include_labels: ["foo"] would make sure that only foo is part of the "labels" map in the kubernetes metadata.

if user wishes to add select annotations then it can be done using include_annotations: ["xyz"]

@elasticmachine
Copy link
Collaborator

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run.

1 similar comment
@elasticmachine
Copy link
Collaborator

Jenkins standing by to test this. If you aren't a maintainer, you can ignore this comment. Someone with commit access, please review this and clear it for Jenkins to run.

return meta
}

type GenFilteredMeta struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering, why do you need 2 different metadata generators? All the filtering logic could go into the default, isn't 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.

just makes the default one slightly faster with lesser checks ;)

//Load default indexer configs
if config.DefaultIndexers.Enabled == true {
for key, cfg := range Indexing.defaultIndexerConfigs {
config.Indexers = append(config.Indexers, map[string]common.Config{key: cfg})
Copy link
Contributor

Choose a reason for hiding this comment

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

this lacks rwmutex logic

Copy link
Contributor Author

Choose a reason for hiding this comment

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

would this need the rwmutex part? processor init is anyway something that is serial

Copy link
Contributor

Choose a reason for hiding this comment

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

that's as of today, some day we may add changes like processors reloading that could mess with concurrency. I prefer just to trust the functions are thread safe as other modules are calling them

//Load default matcher configs
if config.DefaultMatchers.Enabled == true {
for key, cfg := range Indexing.defaultMatcherConfigs {
config.Matchers = append(config.Matchers, map[string]common.Config{key: cfg})
Copy link
Contributor

Choose a reason for hiding this comment

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

rwmutex too

@ruflin ruflin added the libbeat label Apr 19, 2017
@exekias exekias merged commit d4222cc into elastic:master Apr 19, 2017
@vjsamuel vjsamuel deleted the kubernetes_include_labels branch May 2, 2017 15:12
@jstangroome jstangroome mentioned this pull request Nov 14, 2017
10 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants