Skip to content

Introduce add_labels and add_tags processors#9973

Merged
urso merged 8 commits intoelastic:masterfrom
urso:fields-tags-processor
Jan 11, 2019
Merged

Introduce add_labels and add_tags processors#9973
urso merged 8 commits intoelastic:masterfrom
urso:fields-tags-processor

Conversation

@urso
Copy link
Copy Markdown

@urso urso commented Jan 9, 2019

Introduce add_fields and add_tags processors. The fields/tags settings will be marked deprecated starting with 6.7.

Replace custom processors in pipeline/processors.go with new definitions.

Cleanup pipeline/processors.go by removing dead code.

Fix recent regressions from beat -> agent renaming potentially triggering a panic due to concurrent map updates.

@urso urso requested a review from a team as a code owner January 9, 2019 19:08
@urso urso added in progress Pull request is currently in progress. libbeat labels Jan 9, 2019
@urso urso force-pushed the fields-tags-processor branch from 193c6fc to 1b9de01 Compare January 9, 2019 19:18
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

This is a great example for me. Thanks! Just curious, is this test := test necessary here?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

It depends. I tend to put it here so to bind the name locally in case I enable t.Parallel(). functions/go-routines in a for-block accessing a loop variable will see the value being changed otherwise.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

handy tricks, I should use it more often.

Copy link
Copy Markdown
Contributor

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

Great to see these 2 new processor. Left some feedback related to ECS.

If we remove the feature in 7.0, would be perhaps provide some migration code to modify the config to use the processors instead? Not sure how comment the usage of these 2 is, but I assume not rare.

@urso
Copy link
Copy Markdown
Author

urso commented Jan 10, 2019

Note: I will add documentation later. There is some more cleanup/renaming of other processors I want to do as well first.

@urso urso changed the title [WIP] Introduce add_fields and add_tags processors Introduce add_labels and add_tags processors Jan 10, 2019
@urso urso added review needs_backport PR is waiting to be backported to other branches. and removed in progress Pull request is currently in progress. labels Jan 10, 2019
@urso urso force-pushed the fields-tags-processor branch from d1ded38 to acc6372 Compare January 10, 2019 17:25
@ph ph self-requested a review January 10, 2019 17:32
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

handy tricks, I should use it more often.

Copy link
Copy Markdown
Contributor

@ruflin ruflin left a comment

Choose a reason for hiding this comment

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

Will the following config work?

processors:
  add_labels:
    labels:
      foo.bar: 17

I'm mainly concerned about the . in the key. If not, is that something we want to support? Would be nice to have some system tests to confirm this works or not.

@urso
Copy link
Copy Markdown
Author

urso commented Jan 11, 2019

Will the following config work?

What do you mean by work. It works the same current fields settings work. You configuration will produce a document saying:

{
  ...
  "labels": {
    "foo": {
      "bar": 17
    }
  },
  ...
}

The way we read config files one can only choose between dedot or flat dottet output always. The information whether a dot was used in the config or not is lossed thanks to the config file parser.

@urso urso merged commit 14373c7 into elastic:master Jan 11, 2019
@ruflin ruflin mentioned this pull request Jan 14, 2019
@urso urso removed the needs_backport PR is waiting to be backported to other branches. label Jan 31, 2019
@urso urso deleted the fields-tags-processor branch February 19, 2019 18:32
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.

6 participants