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

[libbeat]add EC2 fields to yaml for add_cloud_metadata #12335

Conversation

odacremolbap
Copy link
Contributor

Related to #12307
As mentioned here #12307 (comment)

Adding fields to yaml file for EC2 cloud metadata

@exekias

@@ -44,3 +49,14 @@
type: alias
path: cloud.region
migration: true

- name: meta.cloud.account.id
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do not have the full context on this one but I wonder why migration: true is missing?

Copy link
Contributor

@exekias exekias May 29, 2019

Choose a reason for hiding this comment

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

I submitted another comment with the review 😇, I guess we don't need these alias cause they are primary minded for backwards compatibility, but these are new fields, isn't it?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I copied it from existing fields.yml

I though that those fields needed to be defined at the event, and then aliased to ECS.
are there some docs on how to use this fields.yml at processors and how it fits with ECS?

Copy link
Contributor

Choose a reason for hiding this comment

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

Sorry I don't think there are docs about this. These aliases were created in the context of ECS transition (see #9265 for more details). In a nutshell, old fields were not ECS compliant, so everything was moved to ECS fields, then old fields became aliases pointing to the new ones, as a way to allow smoother upgrades for users.

As you are adding new fields, there is no need to create alias from old names, as nobody is relying on these names for their visualizations

Copy link
Contributor

@exekias exekias left a comment

Choose a reason for hiding this comment

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

Thank you for opening this!

I think account.id field is missing?

This will probably require a make update

@@ -44,3 +49,14 @@
type: alias
path: cloud.region
migration: true

- name: meta.cloud.account.id
Copy link
Contributor

Choose a reason for hiding this comment

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

These new alias are not needed. They were initially created for backwards compatibility with previous names. As these fields are new, we don't need to have them in the old layout

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I thought data from the processor was created under meta.cloud ... and then had to be aliased to ECS cloud. ...

idk how this works, but sounds like I just need to remove this block from fields.yml, and that somehow the field will be mapped to ECS

@odacremolbap odacremolbap requested a review from a team as a code owner May 29, 2019 15:59
@odacremolbap
Copy link
Contributor Author

not sure if something is missing:

  • added cloud.image.id since it is not part of ECS
  • removed previously added at this PR cloud.account.id, being part of ECS

@odacremolbap odacremolbap merged commit f59c5f7 into elastic:master May 31, 2019
@odacremolbap odacremolbap deleted the add_missing_fields_yaml_ec2_cloud_metadata branch May 31, 2019 09:14
DStape pushed a commit to DStape/beats that referenced this pull request Aug 20, 2019
* add EC2 fields to yaml for add_cloud_metadata
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
docs libbeat review Team:Integrations Label for the Integrations team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants