From c743e79ad904a140cc7427cacf4fddf51bb9c1a4 Mon Sep 17 00:00:00 2001 From: Nick Muerdter Date: Thu, 7 Jun 2018 15:45:43 -0600 Subject: [PATCH] Fix analytics tests and fix analytics APIs for no indices. - Get all the analytics-related tests working again in this branch of merged upgrades/things. - We had added some tests for the behavior of analytics API queries against date ranges without any elasticsearch indices present. Fix that behavior in the Rails version of the web-app by handling the lack of aggregation results. - Other test suite fixes. --- ...on => elasticsearch_templates_v1_es1.json} | 18 +--- ...on => elasticsearch_templates_v1_es5.json} | 2 +- ...on => elasticsearch_templates_v2_es5.json} | 3 +- .../init_elasticsearch_templates_data.lua | 8 +- .../app/controllers/admin/stats_controller.rb | 10 ++- .../api/v1/analytics_controller.rb | 82 ++++++++++--------- .../web-app/app/models/log_result/base.rb | 78 ++++++++++-------- .../web-app/app/views/admin/stats/map.rabl | 50 ++++++----- test/apis/admin/stats/test_logs.rb | 1 + test/apis/admin/stats/test_users.rb | 17 ++-- test/proxy/logging/test_basics.rb | 8 +- .../api_umbrella_test_helpers/logging.rb | 2 +- 12 files changed, 150 insertions(+), 129 deletions(-) rename config/{elasticsearch_templates_v1_es2x.json => elasticsearch_templates_v1_es1.json} (91%) rename config/{elasticsearch_templates_v1.json => elasticsearch_templates_v1_es5.json} (99%) rename config/{elasticsearch_templates_v2.json => elasticsearch_templates_v2_es5.json} (98%) diff --git a/config/elasticsearch_templates_v1_es2x.json b/config/elasticsearch_templates_v1_es1.json similarity index 91% rename from config/elasticsearch_templates_v1_es2x.json rename to config/elasticsearch_templates_v1_es1.json index da1caf45a..107412363 100644 --- a/config/elasticsearch_templates_v1_es2x.json +++ b/config/elasticsearch_templates_v1_es1.json @@ -5,7 +5,7 @@ "template": "api-umbrella-logs-v1-*", "settings": { "index": { - "number_of_shards": 1, + "number_of_shards": 3, "codec": "best_compression" }, "analysis": { @@ -27,21 +27,7 @@ "_all": { "enabled": false }, - "date_detection": false, - "numeric_detection": false, - "dynamic_templates": [ - { - "string_template": { - "match": "*", - "match_mapping_type": "string", - "mapping": { - "type": "string", - "index": "analyzed", - "analyzer": "keyword_lowercase" - } - } - } - ], + "dynamic": false, "properties": { "api_backend_id": { "type": "string", diff --git a/config/elasticsearch_templates_v1.json b/config/elasticsearch_templates_v1_es5.json similarity index 99% rename from config/elasticsearch_templates_v1.json rename to config/elasticsearch_templates_v1_es5.json index e81b681ad..41161d1d8 100644 --- a/config/elasticsearch_templates_v1.json +++ b/config/elasticsearch_templates_v1_es5.json @@ -5,7 +5,7 @@ "template": "api-umbrella-logs-v1-*", "settings": { "index": { - "number_of_shards": 1, + "number_of_shards": 3, "codec": "best_compression" }, "analysis": { diff --git a/config/elasticsearch_templates_v2.json b/config/elasticsearch_templates_v2_es5.json similarity index 98% rename from config/elasticsearch_templates_v2.json rename to config/elasticsearch_templates_v2_es5.json index 4fafc3c6a..b270b9335 100644 --- a/config/elasticsearch_templates_v2.json +++ b/config/elasticsearch_templates_v2_es5.json @@ -84,7 +84,8 @@ "normalizer": "lowercase_normalizer" }, "request_ip_city": { - "type": "keyword" + "type": "keyword", + "normalizer": "lowercase_normalizer" }, "request_ip_country": { "type": "keyword", diff --git a/src/api-umbrella/proxy/startup/init_elasticsearch_templates_data.lua b/src/api-umbrella/proxy/startup/init_elasticsearch_templates_data.lua index a866871d6..2ce5e265d 100644 --- a/src/api-umbrella/proxy/startup/init_elasticsearch_templates_data.lua +++ b/src/api-umbrella/proxy/startup/init_elasticsearch_templates_data.lua @@ -1,6 +1,12 @@ local cjson = require "cjson" -local path = os.getenv("API_UMBRELLA_SRC_ROOT") .. "/config/elasticsearch_templates_v" .. config["elasticsearch"]["template_version"] .. "_es2x.json" +local path = os.getenv("API_UMBRELLA_SRC_ROOT") .. "/config/elasticsearch_templates_v" .. config["elasticsearch"]["template_version"] +if config["elasticsearch"]["api_version"] >= 5 then + path = path .. "_es5.json" +else + path = path .. "_es1.json" +end + local f, err = io.open(path, "rb") if err then ngx.log(ngx.ERR, "failed to open file: ", err) diff --git a/src/api-umbrella/web-app/app/controllers/admin/stats_controller.rb b/src/api-umbrella/web-app/app/controllers/admin/stats_controller.rb index 90baa8c77..38c4fdd90 100644 --- a/src/api-umbrella/web-app/app/controllers/admin/stats_controller.rb +++ b/src/api-umbrella/web-app/app/controllers/admin/stats_controller.rb @@ -146,7 +146,7 @@ def users @search.aggregate_by_user_stats!(aggregation_options) @result = @search.result - buckets = @result.aggregations["user_stats"]["buckets"] + buckets = if(@result.aggregations && @result.aggregations["user_stats"]) then @result.aggregations["user_stats"]["buckets"] else [] end @total = buckets.length # If we were sorting by one of the facet fields, then the sorting has @@ -195,7 +195,9 @@ def users respond_to do |format| format.json - format.csv + format.csv do + @filename = "api_users_#{Time.now.utc.strftime("%Y-%m-%d")}.csv" + end end end @@ -216,7 +218,9 @@ def map respond_to do |format| format.json - format.csv + format.csv do + @filename = "api_map_#{Time.now.utc.strftime("%Y-%m-%d")}.csv" + end end end diff --git a/src/api-umbrella/web-app/app/controllers/api/v1/analytics_controller.rb b/src/api-umbrella/web-app/app/controllers/api/v1/analytics_controller.rb index 254c3d115..274f0418a 100644 --- a/src/api-umbrella/web-app/app/controllers/api/v1/analytics_controller.rb +++ b/src/api-umbrella/web-app/app/controllers/api/v1/analytics_controller.rb @@ -28,7 +28,9 @@ def drilldown @result = @search.result respond_to do |format| - format.csv + format.csv do + @filename = "api_drilldown_#{Time.now.utc.strftime("%Y-%m-%d")}.csv" + end format.json do @breadcrumbs = [ :crumb => "All Hosts", @@ -51,48 +53,50 @@ def drilldown :rows => [], } - @result.aggregations["top_path_hits_over_time"]["buckets"].each do |bucket| - @hits_over_time[:cols] << { - :id => bucket["key"], - :label => bucket["key"].split("/", 2).last, - :type => "number", - } - end - - has_other_hits = false - @result.aggregations["hits_over_time"]["buckets"].each_with_index do |total_bucket, index| - cells = [ - { :v => total_bucket["key"], :f => formatted_interval_time(total_bucket["key"]) }, - ] - - path_total_hits = 0 - @result.aggregations["top_path_hits_over_time"]["buckets"].each do |path_bucket| - bucket = path_bucket["drilldown_over_time"]["buckets"][index] - cells << { :v => bucket["doc_count"], :f => number_with_delimiter(bucket["doc_count"]) } - path_total_hits += bucket["doc_count"] + if @result.aggregations + @result.aggregations["top_path_hits_over_time"]["buckets"].each do |bucket| + @hits_over_time[:cols] << { + :id => bucket["key"], + :label => bucket["key"].split("/", 2).last, + :type => "number", + } end - other_hits = total_bucket["doc_count"] - path_total_hits - cells << { :v => other_hits, :f => number_with_delimiter(other_hits) } - - @hits_over_time[:rows] << { - :c => cells, - } - - if(other_hits > 0) - has_other_hits = true + has_other_hits = false + @result.aggregations["hits_over_time"]["buckets"].each_with_index do |total_bucket, index| + cells = [ + { :v => total_bucket["key"], :f => formatted_interval_time(total_bucket["key"]) }, + ] + + path_total_hits = 0 + @result.aggregations["top_path_hits_over_time"]["buckets"].each do |path_bucket| + bucket = path_bucket["drilldown_over_time"]["buckets"][index] + cells << { :v => bucket["doc_count"], :f => number_with_delimiter(bucket["doc_count"]) } + path_total_hits += bucket["doc_count"] + end + + other_hits = total_bucket["doc_count"] - path_total_hits + cells << { :v => other_hits, :f => number_with_delimiter(other_hits) } + + @hits_over_time[:rows] << { + :c => cells, + } + + if(other_hits > 0) + has_other_hits = true + end end - end - if(has_other_hits) - @hits_over_time[:cols] << { - :id => "other", - :label => "Other", - :type => "number", - } - else - @hits_over_time[:rows].each do |row| - row[:c].slice!(-1) + if(has_other_hits) + @hits_over_time[:cols] << { + :id => "other", + :label => "Other", + :type => "number", + } + else + @hits_over_time[:rows].each do |row| + row[:c].slice!(-1) + end end end end diff --git a/src/api-umbrella/web-app/app/models/log_result/base.rb b/src/api-umbrella/web-app/app/models/log_result/base.rb index f7c02716e..18cbbf4b1 100644 --- a/src/api-umbrella/web-app/app/models/log_result/base.rb +++ b/src/api-umbrella/web-app/app/models/log_result/base.rb @@ -19,11 +19,13 @@ def aggregations end def hits_over_time - if(!@hits_over_time && aggregations["hits_over_time"]) + unless @hits_over_time @hits_over_time = {} - aggregations["hits_over_time"]["buckets"].each do |bucket| - @hits_over_time[bucket["key"]] = bucket["doc_count"] + if(aggregations && aggregations["hits_over_time"]) + aggregations["hits_over_time"]["buckets"].each do |bucket| + @hits_over_time[bucket["key"]] = bucket["doc_count"] + end end end @@ -31,24 +33,26 @@ def hits_over_time end def drilldown - if(!@drilldown && aggregations["drilldown"]) + unless @drilldown @drilldown = [] - aggregations["drilldown"]["buckets"].each do |bucket| - depth, path = bucket["key"].split("/", 2) - terminal = !path.end_with?("/") - - depth = depth.to_i - descendent_depth = depth + 1 - descendent_prefix = File.join(descendent_depth.to_s, path) - - @drilldown << { - :depth => depth, - :path => path, - :terminal => terminal, - :descendent_prefix => descendent_prefix, - :hits => bucket["doc_count"], - } + if(aggregations && aggregations["drilldown"]) + aggregations["drilldown"]["buckets"].each do |bucket| + depth, path = bucket["key"].split("/", 2) + terminal = !path.end_with?("/") + + depth = depth.to_i + descendent_depth = depth + 1 + descendent_prefix = File.join(descendent_depth.to_s, path) + + @drilldown << { + :depth => depth, + :path => path, + :terminal => terminal, + :descendent_prefix => descendent_prefix, + :hits => bucket["doc_count"], + } + end end end @@ -86,23 +90,25 @@ def cities unless @cities @cities = {} - @regions = aggregations["regions"]["buckets"] - if(@search.query[:aggregations][:regions][:terms][:field] == "request_ip_city") - @city_names = @regions.map { |bucket| bucket["key"] } - @cities = {} - - if @city_names.any? - cities = LogCityLocation.where(:country => @search.country) - if @search.state - cities = cities.where(:region => @search.state) - end - cities = cities.where(:city.in => @city_names) - - cities.each do |city| - @cities[city.city] = { - "lat" => city.location["coordinates"][1], - "lon" => city.location["coordinates"][0], - } + if(aggregations && aggregations["regions"]) + @regions = aggregations["regions"]["buckets"] + if(@search.query[:aggregations][:regions][:terms][:field] == "request_ip_city") + @city_names = @regions.map { |bucket| bucket["key"] } + @cities = {} + + if @city_names.any? + cities = LogCityLocation.where(:country => @search.country) + if @search.state + cities = cities.where(:region => @search.state) + end + cities = cities.where(:city.in => @city_names) + + cities.each do |city| + @cities[city.city] = { + "lat" => city.location["coordinates"][1], + "lon" => city.location["coordinates"][0], + } + end end end end diff --git a/src/api-umbrella/web-app/app/views/admin/stats/map.rabl b/src/api-umbrella/web-app/app/views/admin/stats/map.rabl index 9c50d0679..4dbec4110 100644 --- a/src/api-umbrella/web-app/app/views/admin/stats/map.rabl +++ b/src/api-umbrella/web-app/app/views/admin/stats/map.rabl @@ -5,32 +5,40 @@ node :region_field do end node :regions do - rows = @result.aggregations["regions"]["buckets"].map do |bucket| - { - :id => region_id(bucket["key"]), - :name => region_name(bucket["key"]), - :hits => bucket["doc_count"], - } - end + if(@result.aggregations && @result.aggregations["regions"]) + rows = @result.aggregations["regions"]["buckets"].map do |bucket| + { + :id => region_id(bucket["key"]), + :name => region_name(bucket["key"]), + :hits => bucket["doc_count"], + } + end - if(@result.aggregations["missing_regions"]["doc_count"] > 0) - rows << { - :id => "missing", - :name => "Unknown", - :hits => @result.aggregations["missing_regions"]["doc_count"], - } - end + if(@result.aggregations["missing_regions"]["doc_count"] > 0) + rows << { + :id => "missing", + :name => "Unknown", + :hits => @result.aggregations["missing_regions"]["doc_count"], + } + end - rows + rows + else + [] + end end node :map_regions do - @result.aggregations["regions"]["buckets"].map do |bucket| - { - :c => region_location_columns(bucket) + [ - { :v => bucket["doc_count"], :f => number_with_delimiter(bucket["doc_count"]) }, - ] - } + if(@result.aggregations && @result.aggregations["regions"]) + @result.aggregations["regions"]["buckets"].map do |bucket| + { + :c => region_location_columns(bucket) + [ + { :v => bucket["doc_count"], :f => number_with_delimiter(bucket["doc_count"]) }, + ] + } + end + else + [] end end diff --git a/test/apis/admin/stats/test_logs.rb b/test/apis/admin/stats/test_logs.rb index 0cb78a86a..ffe02823a 100644 --- a/test/apis/admin/stats/test_logs.rb +++ b/test/apis/admin/stats/test_logs.rb @@ -101,6 +101,7 @@ def test_query_builder_case_insensitive_defaults def test_query_builder_api_key_case_sensitive FactoryBot.create(:log_item, :request_at => Time.parse("2015-01-16T06:06:28.816Z").utc, :api_key => "AbCDeF", :request_user_agent => unique_test_id) + LogItem.refresh_indices! response = Typhoeus.get("https://127.0.0.1:9081/admin/stats/logs.json", http_options.deep_merge(admin_session).deep_merge({ :params => { diff --git a/test/apis/admin/stats/test_users.rb b/test/apis/admin/stats/test_users.rb index 20d30efde..38e404d0d 100644 --- a/test/apis/admin/stats/test_users.rb +++ b/test/apis/admin/stats/test_users.rb @@ -12,7 +12,7 @@ def setup LogItem.clean_indices! end - def test_world + def test_json FactoryBot.create_list(:log_item, 2, :request_at => Time.parse("2015-01-16T00:00:00.000Z").utc, :user_id => @user1.id) FactoryBot.create_list(:log_item, 1, :request_at => Time.parse("2015-01-17T00:00:00.000Z").utc, :user_id => @user2.id) LogItem.refresh_indices! @@ -21,6 +21,7 @@ def test_world :params => { "start_at" => "2015-01-13", "end_at" => "2015-01-18", + "length" => "10", }, })) @@ -69,7 +70,6 @@ def test_csv_download :params => { "start_at" => "2015-01-13", "end_at" => "2015-01-18", - "region" => "world", }, })) @@ -94,23 +94,23 @@ def test_csv_download @user1.email, @user1.first_name, @user1.last_name, - @user1.website, - @user1.registration_source, + @user1.website || "", + @user1.registration_source || "", @user1.created_at.utc.strftime("%Y-%m-%d %H:%M:%S"), "2", "2015-01-16 00:00:00", - @user1.use_description, + @user1.use_description || "", ], csv[1]) assert_equal([ @user2.email, @user2.first_name, @user2.last_name, - @user2.website, - @user2.registration_source, + @user2.website || "", + @user2.registration_source || "", @user2.created_at.utc.strftime("%Y-%m-%d %H:%M:%S"), "1", "2015-01-17 00:00:00", - @user2.use_description, + @user2.use_description || "", ], csv[2]) end @@ -119,6 +119,7 @@ def test_no_results_non_existent_indices :params => { "start_at" => "2000-01-13", "end_at" => "2000-01-18", + "length" => "10", }, })) diff --git a/test/proxy/logging/test_basics.rb b/test/proxy/logging/test_basics.rb index 843d24b27..e0c5350d9 100644 --- a/test/proxy/logging/test_basics.rb +++ b/test/proxy/logging/test_basics.rb @@ -95,11 +95,15 @@ def test_logs_expected_fields_for_non_chunked_non_gzip "request_ip_city", "request_ip_country", "request_ip_region", - "request_url_hierarchy_level5", - "request_url_hierarchy_level6", "response_content_encoding", "response_transfer_encoding", ] + if($config["elasticsearch"]["template_version"] >= 2) + expected_mapping_fields += [ + "request_url_hierarchy_level5", + "request_url_hierarchy_level6", + ] + end assert_equal(expected_mapping_fields.sort, mapping[hit["_index"]]["mappings"][hit["_type"]]["properties"].keys.sort) assert_kind_of(String, record["api_backend_id"]) diff --git a/test/support/api_umbrella_test_helpers/logging.rb b/test/support/api_umbrella_test_helpers/logging.rb index fbc731376..35f19f404 100644 --- a/test/support/api_umbrella_test_helpers/logging.rb +++ b/test/support/api_umbrella_test_helpers/logging.rb @@ -33,7 +33,7 @@ def wait_for_log(response, options = {}) begin Timeout.timeout(options[:timeout]) do loop do - result = LogItem.gateway.client.search({ + result = LogItem.client.search({ :index => "_all", :type => "log", :body => {