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] change server_status_path default setting for nginx module #14099

Merged
merged 8 commits into from
Nov 24, 2019

Conversation

goku321
Copy link
Contributor

@goku321 goku321 commented Oct 16, 2019

Changed default server_status_path to nginx_status

Fixes: #13806

@goku321 goku321 requested a review from a team as a code owner October 16, 2019 19:18
@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?

1 similar comment
@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?

@kaiyan-sheng
Copy link
Contributor

jenkins, test this please

@kaiyan-sheng kaiyan-sheng added the Team:Integrations Label for the Integrations team label Oct 16, 2019
@exekias exekias added the review label Oct 17, 2019
@exekias
Copy link
Contributor

exekias commented Oct 17, 2019

Thank you for contributing @goku321, could you please add a changelog entry? You will also need to run make update in the metricbeat folder

@goku321
Copy link
Contributor Author

goku321 commented Oct 17, 2019

Sure @exekias , will do 👍

@kaiyan-sheng
Copy link
Contributor

Hi @goku321 thanks for contributing. This PR will also need a rebase/merge from master in order to let CI pass.

@goku321
Copy link
Contributor Author

goku321 commented Oct 24, 2019

Hi @kaiyan-sheng I have synced this PR with master. Also, will add a changelog entry. Thanks for your patience

@kaiyan-sheng
Copy link
Contributor

jenkins, test this please

@goku321
Copy link
Contributor Author

goku321 commented Oct 26, 2019

I am struggling with make update (some python issue). Will fix it and update the pr

@kaiyan-sheng
Copy link
Contributor

I am struggling with make update (some python issue). Will fix it and update the pr

Maybe try mage update and/or mage fmt update under metricbeat folder 😬

@goku321
Copy link
Contributor Author

goku321 commented Oct 26, 2019

Thanks for the suggestion @kaiyan-sheng but still getting the same error (This backport is for Python 2.7 only.) Trying to debug it 😅

@kaiyan-sheng
Copy link
Contributor

Yeah unfortunately python2.7 is needed...

@kaiyan-sheng
Copy link
Contributor

Maybe this will help if you don't want to spend too much time on fixing python2.7:

kaiyansheng@KaiyanMacBookPro:~/go/src/github.com/elastic/beats/metricbeat (GH-13806)$ git diff
diff --git a/metricbeat/modules.d/nginx.yml.disabled b/metricbeat/modules.d/nginx.yml.disabled
index 786cc90ed..d9833114e 100644
--- a/metricbeat/modules.d/nginx.yml.disabled
+++ b/metricbeat/modules.d/nginx.yml.disabled
@@ -9,8 +9,8 @@
   # Nginx hosts
   hosts: ["http://127.0.0.1"]
 
-  # Path to server status. Default nginx_status
-  #server_status_path: "nginx_status"
+  # Path to server status. Default server-status
+  #server_status_path: "server-status"
 
   #username: "user"
   #password: "secret"

@kaiyan-sheng
Copy link
Contributor

Hi @goku321, could you rebase this PR by any chance and also I posted the change you need for fixing the breaking python ci test.

@goku321
Copy link
Contributor Author

goku321 commented Nov 21, 2019

@kaiyan-sheng sure, will do it by tomorrow

Copy link
Contributor

@kaiyan-sheng kaiyan-sheng 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 making this change!

@kaiyan-sheng kaiyan-sheng merged commit 1d65ec5 into elastic:master Nov 24, 2019
@kaiyan-sheng kaiyan-sheng added [zube]: In Review needs_backport PR is waiting to be backported to other branches. and removed [zube]: Done labels Nov 24, 2019
@goku321 goku321 deleted the GH-13806 branch November 24, 2019 18:23
@kaiyan-sheng kaiyan-sheng added v7.6.0 and removed needs_backport PR is waiting to be backported to other branches. labels Nov 24, 2019
kaiyan-sheng added a commit that referenced this pull request Nov 25, 2019
…ault setting for nginx module (#14730)

* [Metricbeat] change server_status_path default setting for nginx module (#14099)

* changed default server_status_path to nginx_status

(cherry picked from commit 1d65ec5)

* Update changelog
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
review Team:Integrations Label for the Integrations team v7.6.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[metricbeat] change server_status_path default setting for nginx module
5 participants