Baoyuantop commented on code in PR #12706: URL: https://github.com/apache/apisix/pull/12706#discussion_r2548606284
########## t/cli/test_prometheus_stream.sh: ########## @@ -60,7 +60,7 @@ sleep 1 # wait for sync out="$(curl http://127.0.0.1:9091/apisix/prometheus/metrics)" if ! echo "$out" | grep "apisix_stream_connection_total{route=\"1\"} 1" > /dev/null; then - echo "failed: prometheus can't work in stream subsystem" + echo "failed: prometheus can't work in both http & stream subsystem" Review Comment: Additional testing is needed; the existing tests cannot verify whether prometheus-cache is correctly generated when use_apisix_base=false. ########## apisix/cli/ngx_tpl.lua: ########## @@ -334,6 +334,9 @@ http { {% if enabled_plugins["prometheus"] and not enabled_stream_plugins["prometheus"] then %} lua_shared_dict prometheus-metrics {* http.lua_shared_dict["prometheus-metrics"] *}; + {% if not use_apisix_base then %} + lua_shared_dict prometheus-cache {* meta.lua_shared_dict["prometheus-cache"] *}; Review Comment: If the user has also enabled the Prometheus plugin in the stream subsystem (i.e., `enabled_stream_plugins["prometheus"]` is true), this if block will be skipped. If `use_apisix_base` is also false at this time (skipping the top-level global definition), then neither `prometheus-metrics` nor `prometheus-cache` will be defined. Is this the expected behavior? -- 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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
