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

Remove experimental event.original definition #1053

Merged
merged 4 commits into from
Nov 10, 2020

Conversation

ebeahan
Copy link
Member

@ebeahan ebeahan commented Oct 27, 2020

Summary

While testing the experimental build artifacts, I discovered the generated experimental ES index templates will not install.

Error Details

Elasticsearch returns a HTTP 400 error when attempting to install the generated ES index template:

 curl -XPUT -H "Content-Type: application/json" http://localhost:9200/_template/test -d @experimental/generated/elasticsearch/7/template.json 
{"error":{"root_cause":[{"type":"mapper_parsing_exception","reason":"The field [original] cannot have index = false"}],"type":"mapper_parsing_exception","reason":"Failed to parse mapping [_doc]: The field [original] cannot have index = false","caused_by":{"type":"mapper_parsing_exception","reason":"The field [original] cannot have index = false"}},"status":400}

Proposed Solution

Set index: true in the experimental schema for event.original to override. Also correct in the stage 2 wildcard RFC field definitions.

UPDATED: Agreement reached in #1015 to remove event.original from the initial wildcard field migration. Updating this PR to remove the field from the experimental definitions for 1.7 to avoid users encountering this issue.

@ebeahan
Copy link
Member Author

ebeahan commented Oct 27, 2020

I think this makes sense to backport for 1.7.

@ebeahan ebeahan added the 1.x label Oct 28, 2020
@webmat
Copy link
Contributor

webmat commented Oct 28, 2020

If we move forward with this PR, I 100% agree we should backport to 1.7.

However @leehinman had pointed out in #970 (comment) that indexing it could lead to a significant performance hit. So we kept the wildcard type and decided to leave it index: false.

This may be an inconsistency in how Elasticsearch deals with type:keyword + index:false vs doing the same with type:wildcard.

I think a more appropriate fix would be to adjust our ES template generator, and not set a type at all, when we set index:false.

@ebeahan
Copy link
Member Author

ebeahan commented Oct 28, 2020

However @leehinman had pointed out in #970 (comment) that indexing it could lead to a significant performance hit. So we kept the wildcard type and decided to leave it index: false.

That's right - we did make the decision to remove. Apologies for not recalling that point.

@ebeahan
Copy link
Member Author

ebeahan commented Oct 29, 2020

Compared the result of setting index: false on the three keyword family field types.

keyword

request:

PUT keyword-index
{
  "mappings": {
    "properties": {
      "keyword": {
        "type": "keyword",
        "index": false
      }
    }
  }
}

response:

{
  "acknowledged" : true
}

constant_keyword

request:

{
  "mappings": {
    "properties": {
      "constant_keyword": {
        "type": "constant_keyword",
        "index": false
      }
    }
  }
}

response:

{
  "acknowledged" : true
}

wildcard

request:

PUT wildcard-index
{
  "mappings": {
    "properties": {
      "wildcard": {
        "type": "wildcard",
        "index": false
      }
    }
  }
}

response:

{
  "error" : {
    "root_cause" : [
      {
        "type" : "mapper_parsing_exception",
        "reason" : "The field [wildcard] cannot have index = false"
      }
    ],
    "type" : "mapper_parsing_exception",
    "reason" : "Failed to parse mapping [_doc]: The field [wildcard] cannot have index = false",
    "caused_by" : {
      "type" : "mapper_parsing_exception",
      "reason" : "The field [wildcard] cannot have index = false"
    }
  },
  "status" : 400
}

@markharwood - We came across this inconsistency when using type: wildcard + index: false compared to the other keyword family types. Would you be able to comment if this expected for wildcard?

@ebeahan
Copy link
Member Author

ebeahan commented Oct 29, 2020

I think a more appropriate fix would be to adjust our ES template generator, and not set a type at all, when we set index:false

I experimented with omitting the type but saw exceptions returned when trying to create the index:

{
  "error" : {
    "root_cause" : [
      {
        "type" : "mapper_parsing_exception",
        "reason" : "No type specified for field [none]"
      }
    ],
    "type" : "mapper_parsing_exception",
    "reason" : "Failed to parse mapping [_doc]: No type specified for field [none]",
    "caused_by" : {
      "type" : "mapper_parsing_exception",
      "reason" : "No type specified for field [none]"
    }
  },
  "status" : 400
}

Is there a proper way to omit the type?

@ebeahan ebeahan added the ready Issues we'd like to address in the future. label Nov 3, 2020
@ebeahan
Copy link
Member Author

ebeahan commented Nov 7, 2020

Confirmed wildcard doesn't support setting index: false (or doc_values: false) by design. Reference: https://github.com/elastic/elasticsearch/pull/49993/files#r357572257.

Will move discussion to #1015 for determining how to proceed with handling event.original and wildcard.

@ebeahan ebeahan changed the title Fix experimental event original Remove experimental event original Nov 10, 2020
@ebeahan ebeahan changed the title Remove experimental event original Remove experimental event.original definition Nov 10, 2020
@ebeahan ebeahan force-pushed the fix-experimental-event-original branch from 223c73c to 0bfa754 Compare November 10, 2020 17:48
Copy link
Contributor

@webmat webmat left a comment

Choose a reason for hiding this comment

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

Alright, if Travis is happy with it, LGTM!

@ebeahan ebeahan merged commit 18fc6af into elastic:master Nov 10, 2020
@ebeahan ebeahan deleted the fix-experimental-event-original branch November 10, 2020 18:11
ebeahan added a commit to ebeahan/ecs that referenced this pull request Nov 10, 2020
# Conflicts:
#	experimental/generated/csv/fields.csv
ebeahan added a commit to ebeahan/ecs that referenced this pull request Nov 10, 2020
# Conflicts:
#	experimental/generated/csv/fields.csv
@ebeahan ebeahan removed 1.x needs_backport ready Issues we'd like to address in the future. labels Nov 10, 2020
dseeley added a commit to dseeley/ecs that referenced this pull request May 5, 2021
* bumping version for 1.x release branch (elastic#921)

* [1.x] add related.hosts (elastic#913) (elastic#924)

* [1.x][DOCS] Fixes SIEM links (elastic#936)

* [1.x] Consolidate field-details doc template (elastic#897) (elastic#946)

* Add http.[request|response].mime_type (elastic#944) (elastic#949)

* [1.x] Cut 1.6 Changelog (elastic#933) (elastic#952) (elastic#953)

Co-authored-by: Mathieu Martin <mathieu.martin@elastic.co>

* [1.x] Add threat.technique.subtechnique (elastic#951) (elastic#956)

Co-authored-by: Ross Wolf <31489089+rw-access@users.noreply.github.com>

* [1.x] Nest as for foreign reuse (elastic#960) (elastic#962)

* [1.x] Remove `expected_event_types` from protocol (elastic#964) (elastic#965)

* [1.x] Expand definitions of source and destination field sets (elastic#967) (elastic#973)

* [1.x] Introduce `--strict` flag (elastic#937) (elastic#975)

* [1.x] Add example value composite type checking (elastic#966) (elastic#976)

* Add example value composite type checking (elastic#966)
* generate csv artifact

* [1.x] Add event category configuration (elastic#963) (elastic#977)

* [1.x] Add normalizer multi-field capability (elastic#971) (elastic#978)

Co-authored-by: Eric Beahan <ebeahan@gmail.com>

Co-authored-by: Madison Caldwell <madison.rey.caldwell@gmail.com>

* [1.x] Add mapping network event guidance doc (elastic#969) (elastic#983)

* [1.x] Removing unneeded link under `Additional Information` (elastic#984) (elastic#985)

* [1.x] Add discrete attribute to field details page headers (elastic#989) (elastic#990)

* [1.x] Uniformity across domain name breakdown fields (elastic#981) (elastic#994)

Co-authored-by: Mathieu Martin <webmat@gmail.com>

* Add --oss flag to the ECS generator script (elastic#991) (elastic#995)

* Add network directions ingress and egress (elastic#945) (elastic#997)

* Mention ECS Mapper in the main documentation (elastic#987) (elastic#1000)

Co-authored-by: Dan Roscigno <dan@roscigno.com>

* [1.x] Introduce experimental artifacts (elastic#993) (elastic#1001)

Co-authored-by: Mathieu Martin <webmat@gmail.com>

* Bump version to 1.8.0-dev in branch 1.x (elastic#1011)

* Cut 1.7 changelog (elastic#1010) (elastic#1012)

* [1.x] Clarify that file extension should exclude the dot. (elastic#1016) (elastic#1020)

* [1.x] Add usage docs section (elastic#988) (elastic#1024)

Co-authored-by: Mathieu Martin <mathieu.martin@elastic.co>

* [1.x] feat: include alias path when generating template (elastic#877) (elastic#1035)

Co-authored-by: Richard Gomez <32133502+rgmz@users.noreply.github.com>

* [1.x] Add support for `scaling_factor` in the generator (elastic#1042) (elastic#1055)

Co-authored-by: Mathieu Martin <mathieu.martin@elastic.co>

* [1.x] Add fallback for constant_keyword (elastic#1046) (elastic#1056)

Co-authored-by: Mathieu Martin <mathieu.martin@elastic.co>

* [1.x] Add wildcard type support to go code generator (elastic#1050) (elastic#1057)

* add wildcard type support

* also add version and constant_keyword

* changelog

* [1.x] New default make task that generates main and experimental artifacts. (elastic#1041) (elastic#1060)

Also changing the order of the 'generate' task: it now starts with the new generator, then runs the legacy scripts.

* [1.x] Change the index pattern in the sample template. (elastic#1048) (elastic#1068)

* [1.x] Prepare link to Logs docs changing with the 7.10 release in "getting-started" (elastic#1073) (elastic#1079)

Co-authored-by: EamonnTP <Eamonn.Smith@elastic.co>

* [1.x] Prepare link to Logs docs changing with the 7.10 release in "products-solutions" page (elastic#1074) (elastic#1083)

Co-authored-by: EamonnTP <Eamonn.Smith@elastic.co>

* [1.x] Add event.category session. (elastic#1049) (elastic#1093)

Co-authored-by: Mathieu Martin <mathieu.martin@elastic.co>

* [1.x] Add event.category registry (elastic#1040) (elastic#1094)

Co-authored-by: Mathieu Martin <mathieu.martin@elastic.co>

* [1.x] Add --ref support for experimental artifacts (elastic#1063) (elastic#1101)

Co-authored-by: Mathieu Martin <webmat@gmail.com>

* [1.x] Remove experimental event.original definition (elastic#1053) (elastic#1104)

* [1.x] Add missing `process.thread.name` to experimental definitions (elastic#1103) (elastic#1106)

* [1.x] Remove index parameter for wildcard fields (elastic#1115) (elastic#1119)

* [1.x] Add dns.answer object into experimental schema (elastic#1118) (elastic#1121)

* [1.x] Clarify x509 definition guidance for network events with only one cert (elastic#1114) (elastic#1123)

* [1.x] Indicate when artifacts include experimental changes (elastic#1117) (elastic#1125)

* [1.x] Add os.type field, with list of allowed values (elastic#1111) (elastic#1130)

* [1.x] Add support for constant_keyword's 'value' parameter (elastic#1112) (elastic#1132)

* [1.x] Beta label support (elastic#1051) (elastic#1133)

Co-authored-by: Mathieu Martin <webmat@gmail.com>

* [1.x] Backport elastic#1134 and elastic#1135 (elastic#1136)

* Remove temporary ifeval in "getting started" page, add link to Metrics docs (elastic#1134)
* Remove temporary ifeval from products page, add link to Metrics (elastic#1135)

* Two small documentation backports (elastic#1149)

* Remove an incorrect `event.type` from the 'converting' page (elastic#1146)
* Mention Logstash support for ECS in the 'products' page (elastic#1147)

* [1.x] Reinforce the exclusion of the leading dot from url.extension (elastic#1151) (elastic#1152)

* [1.x] Make all fields linkable directly via an HTML ID (elastic#1148) (elastic#1154)

* [1.x] Tracing fields should be at the root (elastic#1165)

* Add notice to the tracing field set, about not nesting field names. (elastic#1162)
* Tracing fields should be at top level in Beats artifact (elastic#1164)

* [1.x] Usage of brackets for a URL containing IPv6 address (elastic#1131) (elastic#1168)

* [1.x] 6.x index template data type fallback (elastic#1171) (elastic#1172)

* [1.x] Apply RFC 0007 stage 3 changes - multi-user (elastic#1066) (elastic#1175)

Conflict: deleted file rfcs/text/0007-multiple-users.md as RFCs are not backported to version branches.

* [1.x] Handle `error.stack_trace` case for ES 6.x template (elastic#1176) (elastic#1177)

* [1.x] Add composable index templates artifacts (elastic#1156) (elastic#1179)

* [1.x] Move _meta section back inside mappings, in legacy templates. (elastic#1186) (elastic#1187)

Backports the following commits to 1.x:

* Move _meta section back inside mappings, in legacy templates. (elastic#1186) 

This fixes an issue introduced by elastic#1156, discovered in elastic#1180. Composable templates support `_meta` at the template's root, but legacy templates don't. So we're just putting it back inside the mappings for legacy templates.

This also fixes missing updates to the component template, after the introduction of wildcard in elastic#1098.

* [1.x] Apply the RFC 0005 stage 2 (host metrics) changes in the experimental artifacts (elastic#1159) (elastic#1184)

Co-authored-by: Mathieu Martin <mathieu.martin@elastic.co>

* [1.x] Stage 3 changes for wildcard RFC 0001 (elastic#1098) (elastic#1183)

* [1.x] Conditional handling in es_template.template_settings (elastic#1191) (elastic#1192)

* [1.x] Artifacts docs page (elastic#1189) (elastic#1195)

* [1.x] Remove beta warning label from categorization fields docs (elastic#1067) (elastic#1196)

* [1.x] Correct wording of `event.reference` description (elastic#1181) (elastic#1197)

* Bump version to 1.9.0-dev in branch 1.x (elastic#1198)

* [1.x] Cut 1.8 FF changelog.next.md elastic#1199 (elastic#1201)

* Merge custom and core multi_fields arrays (elastic#982) (elastic#1213)

Co-authored-by: Jonathan Buttner <56361221+jonathan-buttner@users.noreply.github.com>

* [1.x] Stage 2 changes for RFC 0009 - data_stream fields (elastic#1215) (elastic#1222)

* [1.x] add http.request.id (elastic#1208) (elastic#1223)

Co-authored-by: Eric Beahan <eric.beahan@elastic.co>
Co-authored-by: Gil Raphaelli <gil@elastic.co>

* [1.x] add cloud.service.name (elastic#1204) (elastic#1224)

* add cloud.platform

* expand cloud.platform description

* move to cloud.service.name

Co-authored-by: Gil Raphaelli <gil@elastic.co>

* [1.x] Add ssdeep hash (elastic#1169) (elastic#1227)

Co-authored-by: Andrew Stucki <andrew.stucki@elastic.co>

* [CI] Switch to GitHub actions (elastic#1236) (elastic#1245)

Co-authored-by: Eric Beahan <ebeahan@gmail.com>

Co-authored-by: Andrew Stucki <andrew.stucki@elastic.co>

* Revert wildcard adoption back to experimental stage (elastic#1235) (elastic#1243)

* Add scaled_float type to go generator (elastic#1250) (elastic#1251)

* add scaled_float

* changelog

* Add categorization fields usage docs (elastic#1242) (elastic#1257)

* add time_zone, postal_code, and continent_code (elastic#1229) (elastic#1258)

* Specify MAC address format (elastic#456) (elastic#1260)

Co-authored-by: Robin Schneider <36660054+ypid-geberit@users.noreply.github.com>

* finalize 1.8.0 changelog (elastic#1262) (elastic#1265)

* Add additional host fields (elastic#1248) (elastic#1267)

Co-authored-by: kaiyan-sheng <kaiyan.sheng@elastic.co>

* Stage 1 changes for RFC 0014 - extend pe fields (elastic#1256) (elastic#1270)

* Add 2 fields to code_signature (elastic#1269) (elastic#1272)

Co-authored-by: Yamin Tian <56367679+Trinity2019@users.noreply.github.com>

* Stage 3 changes for RFC 0007 - remove beta attribute (elastic#1271) (elastic#1273)

* Stage 1 experimental changes for RFC 0008 - threat.indicator fields (elastic#1268) (elastic#1274)

* Stage 1 changes for RFC 0015 - add elf fieldset (elastic#1261) (elastic#1275)

* Cut 1.9 FF CHANGELOG.next.md (elastic#1277)

* lock go version in actions (elastic#1283) (elastic#1290)

* Bump jinja2 from 2.11.2 to 2.11.3 in /scripts (elastic#1310) (elastic#1320)

* Bump jinja2 from 2.11.2 to 2.11.3 in /scripts

* Bump pyyaml from 5.3b1 to 5.4 in /scripts (elastic#1318) (elastic#1325)

Co-authored-by: Eric Beahan <eric.beahan@elastic.co>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* Adjust terminology - change whitelist to allowlist (elastic#1315) (elastic#1331)

Co-authored-by: Dominic Page <11043991+djptek@users.noreply.github.com>

* Remove -dev label from 1.9 version (elastic#1329)

* remove -dev label from 1.9 version

* generate artifacts

* removing rules artifacts

* Cut 1.9 changelog (elastic#1328)

* move 1.9 changes to changelog

* add 1.9 release changes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants