Skip to content

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

Merged
spacewander merged 20 commits into
apache:masterfrom
ccxhwmy:feature_promethues
Jul 19, 2022
Merged

feat(promethues): add ngx.shared.DICT status#7412
spacewander merged 20 commits into
apache:masterfrom
ccxhwmy:feature_promethues

Conversation

@ccxhwmy

@ccxhwmy ccxhwmy commented Jul 8, 2022

Copy link
Copy Markdown
Contributor

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
Comment thread apisix/plugins/prometheus/exporter.lua Outdated
Comment thread t/plugin/prometheus.t Outdated
Comment thread docs/zh/latest/plugins/prometheus.md Outdated
Comment thread apisix/plugins/prometheus/exporter.lua Outdated
Comment thread apisix/plugins/prometheus/exporter.lua Outdated
Comment thread apisix/plugins/prometheus/exporter.lua Outdated
Comment thread apisix/plugins/prometheus/exporter.lua Outdated
…onfiguration file.

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

@spacewander spacewander left a comment

Copy link
Copy Markdown
Member

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

ccxhwmy commented Jul 18, 2022

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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

ccxhwmy commented Jul 18, 2022

Copy link
Copy Markdown
Contributor Author

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
Copy Markdown
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

ccxhwmy commented Jul 18, 2022

Copy link
Copy Markdown
Contributor Author

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
Comment thread t/plugin/prometheus.t Outdated
@ccxhwmy

ccxhwmy commented Jul 19, 2022

Copy link
Copy Markdown
Contributor Author

Is the error a false report?

@tzssangglass

Copy link
Copy Markdown
Member

Is the error a false report?

rerun

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

Copy link
Copy Markdown
Contributor

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

@ccxhwmy

ccxhwmy commented Jul 19, 2022

Copy link
Copy Markdown
Contributor Author

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

Yes, I can.

@soulbird

Copy link
Copy Markdown
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

ccxhwmy commented Jul 24, 2022

Copy link
Copy Markdown
Contributor Author

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

5 participants