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

Add additional php-fpm pool status kpis for Metricbeat module #5287

Merged
merged 9 commits into from
Oct 3, 2017
Merged

Add additional php-fpm pool status kpis for Metricbeat module #5287

merged 9 commits into from
Oct 3, 2017

Conversation

pgilad
Copy link
Contributor

@pgilad pgilad commented Oct 2, 2017

No description provided.

@elasticmachine
Copy link
Collaborator

Can one of the admins verify this patch?

@pgilad
Copy link
Contributor Author

pgilad commented Oct 2, 2017

@exekias This is the new PR to the master branch

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 contributing @pgilad! 🎉 I left a minor comment about date fields.

Could you please add a CHANGELOG.asciidoc entry?

description: >
Number of seconds since FPM has started.
- name: start_time
type: long
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this could be date https://www.elastic.co/guide/en/elasticsearch/reference/current/date.html, you may need to convert the value before pushing it to Elasticsearch

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, it needs a epoch_second format, let me see if I can find how to format that

@exekias
Copy link
Contributor

exekias commented Oct 2, 2017

jenkins test it please

@pgilad
Copy link
Contributor Author

pgilad commented Oct 2, 2017

@exekias Done

@ruflin
Copy link
Contributor

ruflin commented Oct 2, 2017

jenkins, test it

@ruflin
Copy link
Contributor

ruflin commented Oct 2, 2017

@pgilad I think you need to run make fmt to make CI happy.

@pgilad
Copy link
Contributor Author

pgilad commented Oct 2, 2017

@ruflin Thanks, done!

@exekias
Copy link
Contributor

exekias commented Oct 2, 2017

jenkins test it, please

@exekias
Copy link
Contributor

exekias commented Oct 2, 2017

Sorry @pgilad you will also need to update make update to reflect fields.yml updates in docs.

@pgilad
Copy link
Contributor Author

pgilad commented Oct 2, 2017

jenkins test it, please

@pgilad
Copy link
Contributor Author

pgilad commented Oct 3, 2017

@exekias @ruflin anything else to add?

@ruflin ruflin merged commit 8d79c8f into elastic:master Oct 3, 2017
@ruflin
Copy link
Contributor

ruflin commented Oct 3, 2017

@pgilad Merged. Thanks a lot for the contribution.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants