You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@apisix.apache.org by GitBox <gi...@apache.org> on 2022/01/07 10:45:10 UTC
[GitHub] [apisix] zhendongcmss opened a new pull request #6047: fix: the prometheus lables are inconsistent when using the same batch-processor instance on multi plugins
zhendongcmss opened a new pull request #6047:
URL: https://github.com/apache/apisix/pull/6047
fix https://github.com/apache/apisix/issues/6011
### What this PR does / why we need it:
<!--- Why is this change required? What problem does it solve? -->
<!--- If it fixes an open issue, please link to the issue here. -->
### Pre-submission checklist:
<!--
Please follow the PR manners:
1. Use Draft if the PR is not ready to be reviewed
2. Test is required for the feat/fix PR, unless you have a good reason
3. Doc is required for the feat PR
4. Use a new commit to resolve review instead of `push -f`
5. If you need to resolve merge conflicts after the PR is reviewed, please merge master but do not rebase
6. Use "request review" to notify the reviewer once you have resolved the review
7. Only reviewer can click "Resolve conversation" to mark the reviewer's review resolved
-->
* [ ] Did you explain what problem does this PR solve? Or what new features have been added?
* [ ] Have you added corresponding test cases?
* [ ] Have you modified the corresponding document?
* [ ] Is this PR backward compatible? **If it is not backward compatible, please discuss on the [mailing list](https://github.com/apache/apisix/tree/master#community) first**
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zhendongcmss commented on pull request #6047: fix: the prometheus lables are inconsistent when using the same batch-processor instance on multi plugins
Posted by GitBox <gi...@apache.org>.
zhendongcmss commented on pull request #6047:
URL: https://github.com/apache/apisix/pull/6047#issuecomment-1008502557
> What about providing a default value in
>
> https://github.com/apache/apisix/blob/ddb9cd28bf33799652d252df0546aa0832933bcf/apisix/utils/batch-processor.lua#L138
>
> ?
I suggest that don't change the origin behavior. Sometimes we need to judge whether matching route accroding to toute_id.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] spacewander commented on pull request #6047: fix: the prometheus lables are inconsistent when using the same batch-processor instance on multi plugins
Posted by GitBox <gi...@apache.org>.
spacewander commented on pull request #6047:
URL: https://github.com/apache/apisix/pull/6047#issuecomment-1008495244
I can reproduce it in both 2.10 and master branch. What about submitting it to master first?
I reproduced it with `t/plugin/prometheus3.t`:
```
#
# Licensed to the Apache Software Foundation (ASF) under one or more
# contributor license agreements. See the NOTICE file distributed with
# this work for additional information regarding copyright ownership.
# The ASF licenses this file to You under the Apache License, Version 2.0
# (the "License"); you may not use this file except in compliance with
# the License. You may obtain a copy of the License at
#
# http://www.apache.org/licenses/LICENSE-2.0
#
# Unless required by applicable law or agreed to in writing, software
# distributed under the License is distributed on an "AS IS" BASIS,
# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
# See the License for the specific language governing permissions and
# limitations under the License.
#
BEGIN {
if ($ENV{TEST_NGINX_CHECK_LEAK}) {
$SkipReason = "unavailable for the hup tests";
} else {
$ENV{TEST_NGINX_USE_HUP} = 1;
undef $ENV{TEST_NGINX_USE_STAP};
}
}
use t::APISIX 'no_plan';
repeat_each(1);
no_long_string();
no_shuffle();
no_root_location();
add_block_preprocessor(sub {
my ($block) = @_;
if (!$block->request) {
$block->set_value("request", "GET /t");
}
if ((!defined $block->error_log) && (!defined $block->no_error_log)) {
$block->set_value("no_error_log", "[error]");
}
});
run_tests;
__DATA__
=== TEST 1: use original etcd modified index
--- config
location /t {
content_by_lua_block {
local t = require("lib.test_admin").test
local code, body = t('/apisix/admin/plugin_configs/1',
ngx.HTTP_PUT,
[[{
"plugins": {
"prometheus":{}
}
}]]
)
if code >= 300 then
ngx.status = code
ngx.say(body)
return
end
local code, body = t('/apisix/admin/routes/1',
ngx.HTTP_PUT,
[[{
"plugin_config_id": 1,
"upstream": {
"nodes": {
"127.0.0.1:1980": 1
},
"type": "roundrobin"
},
"uri": "/hello"
}]]
)
if code >= 300 then
ngx.status = code
end
ngx.say(body)
}
}
--- response_body
passed
=== TEST 2: hit
--- pipelined_requests eval
["GET /hello", "GET /apisix/prometheus/metrics"]
--- error_code eval
[200, 200]
=== TEST 3: apisix_batch_process_entries, mess with global rules
--- config
location /t {
content_by_lua_block {
local t = require("lib.test_admin").test
local code, body = t('/apisix/admin/routes/1',
ngx.HTTP_PUT,
[[{
"plugins": {
"prometheus": {}
},
"upstream": {
"nodes": {
"127.0.0.1:1980": 1
},
"type": "roundrobin"
},
"uri": "/batch-process-metrics-aa"
}]]
)
if code >= 300 then
ngx.status = code
ngx.say(body)
return
end
local code, body = t('/apisix/admin/plugin_metadata/error-log-logger',
ngx.HTTP_PUT,
[[{
"tcp": {
"host": "127.0.0.1",
"port": 1999
},
"max_retry_count": 1000,
"level": "NOTICE"
}]]
)
if code >= 300 then
ngx.status = code
ngx.say(body)
return
end
local code, body = t('/apisix/admin/global_rules/1',
ngx.HTTP_PUT,
[[{
"plugins": {
"http-logger": {
"uri": "http://127.0.0.1:1979"
}
}
}]]
)
if code >= 300 then
ngx.status = code
end
ngx.say(body)
}
}
--- response_body
passed
=== TEST 4: check metrics
--- yaml_config
plugins:
- error-log-logger
- prometheus
- http-logger
--- request
GET /t
--- config
location /t {
content_by_lua_block {
local http = require "resty.http"
local httpc = http.new()
local uri = "http://127.0.0.1:" .. ngx.var.server_port
.. "/batch-process-metrics-aa"
local res, err = httpc:request_uri(uri, {method = "GET"})
if not res then
ngx.say(err)
return
end
ngx.sleep(2)
local uri = "http://127.0.0.1:" .. ngx.var.server_port
.. "/apisix/prometheus/metrics"
local res, err = httpc:request_uri(uri, {method = "GET"})
if not res then
ngx.say(err)
return
end
ngx.say(res.body)
}
}
--- response_body_like eval
qr/apisix_batch_process_entries\{name="http logger",route_id="1",server_addr="127.0.0.1"\} \d+/
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zhendongcmss commented on pull request #6047: fix: the prometheus lables are inconsistent when using the same batch-processor instance on multi plugins
Posted by GitBox <gi...@apache.org>.
zhendongcmss commented on pull request #6047:
URL: https://github.com/apache/apisix/pull/6047#issuecomment-1008501443
> I can reproduce it in both 2.10 and master branch. What about submitting it to master first?
Sure
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zhendongcmss edited a comment on pull request #6047: fix: the prometheus lables are inconsistent when using the same batch-processor instance on multi plugins
Posted by GitBox <gi...@apache.org>.
zhendongcmss edited a comment on pull request #6047:
URL: https://github.com/apache/apisix/pull/6047#issuecomment-1008508237
I will raise a new pull request to master and close this pull request.
https://github.com/apache/apisix/pull/6055
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zhendongcmss closed pull request #6047: fix: the prometheus lables are inconsistent when using the same batch-processor instance on multi plugins
Posted by GitBox <gi...@apache.org>.
zhendongcmss closed pull request #6047:
URL: https://github.com/apache/apisix/pull/6047
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [apisix] zhendongcmss commented on pull request #6047: fix: the prometheus lables are inconsistent when using the same batch-processor instance on multi plugins
Posted by GitBox <gi...@apache.org>.
zhendongcmss commented on pull request #6047:
URL: https://github.com/apache/apisix/pull/6047#issuecomment-1008508237
I will raise a new pull request to master and close this pll request.
https://github.com/apache/apisix/pull/6055
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: notifications-unsubscribe@apisix.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org