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

[RFC] 0017 Remove log.original stage 2 #1347

Merged
merged 13 commits into from
May 31, 2021
Merged

[RFC] 0017 Remove log.original stage 2 #1347

merged 13 commits into from
May 31, 2021

Conversation

djptek
Copy link
Contributor

@djptek djptek commented Apr 8, 2021

  • Have you signed the contributor license agreement? Y
  • Have you followed the contributor guidelines? Y
  • For proposing substantial changes or additions to the schema, have you reviewed the RFC process? Y
  • If submitting code/script changes, have you verified all tests pass locally using make test? Y
  • If submitting schema/fields updates, have you generated new artifacts by running make and committed those changes? n/a
  • Is your pull request against master? Unless there is a good reason otherwise, we prefer pull requests against master and will backport as needed. Y
  • Have you added an entry to the CHANGELOG.next.md? Pending outcome of RFC process

Progress RFC 0017 Remove log.original to stage 2

@djptek djptek requested review from ebeahan and kgeller April 8, 2021 15:52
@ebeahan ebeahan added the RFC label Apr 12, 2021
@ebeahan
Copy link
Member

ebeahan commented Apr 19, 2021

Thanks, @djptek, for continuing to advance this.

I came across a use of log.original in the Kibana Logs UI app: https://github.com/elastic/kibana/blob/master/x-pack/plugins/infra/server/services/log_entries/message/builtin_rules/generic.test.ts#L136. May be worth following up to see if there are any side-effects introduced there if we removed log.original from the schema.

@ebeahan
Copy link
Member

ebeahan commented Apr 19, 2021

@elastic/security-external-integrations This is a proposal to deprecate log.original in ECS 1.x and remove the field entirely at the next major. Would one or two folks be able to review and share any feedback or concerns from your perspective?

@djptek
Copy link
Contributor Author

djptek commented Apr 20, 2021

@ebeahan yep, there are a few more

grep -Rw "log.original" * | wc                        (master)kibana
grep: test/plugin_functional/plugins/kbn_tp_sample_panel_action/node_modules/.bin/plugin-helpers: No such file or directory
grep: test/plugin_functional/plugins/kbn_tp_embeddable_explorer/node_modules/.bin/plugin-helpers: No such file or directory
      36    8844  388351
beats % grep -Rw "log.original" * | wc                          (master)beats
     610    6782  248334
elasticsearch % grep -Rw "log.original" * | wc          (master)elasticsearch
       0       0       0
logstash % grep -Rw "log.original" * | wc                    (master)logstash
       0       0       0

I would expect this could be resolved in Kibana through an alias, however that won't work in Beats and there may be further impact where original is referenced as a property of log.

I think that's less likely in Logstash, by the timelines but I'll look anyway & I will definitely need to do some digging in elasticsearch/x-pack/plugin/ml and ingest as they may be acting directly on _source therefore the aliases won't help.

I will review this thoroughly update the RFC

@djptek
Copy link
Contributor Author

djptek commented May 26, 2021

I ran make update on Beats with an ECS 1.6 fields.ecs.yml re-generated with log.original excluded

Some of these were actually references to [Filebeat] [SIEM] Fileset for Cisco FTD logs #13286 where this change had already been implemented

cisco/asa fileset: Renamed log.original to event.original and cisco.asa.list_id to cisco.asa.rule_name.

The majority of references are mappings e.g.

