From 24dd995514aa831570bf3449422f38699a3ae32c Mon Sep 17 00:00:00 2001 From: imccarten1 Date: Thu, 27 Aug 2020 17:44:27 -0700 Subject: [PATCH] Add appengine's nginx latency status module. (#810) * Add appengine's nginx latency status module. * Update the commit used for appengine's nginx modules. * Update the appengine commit in WORKSPACE. * Update the appengine repo version to include a bug fix. Also improves the formatting of appengine's latency status modules's output json. * Add appengine nginx module test. Note that this involves adding a gitmodule. * Revert "Add appengine nginx module test." This reverts commit cd5abfaa4f95f27c95cd1eceb19d8b9a0aa43ba4. * Copy test for appengine's nginx latency module. * Move the http_latency_status test to its own directory. * Add a README for the appengine nginx test directory. Co-authored-by: Isabel McCarten --- WORKSPACE | 8 +- src/nginx/main/BUILD | 3 +- src/nginx/main/ngx_modules.c | 9 + third_party/BUILD | 5 +- third_party/appengine-nginx-tests/BUILD | 38 ++++ third_party/appengine-nginx-tests/README.md | 7 + .../http_latency_status.t | 189 ++++++++++++++++++ 7 files changed, 252 insertions(+), 7 deletions(-) create mode 100644 third_party/appengine-nginx-tests/BUILD create mode 100644 third_party/appengine-nginx-tests/README.md create mode 100644 third_party/appengine-nginx-tests/http_latency_status.t diff --git a/WORKSPACE b/WORKSPACE index 17b4349d1..255235e1c 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -57,14 +57,14 @@ nginx_repositories( # Needs to come after nginx git_repository( - name = "iap_jwt_verify_nginx", - commit = "cbdfb7aa74897230c23a46162e3fbe0209cb659a", # Jan 8, 2018 + name = "appengine_nginx", + commit = "6e2854c62b9c456e3ac42b78981fa374373a519f", # Aug 21, 2020 remote = "https://github.com/GoogleCloudPlatform/appengine-sidecars-docker", ) -load("@iap_jwt_verify_nginx//:iap_jwt_verify_nginx.bzl", "iap_jwt_verify_nginx_repositories") +load("@appengine_nginx//:appengine_nginx.bzl", "appengine_nginx_repositories") -iap_jwt_verify_nginx_repositories(True) +appengine_nginx_repositories(True) git_repository( name = "com_github_grpc_grpc", diff --git a/src/nginx/main/BUILD b/src/nginx/main/BUILD index d8b45b261..493d24a0a 100644 --- a/src/nginx/main/BUILD +++ b/src/nginx/main/BUILD @@ -102,7 +102,8 @@ cc_binary( deps = [ "//src/nginx:ngx_esp", "//src/nginx:version_header", - "@iap_jwt_verify_nginx//third_party/iap_jwt_verify_nginx/src:ngx_iap_jwt_verify", + "@appengine_nginx//third_party/iap_jwt_verify_nginx/src:ngx_iap_jwt_verify", + "@appengine_nginx//third_party/nginx_latency_status_module/src:ngx_latency_status_module", "@nginx//:http", "@nginx//:http_access", "@nginx//:http_addition", diff --git a/src/nginx/main/ngx_modules.c b/src/nginx/main/ngx_modules.c index 54f512fa5..f0352d005 100644 --- a/src/nginx/main/ngx_modules.c +++ b/src/nginx/main/ngx_modules.c @@ -198,6 +198,9 @@ extern ngx_module_t ngx_http_stub_status_module; #if (NGX_IAP_JWT_VERIFY) extern ngx_module_t ngx_iap_jwt_verify_module; #endif +#if (NGX_LATENCY_STATUS) +extern ngx_module_t ngx_http_latency_stub_status_module; +#endif #if (NGX_HTTP_ENDPOINTS_RUNTIME) extern ngx_module_t ngx_esp_module; #endif @@ -498,6 +501,9 @@ ngx_module_t *ngx_modules[] = { #if (NGX_IAP_JWT_VERIFY) &ngx_iap_jwt_verify_module, #endif +#if (NGX_LATENCY_STATUS) + &ngx_http_latency_stub_status_module, +#endif #if (NGX_HTTP_ENDPOINTS_RUNTIME) &ngx_esp_module, #endif @@ -799,6 +805,9 @@ char *ngx_module_names[] = { #if (NGX_IAP_JWT_VERIFY) "ngx_iap_jwt_verify_module", #endif +#if (NGX_LATENCY_STATUS) + "ngx_http_latency_stub_status_module", +#endif #if (NGX_HTTP_ENDPOINTS_RUNTIME) "ngx_esp_module", #endif diff --git a/third_party/BUILD b/third_party/BUILD index 8ef3db0d2..d94cb8e39 100644 --- a/third_party/BUILD +++ b/third_party/BUILD @@ -41,8 +41,9 @@ perl_library( nginx_suite( nginx = "//src/nginx/main:nginx-esp", tags = ["exclusive"], - tests = glob( - ["nginx-tests/*.t"], + tests = glob([ + "nginx-tests/*.t", + ], exclude = [ # ssl.t, ssl_certificates.t, ssl_stapling are excluded from the # test suite because boring ssl does not support renegotiation diff --git a/third_party/appengine-nginx-tests/BUILD b/third_party/appengine-nginx-tests/BUILD new file mode 100644 index 000000000..00588a671 --- /dev/null +++ b/third_party/appengine-nginx-tests/BUILD @@ -0,0 +1,38 @@ +# Copyright (C) Extensible Service Proxy Authors +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions +# are met: +# 1. Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# 2. Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS +# OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) +# HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT +# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY +# OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF +# SUCH DAMAGE. +# +################################################################################ +# +load("@io_bazel_rules_perl//perl:perl.bzl", "perl_library") +load("//:nginx.bzl", "nginx_suite") + +licenses(["notice"]) + +nginx_suite( + nginx = "//src/nginx/main:nginx-esp", + tests = ["http_latency_status.t"], + deps = [ + "//third_party:nginx_test", + ], +) diff --git a/third_party/appengine-nginx-tests/README.md b/third_party/appengine-nginx-tests/README.md new file mode 100644 index 000000000..a0a40715a --- /dev/null +++ b/third_party/appengine-nginx-tests/README.md @@ -0,0 +1,7 @@ +# Appengine Nginx Module Test # + +This directory contains a test of ESP's integration with +[AppEngine's Nginx Latency Status Module](https://github.com/GoogleCloudPlatform/appengine-sidecars-docker/tree/master/third_party/nginx_latency_status_module), +and is a copy of a test from the +[GoogleCloudPlatform/appengine-sidecars-docker](https://github.com/GoogleCloudPlatform/appengine-sidecars-docker/tree/master/third_party/nginx_latency_status_module) +repository. diff --git a/third_party/appengine-nginx-tests/http_latency_status.t b/third_party/appengine-nginx-tests/http_latency_status.t new file mode 100644 index 000000000..e8b382ffe --- /dev/null +++ b/third_party/appengine-nginx-tests/http_latency_status.t @@ -0,0 +1,189 @@ +#!/usr/bin/perl + +# Copyright (C) 2002-2016 Igor Sysoev +# Copyright (C) 2011-2016 Nginx, Inc. +# Copyright (C) 2020 Google Inc. +# All rights reserved. +# +# Redistribution and use in source and binary forms, with or without +# modification, are permitted provided that the following conditions +# are met: +# 1. Redistributions of source code must retain the above copyright +# notice, this list of conditions and the following disclaimer. +# 2. Redistributions in binary form must reproduce the above copyright +# notice, this list of conditions and the following disclaimer in the +# documentation and/or other materials provided with the distribution. +# +# THIS SOFTWARE IS PROVIDED BY THE AUTHOR AND CONTRIBUTORS ``AS IS'' AND +# ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +# IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE +# ARE DISCLAIMED. IN NO EVENT SHALL THE AUTHOR OR CONTRIBUTORS BE LIABLE +# FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +# DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS +# OR SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) +# HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT +# LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY +# OUT OF THE USE OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF +# SUCH DAMAGE. + +# tests for the nginx latency status module +# run with TEST_NGINX_BINARY={path to nginx binary} prove http_latency_status.t + +############################################################################### + +use warnings; +use strict; + +use Test::More; + +BEGIN { use FindBin; chdir($FindBin::Bin); } + +use lib "../nginx-tests/lib"; +use Test::Nginx; +use JSON::PP qw(decode_json); +############################################################################### + +select STDERR; $| = 1; +select STDOUT; $| = 1; + +my $t = Test::Nginx->new()->has(qw/http proxy rewrite/)->plan(17); + +$t->write_file_expand('nginx.conf', <<'EOF'); +%%TEST_GLOBALS%% +daemon off; +events { +} +http { + %%TEST_GLOBALS_HTTP%% + store_latency 2 100 1; + upstream u { + server 127.0.0.1:8082; + } + server { + listen 127.0.0.1:8080; + server_name localhost; + location /status { + latency_stub_status; + } + location /success { + return 200 "TEST\n"; + record_latency on; + } + location /pass { + proxy_pass http://u/; + record_latency on; + } + location /error { + return 404; + record_latency on; + } + location /pass-to-monitored { + proxy_pass http://localhost:8081; + } + } + server { + listen 127.0.0.1:8082; + server_name localhost; + location / { + return 200 "TEST\n"; + } + } + server { + listen 127.0.0.1:8081; + server_name localhost; + record_latency on; + location / { + return 200 "TEST\n"; + } + } +} +EOF + +$t->run(); + +my $initial_status = "{\n \"accepted_connections\": 1,\n". + " \"handled_connections\": 1,\n". + " \"active_connections\": 1,\n". + " \"requests\": 1,\n". + " \"reading_connections\": 0,\n". + " \"writing_connections\": 1,\n". + " \"waiting_connections\": 0,\n". + " \"request_latency\":{\n". + " \"latency_sum\": 0,\n". + " \"request_count\": 0,\n". + " \"sum_squares\": 0,\n". + " \"distribution\": [0, 0, 0]\n". + " },\n". + " \"upstream_latency\":{\n". + " \"latency_sum\": 0,\n". + " \"request_count\": 0,\n". + " \"sum_squares\": 0,\n". + " \"distribution\": [0, 0, 0]\n". + " },\n". + " \"websocket_latency\":{\n". + " \"latency_sum\": 0,\n". + " \"request_count\": 0,\n". + " \"sum_squares\": 0,\n". + " \"distribution\": [0, 0, 0]\n". + " },\n". + " \"latency_bucket_bounds\": [ 100, 200]\n". + "}\n"; + +like(http_get('/status'), qr/\Q${initial_status}\E/, 'check initial stats twice'); + +http_get('/success'); +my $status = decode_json(http_get_body('/status')); + +is($status->{'request_latency'}{'request_count'}, 1, 'request count after 1 monitored request'); +is(get_distribution_sum($status, 'request_latency'), 1, 'The sum of the distribution bucket counts after 1 monitored request'); +is($status->{'upstream_latency'}{'request_count'}, 0, 'upstream request count when there have been no proxied requests'); +is(get_distribution_sum($status, 'upstream_latency'), 0, 'sum of upstream distribution bucket counts when there haven been no proxied requests'); +is($status->{'websocket_latency'}{'request_count'}, 0, 'websocket request count when there have been no websocket requests'); +is(get_distribution_sum($status, 'websocket_latency'), 0, 'sum of websocket distribution bucket counts when there have been no websocket requests'); + +http_get('/error'); +$status = decode_json(http_get_body('/status')); + +is($status->{'request_latency'}{'request_count'}, 2, 'erroring requests add to the request count'); +is(get_distribution_sum($status, 'request_latency'), 2, 'erroring requests add to distribution bucket counts'); + +http_get('/pass'); +$status = decode_json(http_get_body('/status')); + +is($status->{'request_latency'}{'request_count'}, 3, 'proxied requests add to the request count'); +is(get_distribution_sum($status, 'request_latency'), 3, 'proxied requests add to the distribution bucket counts'); +is($status->{'upstream_latency'}{'request_count'}, 1, 'proxied requests add to the upstream request count'); +is(get_distribution_sum($status, 'upstream_latency'), 1, 'proxied requests add to the upstream distribution bucket counts'); +is($status->{'websocket_latency'}{'request_count'}, 0, 'a non-websocket proxied request does not add to the websocket request count'); +is(get_distribution_sum($status, 'websocket_latency'), 0, 'non-websocket proxied request does not add to websocket distribution bucket counts'); + +http_get('/pass-to-monitored'); +$status = decode_json(http_get_body('/status')); + +is($status->{'request_latency'}{'request_count'}, 4, 'record_latency set at the server level adds to the request count'); +is(get_distribution_sum($status, 'request_latency'), 4, 'record_latency set at the server level adds to the distribution bucket counts'); + +sub get_distribution_sum { + my ($status, $distribution_name) = @_; + + my $latency_count = 0; + my @distribution = $status->{$distribution_name}{'distribution'}; + for(my $i = 0; $i < @distribution; $i++) { + $latency_count += $status->{$distribution_name}{'distribution'}[$i]; + } + return $latency_count; +} + +sub http_get_body { + my ($uri) = @_; + + return undef if !defined $uri; + + my $text = http_get($uri); + + if ($text !~ /(.*?)\x0d\x0a?\x0d\x0a?(.*)/ms) { + return undef; + } + + return $2; +}