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

Adding service.node.roles #1981

Merged
merged 11 commits into from
Jul 6, 2022
Merged

Adding service.node.roles #1981

merged 11 commits into from
Jul 6, 2022

Conversation

kgeller
Copy link
Contributor

@kgeller kgeller commented Jul 5, 2022

Adding the plural service.node.roles field per #1965

@lukeelmers / @matschaffer let me know if you want any tweaks to the description. I mostly kept what you had for the singlular.

@kgeller kgeller added the 8.4.0 label Jul 5, 2022
@kgeller kgeller requested review from matschaffer and lukeelmers July 5, 2022 18:32
@kgeller kgeller requested a review from a team as a code owner July 5, 2022 18:32
@kgeller kgeller self-assigned this Jul 5, 2022
Copy link
Member

@lukeelmers lukeelmers left a comment

Choose a reason for hiding this comment

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

Thanks for updating this! Noticed one typo & one thing we changed in the Kibana PR, otherwise LGTM.

cc @kobelb

@matschaffer
Copy link
Contributor

The origin/target part is unfamiliar to me. Are we adding that just to maintain parity with where service.node.name is already present in ECS?

@djptek
Copy link
Contributor

djptek commented Jul 6, 2022

@matschaffer

The origin/target part is unfamiliar to me

these were already added see e.g. #1916

this PR only superceeds the single-valued service.node.role field replacing this with multi-valued service.node.roles

@matschaffer
Copy link
Contributor

Oh right! I forgot about the generation step.

Copy link
Contributor

@djptek djptek left a comment

Choose a reason for hiding this comment

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

LGTM

@kgeller kgeller merged commit cbbc4eb into elastic:main Jul 6, 2022
@kgeller kgeller deleted the service-node-roles branch July 6, 2022 14:03
kgeller added a commit to kgeller/ecs that referenced this pull request Jul 6, 2022
# Conflicts:
#	experimental/generated/csv/fields.csv
#	generated/csv/fields.csv
kgeller added a commit that referenced this pull request Jul 6, 2022
mitodrummer pushed a commit to mitodrummer/ecs that referenced this pull request Jul 6, 2022
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.

4 participants