`"msg": {to:[{field: "log.original", setter: fld_set}]}``

in https://github.com/elastic/beats/blob/3a81d81b7f48c7bce3220095e412a98feace5728/x-pack/filebeat/module/fortinet/clientendpoint/config/liblogparser.js#L1069

Full list of remaining references was:

CHANGELOG.asciidoc
build/html_docs/filebeat/docs/filebeat-module-gcp.html
build/html_docs/filebeat/docs/raw/filebeat-module-gcp.html
build/html_docs/libbeat/docs/release-notes-7.4.0.html
build/html_docs/libbeat/docs/breaking-changes-7.4.html
build/html_docs/libbeat/docs/raw/release-notes-7.4.0.html
build/html_docs/libbeat/docs/raw/breaking-changes-7.4.html
filebeat/module/santa/log/test/santa.log-expected.json
filebeat/module/santa/log/ingest/pipeline.yml
filebeat/docs/modules/gcp.asciidoc
filebeat/build/package/module/santa/log/ingest/pipeline.yml
libbeat/docs/release-notes/breaking/breaking-7.4.asciidoc
x-pack/filebeat/module/panw/panos/test/pan_inc_threat.log-expected.json
x-pack/filebeat/module/panw/panos/test/traffic.log-expected.json
x-pack/filebeat/module/panw/panos/test/threat.log-expected.json
x-pack/filebeat/module/panw/panos/test/pan_inc_other.log-expected.json
x-pack/filebeat/module/panw/panos/test/pan_inc_traffic.log-expected.json
x-pack/filebeat/module/panw/panos/ingest/pipeline.yml
x-pack/filebeat/module/cylance/protect/config/liblogparser.js
x-pack/filebeat/module/zscaler/zia/config/liblogparser.js
x-pack/filebeat/module/cyberark/corepas/config/liblogparser.js
x-pack/filebeat/module/tomcat/log/config/liblogparser.js
x-pack/filebeat/module/squid/log/config/liblogparser.js
x-pack/filebeat/module/microsoft/dhcp/config/liblogparser.js
x-pack/filebeat/module/f5/bigipafm/config/liblogparser.js
x-pack/filebeat/module/f5/bigipapm/config/liblogparser.js
x-pack/filebeat/module/proofpoint/emailsecurity/config/liblogparser.js
x-pack/filebeat/module/barracuda/spamfirewall/config/liblogparser.js
x-pack/filebeat/module/barracuda/waf/config/liblogparser.js
x-pack/filebeat/module/netscout/sightline/config/liblogparser.js
x-pack/filebeat/module/bluecoat/director/config/liblogparser.js
x-pack/filebeat/module/cisco/shared/ingest/asa-ftd-pipeline.yml
x-pack/filebeat/module/cisco/ios/pipeline_test.go
x-pack/filebeat/module/cisco/ios/test/cisco-ios-syslog.log-expected.json
x-pack/filebeat/module/cisco/ios/config/pipeline.js
x-pack/filebeat/module/cisco/nexus/config/liblogparser.js
x-pack/filebeat/module/cisco/meraki/config/liblogparser.js
x-pack/filebeat/module/sonicwall/firewall/test/generated.log-expected.json
x-pack/filebeat/module/sonicwall/firewall/test/general.log-expected.json
x-pack/filebeat/module/sonicwall/firewall/config/liblogparser.js
x-pack/filebeat/module/gcp/_meta/docs.asciidoc
x-pack/filebeat/module/radware/defensepro/config/liblogparser.js
x-pack/filebeat/module/snort/log/config/liblogparser.js
x-pack/filebeat/module/infoblox/nios/config/liblogparser.js
x-pack/filebeat/module/juniper/netscreen/config/liblogparser.js
x-pack/filebeat/module/juniper/junos/config/liblogparser.js
x-pack/filebeat/module/juniper/srx/ingest/pipeline.yml
x-pack/filebeat/module/sophos/xg/ingest/pipeline.yml
x-pack/filebeat/module/sophos/utm/test/generated.log-expected.json
x-pack/filebeat/module/sophos/utm/config/liblogparser.js
x-pack/filebeat/module/iptables/log/test/geo.log-expected.json
x-pack/filebeat/module/iptables/log/test/ubiquiti.log-expected.json
x-pack/filebeat/module/iptables/log/test/ipv6.log-expected.json
x-pack/filebeat/module/iptables/log/test/icmp.log-expected.json
x-pack/filebeat/module/iptables/log/test/iptables.log-expected.json
x-pack/filebeat/module/iptables/log/ingest/pipeline.yml
x-pack/filebeat/module/imperva/securesphere/config/liblogparser.js
x-pack/filebeat/module/mssql/log/test/test.log-expected.json
x-pack/filebeat/module/mssql/log/ingest/pipeline.yml
x-pack/filebeat/module/fortinet/fortimail/config/liblogparser.js
x-pack/filebeat/module/fortinet/fortimanager/config/liblogparser.js
x-pack/filebeat/module/fortinet/clientendpoint/config/liblogparser.js
x-pack/filebeat/build/package/module/panw/panos/ingest/pipeline.yml
x-pack/filebeat/build/package/module/cylance/protect/config/liblogparser.js
x-pack/filebeat/build/package/module/zscaler/zia/config/liblogparser.js
x-pack/filebeat/build/package/module/cyberark/corepas/config/liblogparser.js
x-pack/filebeat/build/package/module/tomcat/log/config/liblogparser.js
x-pack/filebeat/build/package/module/squid/log/config/liblogparser.js
x-pack/filebeat/build/package/module/microsoft/dhcp/config/liblogparser.js
x-pack/filebeat/build/package/module/f5/bigipafm/config/liblogparser.js
x-pack/filebeat/build/package/module/f5/bigipapm/config/liblogparser.js
x-pack/filebeat/build/package/module/proofpoint/emailsecurity/config/liblogparser.js
x-pack/filebeat/build/package/module/barracuda/spamfirewall/config/liblogparser.js
x-pack/filebeat/build/package/module/barracuda/waf/config/liblogparser.js
x-pack/filebeat/build/package/module/netscout/sightline/config/liblogparser.js
x-pack/filebeat/build/package/module/bluecoat/director/config/liblogparser.js
x-pack/filebeat/build/package/module/cisco/shared/ingest/asa-ftd-pipeline.yml
x-pack/filebeat/build/package/module/cisco/ios/pipeline_test.go
x-pack/filebeat/build/package/module/cisco/ios/config/pipeline.js
x-pack/filebeat/build/package/module/cisco/nexus/config/liblogparser.js
x-pack/filebeat/build/package/module/cisco/meraki/config/liblogparser.js
x-pack/filebeat/build/package/module/sonicwall/firewall/config/liblogparser.js
x-pack/filebeat/build/package/module/radware/defensepro/config/liblogparser.js
x-pack/filebeat/build/package/module/santa/log/ingest/pipeline.yml
x-pack/filebeat/build/package/module/snort/log/config/liblogparser.js
x-pack/filebeat/build/package/module/infoblox/nios/config/liblogparser.js
x-pack/filebeat/build/package/module/juniper/netscreen/config/liblogparser.js
x-pack/filebeat/build/package/module/juniper/junos/config/liblogparser.js
x-pack/filebeat/build/package/module/juniper/srx/ingest/pipeline.yml
x-pack/filebeat/build/package/module/sophos/xg/ingest/pipeline.yml
x-pack/filebeat/build/package/module/sophos/utm/config/liblogparser.js
x-pack/filebeat/build/package/module/iptables/log/ingest/pipeline.yml
x-pack/filebeat/build/package/module/imperva/securesphere/config/liblogparser.js
x-pack/filebeat/build/package/module/mssql/log/ingest/pipeline.yml
x-pack/filebeat/build/package/module/fortinet/fortimail/config/liblogparser.js
x-pack/filebeat/build/package/module/fortinet/fortimanager/config/liblogparser.js
x-pack/filebeat/build/package/module/fortinet/clientendpoint/config/liblogparser.js

@djptek
Copy link
Contributor Author

djptek commented May 26, 2021

Likewise Kibana has plenty of references to log.original

bazel-kibana/data/node_auto_transpilation_cache_v3/7.13
bazel-kibana/data/node_auto_transpilation_cache_v3/master/data.mdb
bazel-kibana/x-pack/test/apm_api_integration/common/fixtures/es_archiver/8.0.0/mappings.json
bazel-kibana/x-pack/test/apm_api_integration/common/fixtures/es_archiver/observability_overview/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/infra/simple_logs/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/packetbeat/tls/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/singlecluster_yellow_basic_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/singlecluster_three_nodes_shard_relocation_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/logstash/changing_pipelines_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/singlecluster_basic_beats_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/singlecluster_yellow_platinum_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/setup/collection/es_and_kibana_exclusive_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/setup/collection/kibana_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/setup/collection/detect_beats_management_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/setup/collection/detect_beats/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/setup/collection/detect_beats_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/setup/collection/kibana_exclusive_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/setup/collection/es_and_kibana_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/setup/collection/detect_apm/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/multi_basic_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/singlecluster_lots_of_nodes_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/beats_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/apm_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/beats_with_restarted_instance_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/singlecluster_red_platinum_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/logstash_pipelines_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/logs_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/logs/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/multicluster_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/logstash_pipelines_multicluster_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/ccr_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/standalone_cluster_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/singlecluster_green_gold_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/singlecluster_green_trial_two_nodes_one_cgrouped_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/basic_6.3.x_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/singlecluster_green_platinum_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/logs_multiple_clusters_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/singlecluster_yellow_platinum_with_10_alerts_mb/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/monitoring/logs_multiple_clusters/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/uptime/full_heartbeat/mappings.json
bazel-kibana/x-pack/test/functional/es_archives/uptime/blank/mappings.json
bazel-kibana/x-pack/test/security_solution_cypress/es_archives/auditbeat_for_exceptions/mappings.json
bazel-kibana/x-pack/test/security_solution_cypress/es_archives/auditbeat/mappings.json
bazel-kibana/x-pack/test/security_solution_cypress/es_archives/auditbeat_for_exceptions2/mappings.json
bazel-kibana/x-pack/plugins/rule_registry/target/types/common/assets/field_maps/ecs_field_map.d.ts
bazel-kibana/x-pack/plugins/rule_registry/common/assets/field_maps/ecs_field_map.ts
bazel-kibana/x-pack/plugins/security_solution/server/utils/beat_schema/fields.ts
bazel-kibana/x-pack/plugins/infra/server/services/log_entries/message/builtin_rules/generic.ts
bazel-kibana/x-pack/plugins/infra/server/services/log_entries/message/builtin_rules/generic.test.ts
bazel-kibana/x-pack/plugins/apm/target/public/apm.chunk.3.js
bazel-kibana/x-pack/plugins/apm/target/public/apm.chunk.3.js.map
bazel-kibana/x-pack/plugins/observability/target/public/observability.chunk.2.js.map
bazel-kibana/x-pack/plugins/observability/target/public/observability.chunk.2.js
bazel-kibana/x-pack/plugins/observability/public/components/shared/exploratory_view/configurations/test_data/test_index_pattern.json
bazel-kibana/src/plugins/apm_oss/server/tutorial/index_pattern.json
data/node_auto_transpilation_cache_v3/7.13
data/node_auto_transpilation_cache_v3/master/data.mdb
src/plugins/apm_oss/server/tutorial/index_pattern.json
x-pack/test/apm_api_integration/common/fixtures/es_archiver/8.0.0/mappings.json
x-pack/test/apm_api_integration/common/fixtures/es_archiver/observability_overview/mappings.json
x-pack/test/functional/es_archives/infra/simple_logs/mappings.json
x-pack/test/functional/es_archives/packetbeat/tls/mappings.json
x-pack/test/functional/es_archives/monitoring/singlecluster_yellow_basic_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/singlecluster_three_nodes_shard_relocation_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/logstash/changing_pipelines_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/singlecluster_basic_beats_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/singlecluster_yellow_platinum_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/setup/collection/es_and_kibana_exclusive_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/setup/collection/kibana_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/setup/collection/detect_beats_management_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/setup/collection/detect_beats/mappings.json
x-pack/test/functional/es_archives/monitoring/setup/collection/detect_beats_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/setup/collection/kibana_exclusive_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/setup/collection/es_and_kibana_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/setup/collection/detect_apm/mappings.json
x-pack/test/functional/es_archives/monitoring/multi_basic_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/singlecluster_lots_of_nodes_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/beats_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/apm_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/beats_with_restarted_instance_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/singlecluster_red_platinum_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/logstash_pipelines_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/logs_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/logs/mappings.json
x-pack/test/functional/es_archives/monitoring/multicluster_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/logstash_pipelines_multicluster_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/ccr_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/standalone_cluster_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/singlecluster_green_gold_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/singlecluster_green_trial_two_nodes_one_cgrouped_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/basic_6.3.x_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/singlecluster_green_platinum_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/logs_multiple_clusters_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/singlecluster_yellow_platinum_with_10_alerts_mb/mappings.json
x-pack/test/functional/es_archives/monitoring/logs_multiple_clusters/mappings.json
x-pack/test/functional/es_archives/uptime/full_heartbeat/mappings.json
x-pack/test/functional/es_archives/uptime/blank/mappings.json
x-pack/test/security_solution_cypress/es_archives/auditbeat_for_exceptions/mappings.json
x-pack/test/security_solution_cypress/es_archives/auditbeat/mappings.json
x-pack/test/security_solution_cypress/es_archives/auditbeat_for_exceptions2/mappings.json
x-pack/plugins/rule_registry/target/types/common/assets/field_maps/ecs_field_map.d.ts
x-pack/plugins/rule_registry/common/assets/field_maps/ecs_field_map.ts
x-pack/plugins/security_solution/server/utils/beat_schema/fields.ts
x-pack/plugins/infra/server/services/log_entries/message/builtin_rules/generic.ts
x-pack/plugins/infra/server/services/log_entries/message/builtin_rules/generic.test.ts
x-pack/plugins/apm/target/public/apm.chunk.3.js
x-pack/plugins/apm/target/public/apm.chunk.3.js.map
x-pack/plugins/observability/target/public/observability.chunk.2.js.map
x-pack/plugins/observability/target/public/observability.chunk.2.js
x-pack/plugins/observability/public/components/shared/exploratory_view/configurations/test_data/test_index_pattern.json

@djptek djptek marked this pull request as ready for review May 28, 2021 12:31
@djptek djptek marked this pull request as draft May 28, 2021 13:10
@djptek djptek marked this pull request as ready for review May 28, 2021 13:20
Copy link
Member

@ebeahan ebeahan left a comment

Choose a reason for hiding this comment

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

LGTM 💯

I have a few minor notes, but overall I think looks great. Thanks for being thorough and connecting with the other teams/stakeholders mentioned here, @djptek!

@djptek djptek merged commit 1783bcf into elastic:master May 31, 2021
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