Skip to content

Add a target field support. #196

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

Merged
merged 6 commits into from
May 6, 2025

Conversation

mashhurs
Copy link
Contributor

@mashhurs mashhurs commented Apr 29, 2025

Introduces a target field as a field reference (mixin validated) where if set result is placed into the target.

Test logs

  • when using target and DSL query
// config
query => "magType:%{[target-input-field][magType]}"
fields => { "depthError" => "[target-input-field][depthErrorValue]" }
target => "my-ls-filter-target"

// result
{
    "my-ls-filter-target" => {
        "target-input-field" => {
            "depthErrorValue" => 1.697
        }
    },
             "@timestamp" => 2025-05-01T19:55:25.268932Z,
     "target-input-field" => {
                   "dmin" => 2.187,
              "magSource" => "us",
             "depthError" => 1.697,
               "latitude" => -6.3171,
         "locationSource" => "us",
                   "type" => "earthquake",
                "magType" => "mww",
                    "nst" => 324,
                    "mag" => 6.9,
                  "depth" => 10.0,
             "@timestamp" => 2025-04-04T20:04:38.266Z,
               "magError" => 0.028,
        "horizontalError" => 6.62,
                    "gap" => 11,
                    "rms" => 0.78,
               "location" => "POINT (151.5922 -6.3171)",
                   "time" => 2025-04-04T20:04:38.266Z,
                     "id" => "us6000q41n",
                  "place" => "181 km ESE of Kimbe, Papua New Guinea",
                 "magNst" => 121,
                    "net" => "us",
                "updated" => 2025-04-08T19:27:52.892Z,
                 "status" => "reviewed",
              "longitude" => 151.5922
    },
              "@metadata" => {
        "total_hits" => 10
    },
               "@version" => "1"
}

  • when using target and aggregation query
// config
target => "my-ls-filter-target"
aggregation_fields => { "my-aggr" => "my_ls_aggr_field" }
// result
{
    "my-ls-filter-target" => {
        "my_ls_aggr_field" => {
            "doc_count_error_upper_bound" => 0,
                    "sum_other_doc_count" => 0,
                                "buckets" => [
                [0] {
                    "doc_count" => 10,
                          "key" => "mww"
                },
                [1] {
                    "doc_count" => 2,
                          "key" => "mw"
                }
            ]
        }
    },
             "@timestamp" => 2025-05-01T19:53:19.292876Z,
     "target-input-field" => {
                   "dmin" => 2.187,
              "magSource" => "us",
             "depthError" => 1.697,
               "latitude" => -6.3171,
         "locationSource" => "us",
                   "type" => "earthquake",
                "magType" => "mww",
                    "nst" => 324,
                    "mag" => 6.9,
             "@timestamp" => 2025-04-04T20:04:38.266Z,
                  "depth" => 10.0,
               "magError" => 0.028,
        "horizontalError" => 6.62,
                    "gap" => 11,
                    "rms" => 0.78,
               "location" => "POINT (151.5922 -6.3171)",
                     "id" => "us6000q41n",
                   "time" => 2025-04-04T20:04:38.266Z,
                 "magNst" => 121,
                  "place" => "181 km ESE of Kimbe, Papua New Guinea",
                    "net" => "us",
                "updated" => 2025-04-08T19:27:52.892Z,
                 "status" => "reviewed",
              "longitude" => 151.5922
    },
              "@metadata" => {
        "total_hits" => 12
    },
               "@version" => "1"
}

@mashhurs mashhurs linked an issue Apr 29, 2025 that may be closed by this pull request
@mashhurs mashhurs self-assigned this Apr 29, 2025
@mashhurs mashhurs requested a review from yaauie April 29, 2025 20:28
event.set(new_key, value_to_set)
else
event_target = event.get(@target) || {}
@logger.debug("Overwriting existing target field", field: @target, existing_value: event_target) if @logger.debug? && event.include?(@target)
Copy link
Contributor

Choose a reason for hiding this comment

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

Because we do a field-oriented mapping and invoke this once for each top-level field in the result (with a set of the values of that field across all returned documents), a fields mapping of N entries will emit this debug message at least N-1 times (e.g., 1st field sets target, subsequent fields encounter already-set target), even if the target was empty when the filter received the event.

…pplies setting to target with aggregations similarly with es-input.

Co-authored-by: Rye Biesemeyer <ry.biesemeyer@elastic.co>
@mashhurs mashhurs force-pushed the target-field-support branch from 2e9e643 to 7342069 Compare May 1, 2025 20:03
@@ -84,7 +84,9 @@
end

it "fails to register plugin" do
expect { plugin.register }.to raise_error Elasticsearch::Transport::Transport::Errors::Unauthorized
expect { plugin.register }.to raise_error elastic_ruby_v8_client_available? ?
Copy link
Contributor Author

Choose a reason for hiding this comment

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

review note: no any logic related with this but this fixes one integration test failure.

@mashhurs mashhurs requested a review from yaauie May 1, 2025 20:49
Copy link
Contributor

@yaauie yaauie left a comment

Choose a reason for hiding this comment

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

I've left a couple of nitpicks, but this looks sensible.

We may want to also link back to target in each of the affected fields (and improve their docs while we're at it), something like:

diff --git a/docs/index.asciidoc b/docs/index.asciidoc
index b5a6eff..f86be0c 100644
--- a/docs/index.asciidoc
+++ b/docs/index.asciidoc
@@ -176,8 +176,11 @@ filter plugins.
 
   * Value type is <<hash,hash>>
   * Default value is `{}`
+  * format: `+"aggregation_name" => "[path][on][event]"+`:
+  ** `aggregation_name`: aggregation name in result from {es}
+  ** `[path][on][event]`: path for where to place the value on the current event, using field-reference notation
 
-Hash of aggregation names to copy from elasticsearch response into Logstash event fields
+A mapping of aggregations to copy into the <<plugins-{type}s-{plugin}-target>> of the current event.
 
 Example:
 [source,ruby]
@@ -247,8 +250,11 @@ These custom headers will override any headers previously set by the plugin such
 
   * Value type is <<hash,hash>>
   * Default value is `{}`
+  * format: `+"path.in.source" => "[path][on][event]"+`:
+  ** `path.in.source`: field path in document source of result from {es}, using dot-notation
+  ** `[path][on][event]`: path for where to place the value on the current event, using field-reference notation
 
-Hash of docinfo fields to copy from old event (found via elasticsearch) into new event
+A mapping of docinfo (`_source`) fields to copy into the <<plugins-{type}s-{plugin}-target>> of the current event.
 
 Example:
 [source,ruby]
@@ -274,9 +280,11 @@ Whether results should be sorted or not
 
   * Value type is <<array,array>>
   * Default value is `{}`
+  * format: `+"path.in.result" => "[path][on][event]"+`:
+  ** `path.in.result`: field path in indexed result from {es}, using dot-notation
+  ** `[path][on][event]`: path for where to place the value on the current event, using field-reference notation
 
-An array of fields to copy from the old event (found via elasticsearch) into the
-new event, currently being processed.
+A mapping of indexed fields to copy into the <<plugins-{type}s-{plugin}-target>> of the current event.
 
 In the following example, the values of `@timestamp` and `event_id` on the event
 found via elasticsearch are copied to the current event's

mashhurs and others added 2 commits May 6, 2025 11:40
Documentation suggestions.

Co-authored-by: Rye Biesemeyer <yaauie@users.noreply.github.com>
@mashhurs
Copy link
Contributor Author

mashhurs commented May 6, 2025

I've left a couple of nitpicks, but this looks sensible.

We may want to also link back to target in each of the affected fields (and improve their docs while we're at it), something like:

diff --git a/docs/index.asciidoc b/docs/index.asciidoc
index b5a6eff..f86be0c 100644
--- a/docs/index.asciidoc
+++ b/docs/index.asciidoc
@@ -176,8 +176,11 @@ filter plugins.
 
   * Value type is <<hash,hash>>
   * Default value is `{}`
+  * format: `+"aggregation_name" => "[path][on][event]"+`:
+  ** `aggregation_name`: aggregation name in result from {es}
+  ** `[path][on][event]`: path for where to place the value on the current event, using field-reference notation
 
-Hash of aggregation names to copy from elasticsearch response into Logstash event fields
+A mapping of aggregations to copy into the <<plugins-{type}s-{plugin}-target>> of the current event.
 

Applied with commit. The additional changes I made:

  • As I remember we had a doc render issue when using + to keep original text (for =>), so removed + for consistency;
  • just realized I didn't add target support on docsinfo_fields and added with this commit as well

I will be merging on 🟢 CI and iterate with separate changes if required.
I will release 3.x first by rebasing this change.

@mashhurs mashhurs merged commit 5abbe49 into logstash-plugins:main May 6, 2025
3 checks passed
mashhurs added a commit to mashhurs/logstash-filter-elasticsearch that referenced this pull request May 6, 2025
* Introduces a target field as a field reference (mixin validated) where if set result is placed into the target.

* Simplifies the set extracted values to the event with target logic. Applies setting to target with aggregations similarly with es-input.

* Mention to target in each fields which can be placed in the target. Docs info fields are placed in target field.

---------

Co-authored-by: Rye Biesemeyer <yaauie@users.noreply.github.com>
(cherry picked from commit 5abbe49)
mashhurs added a commit that referenced this pull request May 7, 2025
* Introduces a target field as a field reference (mixin validated) where if set result is placed into the target.

* Simplifies the set extracted values to the event with target logic. Applies setting to target with aggregations similarly with es-input.

* Mention to target in each fields which can be placed in the target. Docs info fields are placed in target field.

---------


(cherry picked from commit 5abbe49)

Co-authored-by: Rye Biesemeyer <yaauie@users.noreply.github.com>
@mashhurs mashhurs deleted the target-field-support branch May 7, 2025 17:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a target field support
2 participants