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

Support Nginx monitoring. #11558

Merged
merged 8 commits into from
Nov 17, 2023
Merged

Conversation

weixiang1862
Copy link
Member

Nginx monitoring

  • If this is non-trivial feature, paste the links/URLs to the design doc.

  • Update the documentation to include this new feature.

  • Tests(including UT, IT, E2E) are added to verify the new feature.

  • If it's UI related, attach the screenshots below.

  • If this pull request closes/resolves/fixes an existing issue, replace the issue number. Closes [Feature] Support Nginx monitoring. #11534.

  • Update the CHANGES log.

50cdbaf31624c4c6442e35c2a2eebe8
4c01ad9c80ca6e6bae35b19bef7cb65
a24f6b86a30843bea2d93d932fa8399
0f893b60d8b99f9537db61c7679c9c9

@wu-sheng wu-sheng added backend OAP backend related. feature New feature labels Nov 16, 2023
@wu-sheng wu-sheng added this to the 9.7.0 milestone Nov 16, 2023
@wu-sheng
Copy link
Member

From the screenshot, the log level is absent.

@weixiang1862
Copy link
Member Author

From the screenshot, the log level is absent.

error.log has log level, accesslog doesn't. Should I set the default log level for accesslog?

@wu-sheng
Copy link
Member

error.log has log level, accesslog doesn't. Should I set the default log level for accesslog?

Which logs are you gathering from accesslog? The log output should include the level, isn't it?

@weixiang1862
Copy link
Member Author

error.log has log level, accesslog doesn't. Should I set the default log level for accesslog?

Which logs are you gathering from accesslog? The log output should include the level, isn't it?

nginx output accesslog and errorlog in two files, I collect both of them. error log has log level but access log doesn't.

@wu-sheng
Copy link
Member

log_format myformat '$remote_addr - $remote_user [$time_local] '
                    '"$request" $status $body_bytes_sent '
                    '"$http_referer" "$http_user_agent" '
                    '"$http_x_forwarded_for"';

access_log /path/to/access.log myformat;

I think we should extract $status from a defined format. And tag the error level and http.status_code. Also, add http.status_code into searchableLogsTags.

@weixiang1862
Copy link
Member Author

log_format myformat '$remote_addr - $remote_user [$time_local] '
                    '"$request" $status $body_bytes_sent '
                    '"$http_referer" "$http_user_agent" '
                    '"$http_x_forwarded_for"';

access_log /path/to/access.log myformat;

I think we should extract $status from a defined format. And tag the error level and http.status_code. Also, add http.status_code into searchableLogsTags.

ok, i will extract status and level tag.

@weixiang1862
Copy link
Member Author

accesslog http.status_code & errorlog level is searchable:

ae8090779732ffa4a48bdff2fee92b4
5aa9fe469fa5535fe665772d6b00acc

@@ -0,0 +1,141 @@
# Nginx monitoring
## Nginx performance from nginx-lua-prometheus
The [nginx-lua-prometheus](https://github.com/openresty/lua-nginx-module) is a lua library that can be used with Nginx to collect metrics
Copy link
Member

Choose a reason for hiding this comment

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

The link is wrong?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, thanks. I will fix this.

Copy link
Member

@wankai123 wankai123 left a comment

Choose a reason for hiding this comment

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

LGTM

@wu-sheng
Copy link
Member

@weixiang1862 Generally, this should be good. I will wait for others' review. And would you be able to add Nginx monitoring

@wu-sheng
Copy link
Member

And I believe another blog to introduce your new thing would be cool. This should be more powerful than you introduced in the last one. WDYT?

@weixiang1862
Copy link
Member Author

And I believe another blog to introduce your new thing would be cool. This should be more powerful than you introduced in the last one. WDYT?

Yes, I will add a new blog to introduce it and add it to showcase, I think they will be ok in the end of this month.

@wu-sheng
Copy link
Member

And I believe another blog to introduce your new thing would be cool. This should be more powerful than you introduced in the last one. WDYT?

Yes, I will add a new blog to introduce it and add it to showcase, I think they will be ok in the end of this month.

The time is totally flexible, and up to your reality.

@wu-sheng wu-sheng merged commit 8bff0aa into apache:master Nov 17, 2023
167 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backend OAP backend related. feature New feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Feature] Support Nginx monitoring.
4 participants