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

[Metricbeat] Rename http.request.body to http.request.body.content for ECS #10315

Merged
merged 4 commits into from
Jan 28, 2019

Conversation

ruflin
Copy link
Contributor

@ruflin ruflin commented Jan 24, 2019

The header fields were not touched in this change as not defined yet in ECS.

@ruflin ruflin added module review Metricbeat Metricbeat ecs Team:Integrations Label for the Integrations team labels Jan 24, 2019
@ruflin ruflin requested a review from webmat January 24, 2019 13:52
@ruflin ruflin requested a review from a team as a code owner January 24, 2019 13:52
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.

Looking good so far.

  • One thing to adjust in ecs-migration.
  • Also, I assume the actual fields.yml changes are coming? ;-)

### HTTP
- from: http.request.body
to: http.request.body.content
alias: true
Copy link
Contributor

Choose a reason for hiding this comment

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

Can't be aliased unfortunately, as the new field is moving directly under it. This means http.request.body can't be of type alias, since it has .content nested under 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.

Good point :-(

@ruflin ruflin requested a review from a team as a code owner January 25, 2019 13:04
@ruflin ruflin changed the title [Metricbeat] Rename http.request.body to http.request.body.content fo… [Metricbeat] Rename http.request.body to http.request.body.content for ECS Jan 25, 2019
@@ -18,10 +18,6 @@
type: object
description: >
The HTTP headers sent
- name: body
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was removed because body.content is already in ECS

@ruflin ruflin self-assigned this Jan 25, 2019
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.

Looking good, once the changelog gets the PR ID :-)

CHANGELOG.next.asciidoc Show resolved Hide resolved
@ruflin ruflin merged commit dc5cf51 into elastic:master Jan 28, 2019
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.

3 participants