Skip to content

Commit 317ccdd

Browse files
tkan145tkan145
committed
fix(proxy*): set upstream metrics when sending request via proxy
1 parent 82a098a commit 317ccdd

6 files changed

Lines changed: 289 additions & 8 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
2222
- Ensure the synchronization key is released correctly. [PR #1591](https://github.com/3scale/APIcast/pull/1590)
2323
- Use new cpu.requests formula from Kubernetes. [PR #1595](https://github.com/3scale/APIcast/pull/1595) [THREESCALE-15465](https://redhat.atlassian.net/browse/THREESCALE-15465)
2424
- Fix batcher policy fails silently when configured with string values instead of integers. [PR #1597](https://github.com/3scale/APIcast/pull/1597) [THREESCALE-15547](https://redhat.atlassian.net/browse/THREESCALE-15547)
25+
- Set upstream metrics when sending request via proxy. [PR #1598](https://github.com/3scale/APIcast/pull/1598) [THREESCALE-10571](https://redhat.atlassian.net/browse/THREESCALE-15071)
2526

2627
### Added
2728
- Update APIcast schema manifest [PR #1550](https://github.com/3scale/APIcast/pull/1550)

‎gateway/src/apicast/http_proxy.lua‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -56,6 +56,7 @@ local function forward_https_request(proxy_uri, uri, proxy_opts)
5656
local content_type = ngx_req_get_headers()["Content-Type"]
5757
local content_type_is_urlencoded = content_type and content_type:lower() == "application/x-www-form-urlencoded"
5858
local raw = false
59+
local request_start = ngx.now()
5960

6061
if http_methods_with_body[req_method] then
6162

@@ -165,6 +166,9 @@ local function forward_https_request(proxy_uri, uri, proxy_opts)
165166
res, err = httpc:request(request)
166167

167168
if res then
169+
ngx.ctx.proxy_upstream_status = res.status
170+
ngx.ctx.proxy_upstream_response_time = ngx.now() - request_start
171+
168172
if opts.request_unbuffered and raw then
169173
err = send_response(sock, res, DEFAULT_CHUNKSIZE)
170174
if err then

‎gateway/src/apicast/policy/nginx_metrics/nginx_metrics.lua‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -155,7 +155,9 @@ function _M.log(_, context)
155155
if context.service and extended_metrics then
156156
service = context.service
157157
end
158-
upstream_metrics.report(ngx.var.upstream_status, ngx.var.upstream_response_time, service)
158+
local upstream_status = ngx.var.upstream_status or ngx.ctx.proxy_upstream_status
159+
local upstream_response_time = ngx.var.upstream_response_time or ngx.ctx.proxy_upstream_response_time
160+
upstream_metrics.report(upstream_status, upstream_response_time, service)
159161
report_req_response_time(service)
160162
metrics_updater.inc(apicast_status_metric, status_map[ngx.status])
161163
end

‎spec/http_proxy_spec.lua‎

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -9,11 +9,11 @@ describe('http_proxy', function()
99
stub(ngx.req, 'get_method', function() return 'GET' end)
1010
end
1111

12-
local function stub_resty_http_proxy()
12+
local function stub_resty_http_proxy(response)
1313
local httpc = {
1414
}
1515

16-
local response = {}
16+
response = response or {}
1717
stub(httpc, 'request', function() return response end)
1818
stub(httpc, 'proxy_response')
1919
stub(httpc, 'set_keepalive')
@@ -24,11 +24,6 @@ describe('http_proxy', function()
2424
stub(http_writer, 'proxy_response')
2525
end
2626

27-
before_each(function()
28-
stub_ngx_request()
29-
stub_resty_http_proxy()
30-
end)
31-
3227
describe('on https backend', function()
3328
local upstream = {
3429
uri = {
@@ -41,14 +36,25 @@ describe('http_proxy', function()
4136
}
4237

4338
before_each(function()
39+
stub_ngx_request()
4440
stub(upstream, 'rewrite_request')
4541
end)
4642

4743
it('terminates phase', function()
44+
stub_resty_http_proxy()
4845
local http_proxy = require('apicast.http_proxy')
4946
http_proxy.request(upstream, proxy_uri)
5047
assert.spy(ngx.exit).was_called_with(ngx.OK)
5148
end)
49+
50+
it('stores upstream status and response time in ngx.ctx', function()
51+
stub_resty_http_proxy({ status = 200 })
52+
ngx.ctx = {}
53+
local http_proxy = require('apicast.http_proxy')
54+
http_proxy.request(upstream, proxy_uri)
55+
assert.equal(200, ngx.ctx.proxy_upstream_status)
56+
assert.is_number(ngx.ctx.proxy_upstream_response_time)
57+
end)
5258
end)
5359
end)
5460
end)

‎spec/policy/nginx_metrics/nginx_metrics_spec.lua‎

Lines changed: 44 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,44 @@
1+
local nginx_metrics = require('apicast.policy.nginx_metrics.nginx_metrics')
2+
3+
describe('nginx_metrics policy', function()
4+
describe('.log', function()
5+
local upstream_metrics
6+
7+
before_each(function()
8+
upstream_metrics = require('apicast.metrics.upstream')
9+
stub(upstream_metrics, 'report')
10+
11+
local metrics_updater = require('apicast.metrics.updater')
12+
stub(metrics_updater, 'inc')
13+
14+
ngx.status = 200
15+
end)
16+
17+
it('uses ngx.var upstream values when present', function()
18+
ngx.var = { upstream_status = '200', upstream_response_time = '0.5' }
19+
ngx.ctx = { proxy_upstream_status = 502, proxy_upstream_response_time = 1.2 }
20+
21+
nginx_metrics.log(nil, {})
22+
23+
assert.stub(upstream_metrics.report).was_called_with('200', '0.5', { id = "", system_name = "" })
24+
end)
25+
26+
it('falls back to ngx.ctx values set by the http_proxy module when ngx.var ones are missing', function()
27+
ngx.var = { upstream_status = nil, upstream_response_time = nil }
28+
ngx.ctx = { proxy_upstream_status = 200, proxy_upstream_response_time = 0.75 }
29+
30+
nginx_metrics.log(nil, {})
31+
32+
assert.stub(upstream_metrics.report).was_called_with(200, 0.75, { id = "", system_name = "" })
33+
end)
34+
35+
it('reports nil when neither ngx.var nor ngx.ctx have upstream values', function()
36+
ngx.var = { upstream_status = nil, upstream_response_time = nil }
37+
ngx.ctx = {}
38+
39+
nginx_metrics.log(nil, {})
40+
41+
assert.stub(upstream_metrics.report).was_called_with(nil, nil, { id = "", system_name = "" })
42+
end)
43+
end)
44+
end)

‎t/prometheus-metrics.t‎

Lines changed: 224 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@ use lib 't';
22
use Test::APIcast::Blackbox 'no_plan';
33

44
require("policies.pl");
5+
require("http_proxy.pl");
56
# The output varies between requests, so run only once
67
repeat_each(1);
78

@@ -492,3 +493,226 @@ qr/apicast_status\{status="404"\} 1/,
492493
]]
493494
--- no_error_log
494495
[error]
496+
497+
498+
=== TEST 9: HTTPS proxy reports upstream metrics via ngx.ctx fallback
499+
When the request goes through the HTTPS proxy path, ngx.var.upstream_status is
500+
not set by nginx. The http_proxy module populates ngx.ctx.proxy_upstream_status
501+
and ngx.ctx.proxy_upstream_response_time, and the nginx_metrics policy falls
502+
back to those values. Verify the upstream_status metric is reported.
503+
--- env eval
504+
(
505+
"https_proxy" => $ENV{TEST_NGINX_HTTPS_PROXY},
506+
'BACKEND_ENDPOINT_OVERRIDE' => "http://test_backend.lvh.me:$ENV{TEST_NGINX_SERVER_PORT}"
507+
)
508+
--- configuration random_port env
509+
{
510+
"services": [
511+
{
512+
"id": 42,
513+
"backend_version": 1,
514+
"backend_authentication_type": "service_token",
515+
"backend_authentication_value": "token-value",
516+
"proxy": {
517+
"hosts": ["one"],
518+
"api_backend": "https://test-upstream.lvh.me:$TEST_NGINX_RANDOM_PORT",
519+
"proxy_rules": [
520+
{ "pattern": "/", "http_method": "GET", "metric_system_name": "hits", "delta": 1 }
521+
]
522+
}
523+
}
524+
]
525+
}
526+
--- backend
527+
server_name test_backend.lvh.me;
528+
location /transactions/authrep.xml {
529+
content_by_lua_block {
530+
ngx.exit(ngx.OK)
531+
}
532+
}
533+
--- upstream env
534+
server_name test-upstream.lvh.me;
535+
listen $TEST_NGINX_RANDOM_PORT ssl;
536+
ssl_certificate $TEST_NGINX_SERVER_ROOT/html/server.crt;
537+
ssl_certificate_key $TEST_NGINX_SERVER_ROOT/html/server.key;
538+
location / {
539+
content_by_lua_block {
540+
ngx.exit(200);
541+
}
542+
}
543+
--- request eval
544+
["GET /?user_key=value", "GET /metrics/"]
545+
--- more_headers eval
546+
["Host: one", "Host: metrics"]
547+
--- error_code eval
548+
[ 200, 200 ]
549+
--- expected_response_body_like_multiple eval
550+
[
551+
"",
552+
[
553+
qr/upstream_response_time_seconds(.|\n)/,
554+
qr/upstream_response_time_seconds_bucket\{service_id="",service_system_name="",le=".*"\} 1/,
555+
qr/upstream_status\{status="200",service_id="",service_system_name=""\} 1/
556+
]]
557+
--- no_error_log
558+
[error]
559+
--- user_files fixture=tls.pl eval
560+
561+
562+
=== TEST 10: Report upstream metrics when using http_proxy policy
563+
--- env eval
564+
('BACKEND_ENDPOINT_OVERRIDE' => "http://test_backend.lvh.me:$ENV{TEST_NGINX_SERVER_PORT}")
565+
--- configuration random_port env
566+
{
567+
"services": [
568+
{
569+
"id": 42,
570+
"backend_version": 1,
571+
"backend_authentication_type": "service_token",
572+
"backend_authentication_value": "token-value",
573+
"proxy": {
574+
"hosts": ["one"],
575+
"api_backend": "https://test-upstream.lvh.me:$TEST_NGINX_RANDOM_PORT",
576+
"proxy_rules": [
577+
{ "pattern": "/", "http_method": "GET", "metric_system_name": "hits", "delta": 1 }
578+
],
579+
"policy_chain": [
580+
{
581+
"name": "apicast.policy.apicast"
582+
},
583+
{
584+
"name": "apicast.policy.http_proxy",
585+
"configuration": {
586+
"https_proxy": "$TEST_NGINX_HTTPS_PROXY"
587+
}
588+
}
589+
]
590+
}
591+
}
592+
]
593+
}
594+
--- backend
595+
server_name test_backend.lvh.me;
596+
location /transactions/authrep.xml {
597+
content_by_lua_block {
598+
ngx.exit(ngx.OK)
599+
}
600+
}
601+
--- upstream env
602+
server_name test-upstream.lvh.me;
603+
listen $TEST_NGINX_RANDOM_PORT ssl;
604+
ssl_certificate $TEST_NGINX_SERVER_ROOT/html/server.crt;
605+
ssl_certificate_key $TEST_NGINX_SERVER_ROOT/html/server.key;
606+
location / {
607+
content_by_lua_block {
608+
ngx.exit(200);
609+
}
610+
}
611+
--- request eval
612+
["GET /?user_key=value", "GET /metrics/"]
613+
--- more_headers eval
614+
["Host: one", "Host: metrics"]
615+
--- error_code eval
616+
[ 200, 200 ]
617+
--- expected_response_body_like_multiple eval
618+
[
619+
"",
620+
[
621+
qr/upstream_response_time_seconds(.|\n)/,
622+
qr/upstream_response_time_seconds_bucket\{service_id="",service_system_name="",le=".*"\} 1/,
623+
qr/upstream_status\{status="200",service_id="",service_system_name=""\} 1/
624+
]]
625+
--- no_error_log
626+
[error]
627+
--- user_files fixture=tls.pl eval
628+
629+
630+
=== TEST 11: Report upstream metrics when using camel policy
631+
--- init eval
632+
$Test::Nginx::Util::PROXY_SSL_PORT = Test::APIcast::get_random_port();
633+
$Test::Nginx::Util::ENDPOINT_SSL_PORT = Test::APIcast::get_random_port();
634+
--- configuration random_port env eval
635+
<<EOF
636+
{
637+
"services": [
638+
{
639+
"id": 42,
640+
"backend_version": 1,
641+
"backend_authentication_type": "service_token",
642+
"backend_authentication_value": "token-value",
643+
"proxy": {
644+
"hosts": ["one"],
645+
"secret_token": "token",
646+
"api_backend": "https://localhost:$Test::Nginx::Util::ENDPOINT_SSL_PORT",
647+
"proxy_rules": [
648+
{ "pattern": "/", "http_method": "GET", "metric_system_name": "hits", "delta": 1 }
649+
],
650+
"policy_chain": [
651+
{
652+
"name": "apicast.policy.apicast"
653+
},
654+
{
655+
"name": "apicast.policy.camel",
656+
"configuration": {
657+
"https_proxy": "http://127.0.0.1:$Test::Nginx::Util::PROXY_SSL_PORT"
658+
}
659+
}
660+
]
661+
}
662+
}
663+
]
664+
}
665+
EOF
666+
--- backend
667+
location /transactions/authrep.xml {
668+
content_by_lua_block {
669+
ngx.exit(ngx.OK)
670+
}
671+
}
672+
--- upstream eval
673+
<<EOF
674+
# Endpoint config
675+
listen $Test::Nginx::Util::ENDPOINT_SSL_PORT ssl;
676+
677+
ssl_certificate $Test::Nginx::Util::ServRoot/html/server.crt;
678+
ssl_certificate_key $Test::Nginx::Util::ServRoot/html/server.key;
679+
680+
server_name _ default_server;
681+
682+
location / {
683+
content_by_lua_block {
684+
ngx.exit(200)
685+
}
686+
}
687+
}
688+
server {
689+
# Proxy config
690+
listen $Test::Nginx::Util::PROXY_SSL_PORT ssl;
691+
692+
ssl_certificate $Test::Nginx::Util::ServRoot/html/server.crt;
693+
ssl_certificate_key $Test::Nginx::Util::ServRoot/html/server.key;
694+
695+
server_name _ default_server;
696+
697+
location ~ /.* {
698+
proxy_http_version 1.1;
699+
proxy_pass https://\$http_host;
700+
}
701+
EOF
702+
--- request eval
703+
["GET /?user_key=value", "GET /metrics/"]
704+
--- more_headers eval
705+
["Host: one", "Host: metrics"]
706+
--- error_code eval
707+
[ 200, 200 ]
708+
--- expected_response_body_like_multiple eval
709+
[
710+
"",
711+
[
712+
qr/upstream_response_time_seconds(.|\n)/,
713+
qr/upstream_response_time_seconds_bucket\{service_id="",service_system_name="",le=".*"\} 1/,
714+
qr/upstream_status\{status="200",service_id="",service_system_name=""\} 1/
715+
]]
716+
--- no_error_log
717+
[error]
718+
--- user_files fixture=tls.pl eval

0 commit comments

Comments
 (0)