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.

Sign up for GitHub

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

Jump to bottom

socket_summary add more tcp status #9430

Merged
merged 12 commits into from
Dec 19, 2018
Merged

socket_summary add more tcp status #9430

merged 12 commits into from
Dec 19, 2018

Conversation

yaule
Copy link
Contributor

@yaule yaule commented Dec 7, 2018 •
edited by alvarolobato
Loading

No description provided.

@elasticmachine
Copy link
Collaborator

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

@ruflin ruflin added module review Metricbeat Metricbeat Team:Integrations Label for the Integrations team labels Dec 7, 2018
@elasticmachine
Copy link
Collaborator

Pinging @elastic/infrastructure

@elasticcla
Copy link

Hi @yaule, we have found your signature in our records, but it seems like you have signed with a different e-mail than the one used in yout Git commit. Can you please add both of these e-mails into your Github profile (they can be hidden), so we can match your e-mails to your Github profile?

@ruflin ruflin requested a review from sayden December 7, 2018 12:05
@sayden
Copy link
Contributor

sayden commented Dec 7, 2018

Hi @yaule Thanks for the PR. We'll need also a CHANGELOG entry. CI error is probably because a make update command is missing.

@alvarolobato alvarolobato assigned sayden and unassigned sayden Dec 10, 2018
@sayden
Copy link
Contributor

sayden commented Dec 11, 2018

Awesome! We are one step closer, now we need to add some test. Take a look at the Beat Developer Guide https://www.elastic.co/guide/en/beats/metricbeat/5.5/metricbeat-developer-guide.html and check any other module for examples, like MySQL

@sayden
Copy link
Contributor

sayden commented Dec 11, 2018

Here you can see the current test of the module that you can also modify to add the contents of this PR https://github.com/elastic/beats/blob/037e45f566228976f65b8b6352734f6aa921696e/metricbeat/module/system/socket_summary/socket_summary_test.go

@yaule
Copy link
Contributor Author

yaule commented Dec 14, 2018

hi, @sayden . I submitted the code for the first time. Now I don't know what to do. Please tell me what I should do now.

@jsoriano
Copy link
Member

Hi @yaule, I think the code looks mostly fine, failing test in CI is not related.

@jsoriano
Copy link
Member

jenkins, test this

jsoriano
jsoriano reviewed Dec 14, 2018
View reviewed changes
Copy link
Member

@jsoriano jsoriano 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 adding the tests, this LGTM, I have added some suggestions about fields descriptions.

metricbeat/module/system/socket_summary/_meta/fields.yml Outdated Show resolved Hide resolved
metricbeat/module/system/socket_summary/_meta/fields.yml Outdated Show resolved Hide resolved
metricbeat/module/system/socket_summary/_meta/fields.yml Outdated Show resolved Hide resolved
@yaule
Copy link
Contributor Author

yaule commented Dec 14, 2018

@jsoriano Thank you!

jsoriano and others added 2 commits December 14, 2018 22:07
@jsoriano @yaule
Apply suggestions from code review
5f42c0c 
Co-Authored-By: yaule <lijixing@gmail.com>
@yaule
update asset data
00ecfbd
@yaule yaule requested a review from a team as a code owner December 14, 2018 14:07
jsoriano
jsoriano approved these changes Dec 14, 2018
View reviewed changes
Copy link
Member

@jsoriano jsoriano left a comment

Choose a reason for hiding this comment

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

LGTM, waiting for jenkins

@jsoriano
Copy link
Member

jenkins, test this

1 similar comment
@sayden
Copy link
Contributor

sayden commented Dec 18, 2018

jenkins, test this

@jsoriano jsoriano merged commit ca4e034 into elastic:master Dec 19, 2018
jsoriano added a commit to jsoriano/beats that referenced this pull request Dec 19, 2018
@jsoriano
Add missing changelog entry for elastic#9430
a8c7b1c
@jsoriano jsoriano mentioned this pull request Dec 19, 2018
Merged
@jsoriano jsoriano added needs_backport PR is waiting to be backported to other branches. v6.7.0 labels Dec 19, 2018
@jsoriano jsoriano mentioned this pull request Dec 19, 2018
Merged
@jsoriano jsoriano added v6.6.0 and removed needs_backport PR is waiting to be backported to other branches. labels Dec 19, 2018
jsoriano pushed a commit to jsoriano/beats that referenced this pull request Dec 19, 2018
@yaule @jsoriano
Add more tcp statuses to socker_summary metricset (elastic#9430)
2841114 
(cherry picked from commit ca4e034)
jsoriano added a commit to jsoriano/beats that referenced this pull request Dec 19, 2018
@jsoriano
Add missing changelog entry for elastic#9430
c1b173d 
(cherry picked from commit a8c7b1c)
jsoriano added a commit that referenced this pull request Dec 19, 2018
@jsoriano
Add missing changelog entry for #9430 (#9666)
386a16e
jsoriano added a commit that referenced this pull request Dec 20, 2018
@jsoriano @yaule
Cherry-pick #9430 to 6.x: socket_summary add more tcp status (#9667)
deecb53 
(cherry picked from commit ca4e034)

Co-authored-by: yaule <lijixing@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jsoriano jsoriano jsoriano approved these changes

@sayden sayden Awaiting requested review from sayden

Assignees
No one assigned
Labels
Metricbeat Metricbeat module review Team:Integrations Label for the Integrations team v6.6.0 v6.7.0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@yaule @elasticmachine @elasticcla @sayden @jsoriano @ruflin