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

feat(promethues): add ngx.shared.DICT status #7412

Merged
merged 20 commits into from
Jul 19, 2022

Conversation

ccxhwmy
Copy link
Contributor

@ccxhwmy ccxhwmy commented Jul 8, 2022

Description

Fixes #5837
Use the request header to carry the name of ngx.shared.DICT that needs to be fetched, use ngx.shared.DICT.capacity and ngx.shared.DICT.free_space to get the current status of ngx.shared.DICT, and use gauge to store the status

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@ccxhwmy ccxhwmy changed the title Feature promethues: add ngx.shared.DICT status Feature(promethues): add ngx.shared.DICT status Jul 8, 2022
@ccxhwmy ccxhwmy changed the title Feature(promethues): add ngx.shared.DICT status feat(promethues): add ngx.shared.DICT status Jul 8, 2022
@ccxhwmy ccxhwmy marked this pull request as ready for review July 8, 2022 08:37
apisix/plugins/prometheus/exporter.lua Outdated Show resolved Hide resolved
t/plugin/prometheus.t Outdated Show resolved Hide resolved
docs/zh/latest/plugins/prometheus.md Outdated Show resolved Hide resolved
apisix/plugins/prometheus/exporter.lua Outdated Show resolved Hide resolved
apisix/plugins/prometheus/exporter.lua Outdated Show resolved Hide resolved
apisix/plugins/prometheus/exporter.lua Outdated Show resolved Hide resolved
apisix/plugins/prometheus/exporter.lua Outdated Show resolved Hide resolved
…onfiguration file.

2.distinguish prometheus metric of ngx.shared.DICT with name, instead of label.
apisix/plugins/prometheus/exporter.lua Outdated Show resolved Hide resolved
apisix/plugins/prometheus/exporter.lua Outdated Show resolved Hide resolved
apisix/plugins/prometheus/exporter.lua Outdated Show resolved Hide resolved
@ccxhwmy ccxhwmy requested a review from soulbird July 14, 2022 05:35
tzssangglass
tzssangglass previously approved these changes Jul 14, 2022
t/plugin/prometheus.t Outdated Show resolved Hide resolved
apisix/plugins/prometheus/exporter.lua Outdated Show resolved Hide resolved
docs/en/latest/plugins/prometheus.md Show resolved Hide resolved
t/plugin/prometheus.t Outdated Show resolved Hide resolved
Copy link
Member

@spacewander spacewander left a comment

Choose a reason for hiding this comment

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

Please make the CI pass, thanks!

@ccxhwmy
Copy link
Contributor Author

ccxhwmy commented Jul 18, 2022

Please make the CI pass, thanks!

I don't know why this ci error is reported here, Would you give me some suggestion.

@spacewander
Copy link
Member

Please make the CI pass, thanks!

I don't know why this ci error is reported here, Would you give me some suggestion.

The error message is:

Bailout called.  Further testing stopped:  t/plugin/prometheus.t TEST 42: fetch the prometheus multiple shared dict data - You asked for many requests, the expected results should be arrays as well.
FAILED--Further testing stopped: t/plugin/prometheus.t TEST 42: fetch the prometheus multiple shared dict data - You asked for many requests, the expected results should be arrays as well.

You can look up it.

@ccxhwmy
Copy link
Contributor Author

ccxhwmy commented Jul 18, 2022

Please make the CI pass, thanks!

I don't know why this ci error is reported here, Would you give me some suggestion.

The error message is:

Bailout called.  Further testing stopped:  t/plugin/prometheus.t TEST 42: fetch the prometheus multiple shared dict data - You asked for many requests, the expected results should be arrays as well.
FAILED--Further testing stopped: t/plugin/prometheus.t TEST 42: fetch the prometheus multiple shared dict data - You asked for many requests, the expected results should be arrays as well.

You can look up it.

The error message report that You asked for many requests, the expected results should be arrays as well., but the test case is:

=== TEST 42: fetch the prometheus multiple shared dict data
--- request eval
["GET /apisix/prometheus/metrics",
"GET /apisix/prometheus/metrics",
"GET /apisix/prometheus/metrics",
"GET /apisix/prometheus/metrics",
"GET /apisix/prometheus/metrics",
"GET /apisix/prometheus/metrics",
"GET /apisix/prometheus/metrics",
"GET /apisix/prometheus/metrics"]
--- response_body_like eval
[qr/.*apisix_shared_dict_capacity_bytes{name="internal-status"} \d+(?:.|\n)*
apisix_shared_dict_free_space_bytes{name="internal-status"} \d+.*/,
qr/.*apisix_shared_dict_capacity_bytes{name="upstream-healthcheck"} \d+(?:.|\n)*
apisix_shared_dict_free_space_bytes{name="upstream-healthcheck"} \d+.*/,
qr/.*apisix_shared_dict_capacity_bytes{name="worker-events"} \d+(?:.|\n)*
apisix_shared_dict_free_space_bytes{name="worker-events"} \d+.*/,
qr/.*apisix_shared_dict_capacity_bytes{name="lrucache-lock"} \d+(?:.|\n)*
apisix_shared_dict_free_space_bytes{name="lrucache-lock"} \d+.*/,
qr/.*apisix_shared_dict_capacity_bytes{name="balancer-ewma"} \d+(?:.|\n)*
apisix_shared_dict_free_space_bytes{name="balancer-ewma"} \d+.*/,
qr/.*apisix_shared_dict_capacity_bytes{name="balancer-ewma-locks"} \d+(?:.|\n)*
apisix_shared_dict_free_space_bytes{name="balancer-ewma-locks"} \d+.*/,
qr/.*apisix_shared_dict_capacity_bytes{name="balancer-ewma-last-touched-at"} \d+(?:.|\n)*
apisix_shared_dict_free_space_bytes{name="balancer-ewma-last-touched-at"} \d+.*/,
qr/.*apisix_shared_dict_capacity_bytes{name="etcd-cluster-health-check"} \d+(?:.|\n)*
apisix_shared_dict_free_space_bytes{name="etcd-cluster-health-check"} \d+.*/]

which has a array result. and commit dbf7e7e have a similar test case:

=== TEST 44: fetch the prometheus multiple shared dict data
, so this ci error confused me.

@tzssangglass
Copy link
Member

The error message report that You asked for many requests, the expected results should be arrays as well., but the test case is:

It does seem strange, so if you don't know how to modify it, consider a different type of validation.

define lua_shared_dict in test case and check it.
@ccxhwmy
Copy link
Contributor Author

ccxhwmy commented Jul 18, 2022

The error message report that You asked for many requests, the expected results should be arrays as well., but the test case is:

It does seem strange, so if you don't know how to modify it, consider a different type of validation.

You are right! I found a new type of validation, it maybe more suitable.

spacewander
spacewander previously approved these changes Jul 18, 2022
t/plugin/prometheus.t Outdated Show resolved Hide resolved
@ccxhwmy
Copy link
Contributor Author

ccxhwmy commented Jul 19, 2022

Is the error a false report?

@tzssangglass
Copy link
Member

Is the error a false report?

rerun

@spacewander spacewander merged commit 4620cea into apache:master Jul 19, 2022
@soulbird
Copy link
Contributor

@ccxhwmy could you update the apisix-grafana-dashboard meta for shared dict?

@ccxhwmy
Copy link
Contributor Author

ccxhwmy commented Jul 19, 2022

@ccxhwmy could you update the apisix-grafana-dashboard meta for shared dict?

Yes, I can.

@soulbird
Copy link
Contributor

@ccxhwmy could you update the apisix-grafana-dashboard meta for shared dict?

Yes, I can.

Thanks a lot, let's do it in another PR.

@ccxhwmy ccxhwmy deleted the feature_promethues branch July 19, 2022 23:32
@ccxhwmy
Copy link
Contributor Author

ccxhwmy commented Jul 24, 2022

I have update the apisix-grafana-dashboard meta for shared dict in the (PR)[https://github.com//pull/7529].

Liu-Junlin pushed a commit to Liu-Junlin/apisix that referenced this pull request Nov 4, 2022
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.

request: Expect monitoring metrics to add monitoring of nginx shared memory usage
4 participants