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

[ECS] Cleaning up unused ECS field usages for juniper_srx and netflow integrations #8011

Merged
merged 2 commits into from Sep 29, 2023

Conversation

kgeller
Copy link
Contributor

@kgeller kgeller commented Sep 28, 2023

What does this PR do?

This is a further continuation of #7965 as there we some more unused ECS fields at the root-level (see comment)

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

Related issues

@elasticmachine
Copy link

elasticmachine commented Sep 28, 2023

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-09-28T22:13:38.172+0000

  • Duration: 17 min 8 sec

Test stats 🧪

Test Results
Failed 0
Passed 28
Skipped 0
Total 28

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@elasticmachine
Copy link

elasticmachine commented Sep 28, 2023

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (2/2) 💚
Files 100.0% (9/9) 💚 5.556
Classes 100.0% (9/9) 💚 5.556
Methods 100.0% (68/68) 💚 13.953
Lines 68.478% (2079/3036) 👎 -17.59
Conditionals 100.0% (0/0) 💚

@kgeller kgeller changed the title [ECS] Cleaning up unused ECS field usages [ECS] Cleaning up unused ECS field usages for juniper_srx and netflow integrations Sep 28, 2023
@kgeller kgeller marked this pull request as ready for review September 28, 2023 18:26
@kgeller kgeller requested a review from a team as a code owner September 28, 2023 18:26
@elasticmachine
Copy link

Pinging @elastic/security-external-integrations (Team:Security-External Integrations)

Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

The scope of the changes is larger than I expected.

For review purposes I was hoping we could get a PR that only addresses the invalid usages of ECS (i.e. external: ecs on fields that don't exist). Below are fields that are used incorrectly. If they don't appear in the ingest pipeline then they can be removed. Otherwise, the pipeline should be adjusted to use a valid ECS field.

Removing the unused fields can be its own change. That issue is not blocking the adoption of format_version: 3.0.0. Determining whether a field is unused takes a bit more effort to review. For example, as a reviewer I need to consider if the removed field is generated by the ingest node pipeline, the fleet final pipeline, any of the inputs, filebeat itself, any of the implicit processors (add_host_metadata, add_cloud_metadata, add_kubernetes_metadata). That will take me a bit more time to ensure correctness so I would like to separate the changes.

# Netflow
as.organization.name
geo.city_name
geo.continent_name
geo.country_iso_code
geo.country_name
geo.location
geo.name
geo.region_iso_code
geo.region_name
hash.md5
hash.sha1
hash.sha256
hash.sha512
os.family
os.full
os.kernel
os.name
os.platform
os.version
# Juniper SRX
as.organization.name
code_signature.exists
code_signature.status
code_signature.subject_name
code_signature.trusted
code_signature.valid
hash.md5
hash.sha1
hash.sha256
hash.sha512
pe.architecture
pe.company
pe.description
pe.file_version
pe.imphash
pe.original_file_name
pe.product

packages/juniper_srx/data_stream/log/fields/ecs.yml Outdated Show resolved Hide resolved
packages/juniper_srx/data_stream/log/fields/ecs.yml Outdated Show resolved Hide resolved
@kgeller kgeller marked this pull request as draft September 28, 2023 19:23
@kgeller
Copy link
Contributor Author

kgeller commented Sep 28, 2023

/test

@kgeller
Copy link
Contributor Author

kgeller commented Sep 28, 2023

For review purposes I was hoping we could get a PR that only addresses the invalid usages of ECS (i.e. external: ecs on fields that don't exist). Below are fields that are used incorrectly. If they don't appear in the ingest pipeline then they can be removed. Otherwise, the pipeline should be adjusted to use a valid ECS field.

@andrewkroh Updated the PR to only be the invalid fields. They do not exist in the pipelines, so it's just removals.

@kgeller kgeller marked this pull request as ready for review September 28, 2023 20:29
@kgeller
Copy link
Contributor Author

kgeller commented Sep 28, 2023

Pretty sure the build is only unhappy due to docker issues https://www.dockerstatus.com/ , so will re-run when it looks like things are better

@kgeller
Copy link
Contributor Author

kgeller commented Sep 28, 2023

/test

1 similar comment
@kgeller
Copy link
Contributor Author

kgeller commented Sep 28, 2023

/test

@andrewkroh andrewkroh merged commit eaa7acf into elastic:main Sep 29, 2023
4 checks passed
@kgeller kgeller deleted the 7808-field-cleanup branch September 29, 2023 12:55
@elasticmachine
Copy link

Package juniper_srx - 1.16.2 containing this change is available at https://epr.elastic.co/search?package=juniper_srx

@elasticmachine
Copy link

Package netflow - 2.15.2 containing this change is available at https://epr.elastic.co/search?package=netflow

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.

None yet

3 participants