Skip to content

[Metricbeat] Align rabbitmq with ECS and have module fields#10563

Merged
ruflin merged 5 commits intoelastic:masterfrom
ruflin:rabbitmq-ecs2
Feb 5, 2019
Merged

[Metricbeat] Align rabbitmq with ECS and have module fields#10563
ruflin merged 5 commits intoelastic:masterfrom
ruflin:rabbitmq-ecs2

Conversation

@ruflin
Copy link
Copy Markdown
Contributor

@ruflin ruflin commented Feb 5, 2019

No description provided.

@ruflin ruflin added module review Metricbeat Metricbeat ecs Team:Integrations Label for the Integrations team labels Feb 5, 2019
@ruflin ruflin self-assigned this Feb 5, 2019
@ruflin ruflin requested a review from a team as a code owner February 5, 2019 13:18
@ruflin ruflin requested a review from a team as a code owner February 5, 2019 13:24
@ruflin ruflin requested a review from webmat February 5, 2019 13:24
Copy link
Copy Markdown
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.

Nothing major. Perhaps you had good reasons, but here are my concerns:

  • A field def is deleted instead of being aliased to rabbitmq.node.name
  • Why delete assertions, shouldn't you instead adjust the field names in the assertion? Even if redundant field names, different code paths set these values.

@webmat
Copy link
Copy Markdown
Contributor

webmat commented Feb 5, 2019

@ruflin Oh, forgot: ecs-migration.yml for that missing alias

@ruflin
Copy link
Copy Markdown
Contributor Author

ruflin commented Feb 5, 2019

@webmat Seems like it's already there?

Copy link
Copy Markdown
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.

LGTM

@ruflin You're right. Didn't see it in your recent commit fixing fields.yml. But it was already in there before that. 👍

@ruflin ruflin merged commit 38d256c into elastic:master Feb 5, 2019
@ruflin ruflin deleted the rabbitmq-ecs2 branch February 5, 2019 18:37
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

ecs Metricbeat Metricbeat module review Team:Integrations Label for the Integrations team

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants