From 3048d3d07da9579a4ea4f09560c67655deb6b224 Mon Sep 17 00:00:00 2001 From: Bianca Nenciu Date: Tue, 29 Nov 2022 13:07:42 +0200 Subject: [PATCH] FEATURE: Track API and user API requests (#19186) Adds stats for API and user API requests similar to regular page views. This comes with a new report to visualize API requests per day like the consolidated page views one. --- app/models/application_request.rb | 4 +- .../reports/consolidated_api_requests.rb | 40 +++++++++++++++ app/models/report.rb | 1 + config/locales/server.en.yml | 7 +++ lib/middleware/request_tracker.rb | 15 ++++-- spec/integration/request_tracker_spec.rb | 51 +++++++++++++++++++ spec/lib/middleware/request_tracker_spec.rb | 27 ++++++++++ spec/models/report_spec.rb | 47 +++++++++++++++++ 8 files changed, 187 insertions(+), 5 deletions(-) create mode 100644 app/models/concerns/reports/consolidated_api_requests.rb create mode 100644 spec/integration/request_tracker_spec.rb diff --git a/app/models/application_request.rb b/app/models/application_request.rb index 948c2185837..81125566e4a 100644 --- a/app/models/application_request.rb +++ b/app/models/application_request.rb @@ -11,7 +11,9 @@ class ApplicationRequest < ActiveRecord::Base page_view_logged_in page_view_anon page_view_logged_in_mobile - page_view_anon_mobile) + page_view_anon_mobile + api + user_api) include CachedCounting diff --git a/app/models/concerns/reports/consolidated_api_requests.rb b/app/models/concerns/reports/consolidated_api_requests.rb new file mode 100644 index 00000000000..ab25b1f73f1 --- /dev/null +++ b/app/models/concerns/reports/consolidated_api_requests.rb @@ -0,0 +1,40 @@ +# frozen_string_literal: true + +module Reports::ConsolidatedApiRequests + extend ActiveSupport::Concern + + class_methods do + def report_consolidated_api_requests(report) + filters = %w[ + api + user_api + ] + + report.modes = [:stacked_chart] + + tertiary = ColorScheme.hex_for_name('tertiary') || '0088cc' + danger = ColorScheme.hex_for_name('danger') || 'e45735' + + requests = filters.map do |filter| + color = filter == "api" ? report.rgba_color(tertiary) : report.rgba_color(danger) + + { + req: filter, + label: I18n.t("reports.consolidated_api_requests.xaxis.#{filter}"), + color: color, + data: ApplicationRequest.where(req_type: ApplicationRequest.req_types[filter]) + } + end + + requests.each do |request| + request[:data] = request[:data].where('date >= ? AND date <= ?', report.start_date, report.end_date) + .order(date: :asc) + .group(:date) + .sum(:count) + .map { |date, count| { x: date, y: count } } + end + + report.data = requests + end + end +end diff --git a/app/models/report.rb b/app/models/report.rb index 91a6a808b67..fa893edfcb4 100644 --- a/app/models/report.rb +++ b/app/models/report.rb @@ -40,6 +40,7 @@ class Report include Reports::UserFlaggingRatio include Reports::TrustLevelGrowth include Reports::ConsolidatedPageViews + include Reports::ConsolidatedApiRequests include Reports::Visits include Reports::TimeToFirstResponse include Reports::UsersByTrustLevel diff --git a/config/locales/server.en.yml b/config/locales/server.en.yml index b998a286208..fedb1ac76a2 100644 --- a/config/locales/server.en.yml +++ b/config/locales/server.en.yml @@ -1179,6 +1179,13 @@ en: editor: Editor author: Author edit_reason: Reason + consolidated_api_requests: + title: "Consolidated API Requests" + xaxis: + api: "API" + user_api: "User API" + yaxis: "Day" + description: "API requests for regular API keys and user API keys." dau_by_mau: title: "DAU/MAU" xaxis: "Day" diff --git a/lib/middleware/request_tracker.rb b/lib/middleware/request_tracker.rb index 502b78daf6a..fda5fc46f28 100644 --- a/lib/middleware/request_tracker.rb +++ b/lib/middleware/request_tracker.rb @@ -64,10 +64,11 @@ class Middleware::RequestTracker end def self.log_request(data) - status = data[:status] - track_view = data[:track_view] - - if track_view + if data[:is_api] + ApplicationRequest.increment!(:api) + elsif data[:is_user_api] + ApplicationRequest.increment!(:user_api) + elsif data[:track_view] if data[:is_crawler] ApplicationRequest.increment!(:page_view_crawler) WebCrawlerRequest.increment!(data[:user_agent]) @@ -82,6 +83,7 @@ class Middleware::RequestTracker ApplicationRequest.increment!(:http_total) + status = data[:status] if status >= 500 ApplicationRequest.increment!(:http_5xx) elsif data[:is_background] @@ -110,6 +112,9 @@ class Middleware::RequestTracker has_auth_cookie = Auth::DefaultCurrentUserProvider.find_v0_auth_cookie(request).present? has_auth_cookie ||= Auth::DefaultCurrentUserProvider.find_v1_auth_cookie(env).present? + is_api ||= !!env[Auth::DefaultCurrentUserProvider::API_KEY_ENV] + is_user_api ||= !!env[Auth::DefaultCurrentUserProvider::USER_API_KEY_ENV] + is_message_bus = request.path.start_with?("#{Discourse.base_path}/message-bus/") is_topic_timings = request.path.start_with?("#{Discourse.base_path}/topics/timings") @@ -117,6 +122,8 @@ class Middleware::RequestTracker status: status, is_crawler: helper.is_crawler?, has_auth_cookie: has_auth_cookie, + is_api: is_api, + is_user_api: is_user_api, is_background: is_message_bus || is_topic_timings, background_type: is_message_bus ? "message-bus" : "topic-timings", is_mobile: helper.is_mobile?, diff --git a/spec/integration/request_tracker_spec.rb b/spec/integration/request_tracker_spec.rb new file mode 100644 index 00000000000..239d4155d3d --- /dev/null +++ b/spec/integration/request_tracker_spec.rb @@ -0,0 +1,51 @@ +# frozen_string_literal: true + +RSpec.describe 'request tracker' do + let(:api_key) do + Fabricate( + :api_key, + user: Fabricate.build(:user), + api_key_scopes: [ApiKeyScope.new(resource: 'users', action: 'show')] + ) + end + + let(:user_api_key) do + Fabricate(:user_api_key, scopes: [Fabricate.build(:user_api_key_scope, name: 'session_info')]) + end + + before do + CachedCounting.reset + CachedCounting.enable + ApplicationRequest.enable + end + + after do + ApplicationRequest.disable + CachedCounting.reset + CachedCounting.disable + end + + context 'when using an api key' do + it 'is counted as an API request' do + get "/u/#{api_key.user.username}.json", headers: { HTTP_API_KEY: api_key.key } + expect(response.status).to eq(200) + + CachedCounting.flush + expect(ApplicationRequest.http_total.first.count).to eq(1) + expect(ApplicationRequest.http_2xx.first.count).to eq(1) + expect(ApplicationRequest.api.first.count).to eq(1) + end + end + + context 'when using an user api key' do + it 'is counted as a user API request' do + get '/session/current.json', headers: { HTTP_USER_API_KEY: user_api_key.key } + expect(response.status).to eq(200) + + CachedCounting.flush + expect(ApplicationRequest.http_total.first.count).to eq(1) + expect(ApplicationRequest.http_2xx.first.count).to eq(1) + expect(ApplicationRequest.user_api.first.count).to eq(1) + end + end +end diff --git a/spec/lib/middleware/request_tracker_spec.rb b/spec/lib/middleware/request_tracker_spec.rb index 721c836ce40..b3953a53913 100644 --- a/spec/lib/middleware/request_tracker_spec.rb +++ b/spec/lib/middleware/request_tracker_spec.rb @@ -100,6 +100,33 @@ RSpec.describe Middleware::RequestTracker do expect(ApplicationRequest.page_view_crawler.first.count).to eq(1) end + it "logs API requests correctly" do + data = Middleware::RequestTracker.get_data( + env("_DISCOURSE_API" => "1"), ["200", { "Content-Type" => 'text/json' }], 0.1 + ) + + Middleware::RequestTracker.log_request(data) + + data = Middleware::RequestTracker.get_data( + env("_DISCOURSE_API" => "1"), ["404", { "Content-Type" => 'text/json' }], 0.1 + ) + + Middleware::RequestTracker.log_request(data) + + data = Middleware::RequestTracker.get_data( + env("_DISCOURSE_USER_API" => "1"), ["200", {}], 0.1 + ) + + Middleware::RequestTracker.log_request(data) + CachedCounting.flush + + expect(ApplicationRequest.http_total.first.count).to eq(3) + expect(ApplicationRequest.http_2xx.first.count).to eq(2) + + expect(ApplicationRequest.api.first.count).to eq(2) + expect(ApplicationRequest.user_api.first.count).to eq(1) + end + it "can log Discourse user agent requests correctly" do # log discourse api agents as crawlers for page view stats... data = Middleware::RequestTracker.get_data(env( diff --git a/spec/models/report_spec.rb b/spec/models/report_spec.rb index 930398864d8..c7e7ca7838d 100644 --- a/spec/models/report_spec.rb +++ b/spec/models/report_spec.rb @@ -1233,6 +1233,53 @@ RSpec.describe Report do end end + describe ".report_consolidated_api_requests" do + before do + freeze_time(Time.now.at_midnight) + Theme.clear_default! + end + + let(:reports) { Report.find('consolidated_api_requests') } + + context "with no data" do + it "works" do + reports.data.each do |report| + expect(report[:data]).to be_empty + end + end + end + + context "with data" do + before do + CachedCounting.reset + CachedCounting.enable + ApplicationRequest.enable + end + + after do + ApplicationRequest.disable + CachedCounting.disable + end + + it "works" do + 2.times { ApplicationRequest.increment!(:api) } + ApplicationRequest.increment!(:user_api) + + CachedCounting.flush + + api_report = reports.data.find { |r| r[:req] == "api" } + user_api_report = reports.data.find { |r| r[:req] == "user_api" } + + expect(api_report[:color]).to eql("rgba(0,136,204,1)") + expect(api_report[:data][0][:y]).to eql(2) + + expect(user_api_report[:color]).to eql("rgba(200,0,1,1)") + expect(user_api_report[:data][0][:y]).to eql(1) + ensure + end + end + end + describe "trust_level_growth" do before do freeze_time(Time.now.at_midnight)