From 820ce8765e21a71492fde5109655aff3c2bdf870 Mon Sep 17 00:00:00 2001 From: Sam Date: Fri, 6 Feb 2015 14:39:04 +1100 Subject: [PATCH] refactor traffic report split traffic report in 2, page view vs raw traffic hide raw traffic report by default improve flushing logic for application reqs --- .../admin/controllers/admin-dashboard.js.es6 | 3 + .../javascripts/admin/templates/dashboard.hbs | 47 ++++++++++--- app/models/admin_dashboard_data.rb | 3 +- app/models/application_request.rb | 25 ++++++- app/models/report.rb | 9 ++- config/initializers/99-request_tracker.rb | 2 +- config/locales/client.en.yml | 3 + config/locales/server.en.yml | 66 ++++++++++--------- ...150206004143_flush_application_requests.rb | 8 +++ lib/middleware/request_tracker.rb | 49 +++++++------- .../middleware/request_tracker_spec.rb | 17 +++-- spec/models/application_request_spec.rb | 37 +++++++---- 12 files changed, 177 insertions(+), 92 deletions(-) create mode 100644 db/migrate/20150206004143_flush_application_requests.rb diff --git a/app/assets/javascripts/admin/controllers/admin-dashboard.js.es6 b/app/assets/javascripts/admin/controllers/admin-dashboard.js.es6 index 65e7f3f01..ad572337e 100644 --- a/app/assets/javascripts/admin/controllers/admin-dashboard.js.es6 +++ b/app/assets/javascripts/admin/controllers/admin-dashboard.js.es6 @@ -53,6 +53,9 @@ export default Ember.Controller.extend({ actions: { refreshProblems: function() { this.loadProblems(); + }, + showTrafficReport: function() { + this.set("showTrafficReport", true); } } diff --git a/app/assets/javascripts/admin/templates/dashboard.hbs b/app/assets/javascripts/admin/templates/dashboard.hbs index 5feb34ce2..2d6b17b23 100644 --- a/app/assets/javascripts/admin/templates/dashboard.hbs +++ b/app/assets/javascripts/admin/templates/dashboard.hbs @@ -66,7 +66,7 @@ - + @@ -75,19 +75,15 @@ {{#unless loading}} - {{ render 'admin_report_counts' topic_anon_reqs }} - {{ render 'admin_report_counts' topic_logged_in_reqs }} - {{ render 'admin_report_counts' topic_crawler_reqs }} - {{ render 'admin_report_counts' background_reqs }} - {{ render 'admin_report_counts' success_reqs }} - {{ render 'admin_report_counts' redirect_reqs }} - {{ render 'admin_report_counts' server_error_reqs }} - {{ render 'admin_report_counts' client_error_reqs }} - {{ render 'admin_report_counts' total_reqs }} + {{ render 'admin_report_counts' page_view_anon_reqs }} + {{ render 'admin_report_counts' page_view_logged_in_reqs }} + {{ render 'admin_report_counts' page_view_crawler_reqs }} + {{ render 'admin_report_counts' page_view_total_reqs }} {{/unless}}
{{i18n 'admin.dashboard.traffic_short'}}{{i18n 'admin.dashboard.page_views_short'}} {{i18n 'admin.dashboard.reports.today'}} {{i18n 'admin.dashboard.reports.yesterday'}} {{i18n 'admin.dashboard.reports.last_7_days'}}
+
@@ -148,6 +144,37 @@
+ + {{#unless loading}} + {{#if showTrafficReport}} +
+ + + + + + + + + + + + {{#unless loading}} + {{ render 'admin_report_counts' http_2xx_reqs }} + {{ render 'admin_report_counts' http_3xx_reqs}} + {{ render 'admin_report_counts' http_4xx_reqs}} + {{ render 'admin_report_counts' http_5xx_reqs}} + {{ render 'admin_report_counts' http_background_reqs }} + {{ render 'admin_report_counts' http_total_reqs }} + {{/unless}} +
{{i18n 'admin.dashboard.traffic_short'}}{{i18n 'admin.dashboard.reports.today'}}{{i18n 'admin.dashboard.reports.yesterday'}}{{i18n 'admin.dashboard.reports.last_7_days'}}{{i18n 'admin.dashboard.reports.last_30_days'}}{{i18n 'admin.dashboard.reports.all'}}
+
+ {{else}} +
+ {{i18n 'admin.dashboard.show_traffic_report'}} +
+ {{/if}} + {{/unless}}
diff --git a/app/models/admin_dashboard_data.rb b/app/models/admin_dashboard_data.rb index aec71b03a..faae6dfc8 100644 --- a/app/models/admin_dashboard_data.rb +++ b/app/models/admin_dashboard_data.rb @@ -16,7 +16,8 @@ class AdminDashboardData 'system_private_messages', 'moderator_warning_private_messages', 'notify_moderators_private_messages', - 'notify_user_private_messages' + 'notify_user_private_messages', + 'page_view_total_reqs' ] + ApplicationRequest.req_types.keys.map{|r| r + "_reqs"} def problems diff --git a/app/models/application_request.rb b/app/models/application_request.rb index dd7d0a188..a178a0cf4 100644 --- a/app/models/application_request.rb +++ b/app/models/application_request.rb @@ -1,9 +1,21 @@ class ApplicationRequest < ActiveRecord::Base - enum req_type: %i(total success background topic_anon topic_logged_in topic_crawler server_error client_error redirect) + enum req_type: %i(http_total + http_2xx + http_background + http_3xx + http_4xx + http_5xx + page_view_crawler + page_view_logged_in + page_view_anon) - cattr_accessor :autoflush + cattr_accessor :autoflush, :autoflush_seconds, :last_flush # auto flush if backlog is larger than this - self.autoflush = 200 + self.autoflush = 2000 + + # auto flush if older than this + self.autoflush_seconds = 5.minutes + self.last_flush = Time.now def self.increment!(type, opts=nil) key = redis_key(type) @@ -13,6 +25,11 @@ class ApplicationRequest < ActiveRecord::Base autoflush = (opts && opts[:autoflush]) || self.autoflush if autoflush > 0 && val >= autoflush write_cache! + return + end + + if (Time.now - last_flush).to_i > autoflush_seconds + write_cache! end end @@ -23,6 +40,8 @@ class ApplicationRequest < ActiveRecord::Base return end + self.last_flush = Time.now + date = date.to_date # this may seem a bit fancy but in so it allows diff --git a/app/models/report.rb b/app/models/report.rb index 848644c9f..03092648e 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -50,7 +50,14 @@ class Report end def self.req_report(report, filter=nil) - data = ApplicationRequest.where(req_type: ApplicationRequest.req_types[filter]) + data = + if filter == :page_view_total + ApplicationRequest.where(req_type: [ + ApplicationRequest.req_types.map{|k,v| v if k =~ /page_view/}.compact + ]) + else + ApplicationRequest.where(req_type: ApplicationRequest.req_types[filter]) + end filtered_results = data.where('date >= ? AND date <= ?', report.start_date.to_date, report.end_date.to_date) diff --git a/config/initializers/99-request_tracker.rb b/config/initializers/99-request_tracker.rb index 006748633..5ba1330e1 100644 --- a/config/initializers/99-request_tracker.rb +++ b/config/initializers/99-request_tracker.rb @@ -1,6 +1,6 @@ # no reason to track this in development, that is 300+ redis calls saved per # page view (we serve all assets out of thin in development) -if Rails.env != 'development' +if Rails.env != 'development' || ENV['TRACK_REQUESTS'] require 'middleware/request_tracker' Rails.configuration.middleware.unshift Middleware::RequestTracker end diff --git a/config/locales/client.en.yml b/config/locales/client.en.yml index e915a2016..f00b12b0a 100644 --- a/config/locales/client.en.yml +++ b/config/locales/client.en.yml @@ -1537,6 +1537,9 @@ en: backups: "backups" traffic_short: "Traffic" traffic: "Application web requests" + page_views: "Page Views" + page_views_short: "Page Views" + show_traffic_report: "Show Detailed Traffic Report" reports: today: "Today" diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index 0294b309f..611dd62af 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -590,42 +590,46 @@ en: title: "Top Referred Topics" xaxis: "Topic" num_clicks: "Clicks" - topic_anon_reqs: - title: "Anonymous Topic Views" + page_view_anon_reqs: + title: "Anonymous" xaxis: "Day" - yaxis: "Anonymous Topic Views" - topic_logged_in_reqs: - title: "Logged In Topic Views" + yaxis: "Anonymous Page Views" + page_view_logged_in_reqs: + title: "Logged In" xaxis: "Day" - yaxis: "Logged In Topic Views" - topic_crawler_reqs: - title: "Crawler Topic Views" + yaxis: "Logged In Page Views" + page_view_crawler_reqs: + title: "Web Crawlers" xaxis: "Day" - yaxis: "Crawler Topic Views" - background_reqs: - title: "Background requests" - xaxis: "Day" - yaxis: "Requests used for live update and tracking" - success_reqs: - title: "Successful requests" - xaxis: "Day" - yaxis: "Successful requests (Status 2xx)" - redirect_reqs: - title: "Redirect requests" - xaxis: "Day" - yaxis: "Redirect requests (Status 3xx)" - server_error_reqs: - title: "Server Errors" - xaxis: "Day" - yaxis: "Server Errors (Status 5xx)" - client_error_reqs: - title: "Bad requests" - xaxis: "Day" - yaxis: "Client Errors (Status 4xx)" - total_reqs: + yaxis: "Web Crawler Page Views" + page_view_total_reqs: title: "Total" xaxis: "Day" - yaxis: "Total application requests" + yaxis: "Total Page Views" + http_background_reqs: + title: "Background" + xaxis: "Day" + yaxis: "Requests used for live update and tracking" + http_2xx_reqs: + title: "Status 2xx (OK)" + xaxis: "Day" + yaxis: "Successful requests (Status 2xx)" + http_3xx_reqs: + title: "HTTP 3xx (Redirect)" + xaxis: "Day" + yaxis: "Redirect requests (Status 3xx)" + http_4xx_reqs: + title: "HTTP 4xx (Client Error)" + xaxis: "Day" + yaxis: "Client Errors (Status 4xx)" + http_5xx_reqs: + title: "HTTP 5xx (Server Error)" + xaxis: "Day" + yaxis: "Server Errors (Status 5xx)" + http_total_reqs: + title: "Total" + xaxis: "Day" + yaxis: "Total requests" dashboard: rails_env_warning: "Your server is running in %{env} mode." diff --git a/db/migrate/20150206004143_flush_application_requests.rb b/db/migrate/20150206004143_flush_application_requests.rb new file mode 100644 index 000000000..5d5a6e454 --- /dev/null +++ b/db/migrate/20150206004143_flush_application_requests.rb @@ -0,0 +1,8 @@ +class FlushApplicationRequests < ActiveRecord::Migration + def up + # flush as enum changed + execute "TRUNCATE TABLE application_requests" + end + def down + end +end diff --git a/lib/middleware/request_tracker.rb b/lib/middleware/request_tracker.rb index b300bb20c..b6e9b3a4e 100644 --- a/lib/middleware/request_tracker.rb +++ b/lib/middleware/request_tracker.rb @@ -14,44 +14,45 @@ class Middleware::RequestTracker end PATH_PARAMS = "action_dispatch.request.path_parameters".freeze + TRACK_VIEW = "HTTP_DISCOURSE_TRACK_VIEW".freeze + def self.log_request(result,env,helper=nil) helper ||= Middleware::AnonymousCache::Helper.new(env) - params = env[PATH_PARAMS] request = Rack::Request.new(env) - ApplicationRequest.increment!(:total) - - status,_ = result + status,headers = result status = status.to_i - if status >= 500 - ApplicationRequest.increment!(:server_error) - elsif status >= 400 - ApplicationRequest.increment!(:client_error) - elsif status >= 300 - ApplicationRequest.increment!(:redirect) - end - - if request.path =~ /^\/message-bus\// || request.path == /\/topics\/timings/ - ApplicationRequest.increment!(:background) - elsif status >= 200 && status < 300 - ApplicationRequest.increment!(:success) - end - - if params && params[:controller] == "topics" && params[:action] == "show" + if (env[TRACK_VIEW] || (request.get? && !request.xhr? && headers["Content-Type"] =~ /text\/html/)) && status == 200 if helper.is_crawler? - ApplicationRequest.increment!(:topic_crawler) + ApplicationRequest.increment!(:page_view_crawler) elsif helper.has_auth_cookie? - ApplicationRequest.increment!(:topic_logged_in) + ApplicationRequest.increment!(:page_view_logged_in) else - ApplicationRequest.increment!(:topic_anon) + ApplicationRequest.increment!(:page_view_anon) end end - rescue => ex - Discourse.handle_exception(ex, {message: "Failed to log request"}) + ApplicationRequest.increment!(:http_total) + + if status >= 500 + ApplicationRequest.increment!(:http_5xx) + elsif status >= 400 + ApplicationRequest.increment!(:http_4xx) + elsif status >= 300 + ApplicationRequest.increment!(:http_3xx) + else + if request.path =~ /^\/message-bus\// || request.path == /\/topics\/timings/ + ApplicationRequest.increment!(:http_background) + elsif status >= 200 && status < 300 + ApplicationRequest.increment!(:http_2xx) + end + end + + # rescue => ex + # Discourse.handle_exception(ex, {message: "Failed to log request"}) end diff --git a/spec/components/middleware/request_tracker_spec.rb b/spec/components/middleware/request_tracker_spec.rb index bbf0e817d..4de9af7eb 100644 --- a/spec/components/middleware/request_tracker_spec.rb +++ b/spec/components/middleware/request_tracker_spec.rb @@ -18,21 +18,20 @@ describe Middleware::RequestTracker do ApplicationRequest.clear_cache! - Middleware::RequestTracker.log_request(["200"], env( - "HTTP_USER_AGENT" => "AdsBot-Google (+http://www.google.com/adsbot.html)", - "action_dispatch.request.path_parameters" => {controller: "topics", action: "show"} + Middleware::RequestTracker.log_request(["200",{"Content-Type" => 'text/html'}], env( + "HTTP_USER_AGENT" => "AdsBot-Google (+http://www.google.com/adsbot.html)" )) - Middleware::RequestTracker.log_request(["200"], env( - "action_dispatch.request.path_parameters" => {controller: "topics", action: "show"} + Middleware::RequestTracker.log_request(["200",{}], env( + "HTTP_DISCOURSE_TRACK_VIEW" => "1" )) ApplicationRequest.write_cache! - ApplicationRequest.total.first.count.should == 2 - ApplicationRequest.success.first.count.should == 2 + ApplicationRequest.http_total.first.count.should == 2 + ApplicationRequest.http_2xx.first.count.should == 2 - ApplicationRequest.topic_anon.first.count.should == 1 - ApplicationRequest.topic_crawler.first.count.should == 1 + ApplicationRequest.page_view_anon.first.count.should == 1 + ApplicationRequest.page_view_crawler.first.count.should == 1 end end end diff --git a/spec/models/application_request_spec.rb b/spec/models/application_request_spec.rb index 9f823fe89..bfc4ff63b 100644 --- a/spec/models/application_request_spec.rb +++ b/spec/models/application_request_spec.rb @@ -18,19 +18,32 @@ describe ApplicationRequest do it 'can automatically flush' do t1 = Time.now.utc.at_midnight freeze_time(t1) - inc(:total) - inc(:total) - inc(:total, autoflush: 3) + inc(:http_total) + inc(:http_total) + inc(:http_total, autoflush: 3) - ApplicationRequest.first.count.should == 3 + ApplicationRequest.http_total.first.count.should == 3 + end + + it 'can flush based on time' do + t1 = Time.now.utc.at_midnight + freeze_time(t1) + ApplicationRequest.write_cache! + inc(:http_total) + ApplicationRequest.count.should == 0 + + freeze_time(t1 + ApplicationRequest.autoflush_seconds + 1) + inc(:http_total) + + ApplicationRequest.count.should == 1 end it 'flushes yesterdays results' do t1 = Time.now.utc.at_midnight freeze_time(t1) - inc(:total) + inc(:http_total) freeze_time(t1.tomorrow) - inc(:total) + inc(:http_total) ApplicationRequest.write_cache! ApplicationRequest.count.should == 2 @@ -49,15 +62,15 @@ describe ApplicationRequest do time = Time.now.at_midnight freeze_time(time) - 3.times { inc(:total) } - 2.times { inc(:success) } - 4.times { inc(:redirect) } + 3.times { inc(:http_total) } + 2.times { inc(:http_2xx) } + 4.times { inc(:http_3xx) } ApplicationRequest.write_cache! - ApplicationRequest.total.first.count.should == 3 - ApplicationRequest.success.first.count.should == 2 - ApplicationRequest.redirect.first.count.should == 4 + ApplicationRequest.http_total.first.count.should == 3 + ApplicationRequest.http_2xx.first.count.should == 2 + ApplicationRequest.http_3xx.first.count.should == 4 end end