From bc6806aa896177dbf919c03abc19d80b633cfbdc Mon Sep 17 00:00:00 2001 From: Georg Gadinger Date: Mon, 13 Feb 2023 20:13:32 +0100 Subject: [PATCH 1/6] initial metrics --- Gemfile | 2 ++ Gemfile.lock | 2 ++ app/controllers/inbox_controller.rb | 11 +++++++ app/controllers/metrics_controller.rb | 17 ++++++++++ config/routes.rb | 3 ++ lib/retrospring/metrics.rb | 36 +++++++++++++++++++++ lib/use_case/question/create.rb | 11 +++++++ lib/use_case/question/create_followers.rb | 11 +++++++ spec/controllers/metrics_controller_spec.rb | 16 +++++++++ 9 files changed, 109 insertions(+) create mode 100644 app/controllers/metrics_controller.rb create mode 100644 lib/retrospring/metrics.rb create mode 100644 spec/controllers/metrics_controller_spec.rb diff --git a/Gemfile b/Gemfile index 1b14f588..fb0afd81 100644 --- a/Gemfile +++ b/Gemfile @@ -117,3 +117,5 @@ gem "openssl", "~> 3.1" # mail 2.8.0 breaks sendmail usage: https://github.com/mikel/mail/issues/1538 gem "mail", "~> 2.7.1" + +gem "prometheus-client", "~> 4.0" diff --git a/Gemfile.lock b/Gemfile.lock index 8fd5df6d..f7d3b596 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -279,6 +279,7 @@ GEM pg (1.4.5) pghero (3.1.0) activerecord (>= 6) + prometheus-client (4.0.0) public_suffix (4.0.7) puma (6.1.0) nio4r (~> 2.0) @@ -522,6 +523,7 @@ DEPENDENCIES openssl (~> 3.1) pg pghero + prometheus-client (~> 4.0) puma pundit (~> 2.3) questiongenerator (~> 1.1) diff --git a/app/controllers/inbox_controller.rb b/app/controllers/inbox_controller.rb index d849dafc..16c47099 100644 --- a/app/controllers/inbox_controller.rb +++ b/app/controllers/inbox_controller.rb @@ -37,6 +37,7 @@ class InboxController < ApplicationController user: current_user) inbox = Inbox.create!(user: current_user, question_id: question.id, new: true) + increment_metric respond_to do |format| format.turbo_stream do @@ -85,4 +86,14 @@ class InboxController < ApplicationController # using .dup to not modify @inbox -- useful in tests @inbox&.dup&.update_all(new: false) # rubocop:disable Rails/SkipsModelValidations end + + def increment_metric + Retrospring::Metrics::QUESTIONS_ASKED.increment( + labels: { + anonymous: true, + followers: false, + generated: true, + } + ) + end end diff --git a/app/controllers/metrics_controller.rb b/app/controllers/metrics_controller.rb new file mode 100644 index 00000000..dae673a6 --- /dev/null +++ b/app/controllers/metrics_controller.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +require "prometheus/client/formats/text" + +class MetricsController < ActionController::API + include ActionController::MimeResponds + + def show + render plain: metrics + end + + private + + def metrics + Prometheus::Client::Formats::Text.marshal(Prometheus::Client.registry) + end +end diff --git a/config/routes.rb b/config/routes.rb index 744a8d83..fc210a32 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -49,6 +49,9 @@ Rails.application.routes.draw do get "/linkfilter", to: "link_filter#index", as: :linkfilter get "/manifest.json", to: "manifests#show", as: :webapp_manifest + # TODO: limit this endpoint + get "/metrics", to: "metrics#show" + # Devise routes devise_for :users, path: "user", skip: %i[sessions registrations] as :user do diff --git a/lib/retrospring/metrics.rb b/lib/retrospring/metrics.rb new file mode 100644 index 00000000..0072fd81 --- /dev/null +++ b/lib/retrospring/metrics.rb @@ -0,0 +1,36 @@ +# frozen_string_literal: true + +module Retrospring + module Metrics + PROMETHEUS = Prometheus::Client.registry + + # avoid re-registering metrics to make autoreloader happy: + class << self + %i[counter gauge histogram summary].each do |meth| + define_method meth do |name, *args, **kwargs| + PROMETHEUS.public_send(meth, name, *args, **kwargs) + rescue Prometheus::Client::Registry::AlreadyRegisteredError + raise unless Rails.env.development? + + PROMETHEUS.unregister name + retry + end + end + end + + VERSION_INFO = gauge( + :retrospring_version_info, + docstring: "Information about the currently running version", + labels: [:version], + preset_labels: { + version: Retrospring::Version.to_s, + } + ).tap { _1.set 1 } + + QUESTIONS_ASKED = counter( + :retrospring_questions_asked_total, + docstring: "How many questions got asked", + labels: %i[anonymous followers generated] + ) + end +end diff --git a/lib/use_case/question/create.rb b/lib/use_case/question/create.rb index 5cc8f174..a7c1345d 100644 --- a/lib/use_case/question/create.rb +++ b/lib/use_case/question/create.rb @@ -18,6 +18,7 @@ module UseCase return if filtered?(question) increment_asked_count + increment_metric inbox = ::Inbox.create!(user: target_user, question:, new: true) notify(inbox) @@ -92,6 +93,16 @@ module UseCase source_user.save end + def increment_metric + Retrospring::Metrics::QUESTIONS_ASKED.increment( + labels: { + anonymous:, + followers: false, + generated: false, + } + ) + end + def filtered?(question) target_user.mute_rules.any? { |rule| rule.applies_to? question } || (anonymous && AnonymousBlock.where(identifier: question.author_identifier, user_id: [target_user.id, nil]).any?) || diff --git a/lib/use_case/question/create_followers.rb b/lib/use_case/question/create_followers.rb index 21c2e89d..a6bddb0f 100644 --- a/lib/use_case/question/create_followers.rb +++ b/lib/use_case/question/create_followers.rb @@ -17,6 +17,7 @@ module UseCase ) increment_asked_count + increment_metric QuestionWorker.perform_async(source_user_id, question.id) @@ -33,6 +34,16 @@ module UseCase source_user.save end + def increment_metric + Retrospring::Metrics::QUESTIONS_ASKED.increment( + labels: { + anonymous: false, + followers: true, + generated: false, + } + ) + end + def source_user @source_user ||= ::User.find(source_user_id) end diff --git a/spec/controllers/metrics_controller_spec.rb b/spec/controllers/metrics_controller_spec.rb new file mode 100644 index 00000000..c278e308 --- /dev/null +++ b/spec/controllers/metrics_controller_spec.rb @@ -0,0 +1,16 @@ +# frozen_string_literal: true + +require "rails_helper" + +describe MetricsController, type: :controller do + describe "#show" do + subject { get :show } + + it "returns the metrics" do + # ensure we have at least a metric set + Retrospring::Metrics::VERSION_INFO.set 1 + + expect(subject.body).to include "retrospring_version_info" + end + end +end From 9fadeea3fe4bf59491950e36aeb3dadeeff7ee11 Mon Sep 17 00:00:00 2001 From: Georg Gadinger Date: Mon, 13 Feb 2023 20:14:20 +0100 Subject: [PATCH 2/6] rubodog --- app/controllers/inbox_controller.rb | 2 +- lib/use_case/question/create.rb | 4 ++-- lib/use_case/question/create_followers.rb | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/app/controllers/inbox_controller.rb b/app/controllers/inbox_controller.rb index 16c47099..f6a1aff2 100644 --- a/app/controllers/inbox_controller.rb +++ b/app/controllers/inbox_controller.rb @@ -5,7 +5,7 @@ class InboxController < ApplicationController after_action :mark_inbox_entries_as_read, only: %i[show] - def show # rubocop:disable Metrics/MethodLength, Metrics/AbcSize + def show # rubocop:disable Metrics/MethodLength find_author find_inbox_entries diff --git a/lib/use_case/question/create.rb b/lib/use_case/question/create.rb index a7c1345d..17987a1d 100644 --- a/lib/use_case/question/create.rb +++ b/lib/use_case/question/create.rb @@ -27,8 +27,8 @@ module UseCase status: 201, resource: question, extra: { - inbox: - } + inbox:, + }, } end diff --git a/lib/use_case/question/create_followers.rb b/lib/use_case/question/create_followers.rb index a6bddb0f..04fe3844 100644 --- a/lib/use_case/question/create_followers.rb +++ b/lib/use_case/question/create_followers.rb @@ -9,9 +9,9 @@ module UseCase def call question = ::Question.create!( - content: content, + content:, author_is_anonymous: false, - author_identifier: author_identifier, + author_identifier:, user: source_user, direct: false ) @@ -23,7 +23,7 @@ module UseCase { status: 201, - resource: question + resource: question, } end From c338a0c8cc889385dc33656fb730c673e51369c3 Mon Sep 17 00:00:00 2001 From: Georg Gadinger Date: Mon, 13 Feb 2023 20:36:15 +0100 Subject: [PATCH 3/6] add prometheus initialiser this ensures it works great in multi-process setups (like with puma) --- config/initializers/prometheus.rb | 17 +++++++++++++++++ 1 file changed, 17 insertions(+) create mode 100644 config/initializers/prometheus.rb diff --git a/config/initializers/prometheus.rb b/config/initializers/prometheus.rb new file mode 100644 index 00000000..7d86c92f --- /dev/null +++ b/config/initializers/prometheus.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +return if Rails.env.test? # no need for the direct file store in testing + +require "prometheus/client/data_stores/direct_file_store" + +Rails.application.config.before_configuration do + dir = Rails.root.join("tmp/prometheus_metrics") + FileUtils.mkdir_p dir + + Prometheus::Client.config.data_store = Prometheus::Client::DataStores::DirectFileStore.new(dir:) +end + +Rails.application.config.after_initialize do + # ensure the version metric is populated + Retrospring::Metrics::VERSION_INFO +end From 64adbb5707ddd5a97b06c2d77f5d5ec9f9b438f5 Mon Sep 17 00:00:00 2001 From: Georg Gadinger Date: Mon, 13 Feb 2023 21:53:40 +0100 Subject: [PATCH 4/6] allow /metrics to be reached from private subnets only this commit is not approved by the IPv6 crowd. patches welcome --- config/routes.rb | 5 +- lib/constraints/local_network.rb | 22 +++++++++ spec/lib/constraints/local_network_spec.rb | 53 ++++++++++++++++++++++ 3 files changed, 78 insertions(+), 2 deletions(-) create mode 100644 lib/constraints/local_network.rb create mode 100644 spec/lib/constraints/local_network_spec.rb diff --git a/config/routes.rb b/config/routes.rb index fc210a32..8f485cb7 100644 --- a/config/routes.rb +++ b/config/routes.rb @@ -49,8 +49,9 @@ Rails.application.routes.draw do get "/linkfilter", to: "link_filter#index", as: :linkfilter get "/manifest.json", to: "manifests#show", as: :webapp_manifest - # TODO: limit this endpoint - get "/metrics", to: "metrics#show" + constraints(Constraints::LocalNetwork) do + get "/metrics", to: "metrics#show" + end # Devise routes devise_for :users, path: "user", skip: %i[sessions registrations] diff --git a/lib/constraints/local_network.rb b/lib/constraints/local_network.rb new file mode 100644 index 00000000..2db324a5 --- /dev/null +++ b/lib/constraints/local_network.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module Constraints + module LocalNetwork + module_function + + SUBNETS = %w[ + 10.0.0.0/8 + 127.0.0.0/8 + 172.16.0.0/12 + 192.168.0.0/16 + ].map { IPAddr.new(_1) }.freeze + + def matches?(request) + SUBNETS.find do |net| + net.include? request.remote_ip + rescue + false + end + end + end +end diff --git a/spec/lib/constraints/local_network_spec.rb b/spec/lib/constraints/local_network_spec.rb new file mode 100644 index 00000000..88326ccb --- /dev/null +++ b/spec/lib/constraints/local_network_spec.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +require "rails_helper" + +describe Constraints::LocalNetwork do + describe ".matches?" do + let(:request) { double("Rack::Request", remote_ip:) } + + subject { described_class.matches?(request) } + + context "with a private address from the 10.0.0.0/8 range" do + let(:remote_ip) { "10.0.2.100" } + + it { is_expected.to be_truthy } + end + + context "with a private address from the 127.0.0.0/8 range" do + let(:remote_ip) { "127.0.0.1" } + + it { is_expected.to be_truthy } + end + + context "with a private address from the 172.16.0.0/12 range" do + let(:remote_ip) { "172.31.33.7" } + + it { is_expected.to be_truthy } + end + + context "with a private address from the 192.168.0.0/16 range" do + let(:remote_ip) { "192.168.123.45" } + + it { is_expected.to be_truthy } + end + + context "with a non-private/loopback address" do + let(:remote_ip) { "193.186.6.83" } + + it { is_expected.to be_falsey } + end + + context "with some fantasy address" do + let(:remote_ip) { "fe80:3::1ff:fe23:4567:890a" } + + it { is_expected.to be_falsey } + end + + context "with an actual invalid address" do + let(:remote_ip) { "herbert" } + + it { is_expected.to be_falsey } + end + end +end From b937a10096e008e741fa6283f7fab50898cf7b3a Mon Sep 17 00:00:00 2001 From: Georg Gadinger Date: Mon, 13 Feb 2023 22:24:52 +0100 Subject: [PATCH 5/6] add some more metrics for answers and comments --- app/models/user.rb | 4 ++++ lib/retrospring/metrics.rb | 12 +++++++++++- 2 files changed, 15 insertions(+), 1 deletion(-) diff --git a/app/models/user.rb b/app/models/user.rb index 6370ed01..725582ef 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -115,6 +115,8 @@ class User < ApplicationRecord # rubocop:disable Metrics/ClassLength raise Errors::AnsweringSelfBlockedOther if self.blocking?(question.user) # rubocop:enable Style/RedundantSelf + Retrospring::Metrics::QUESTIONS_ANSWERED.increment + Answer.create!(content:, user: self, question:) end @@ -128,6 +130,8 @@ class User < ApplicationRecord # rubocop:disable Metrics/ClassLength raise Errors::CommentingOtherBlockedSelf if answer.user.blocking?(self) # rubocop:enable Style/RedundantSelf + Retrospring::Metrics::COMMENTS_CREATED.increment + Comment.create!(user: self, answer:, content:) end diff --git a/lib/retrospring/metrics.rb b/lib/retrospring/metrics.rb index 0072fd81..7259f33f 100644 --- a/lib/retrospring/metrics.rb +++ b/lib/retrospring/metrics.rb @@ -4,7 +4,7 @@ module Retrospring module Metrics PROMETHEUS = Prometheus::Client.registry - # avoid re-registering metrics to make autoreloader happy: + # avoid re-registering metrics to make autoreloader happy during dev: class << self %i[counter gauge histogram summary].each do |meth| define_method meth do |name, *args, **kwargs| @@ -32,5 +32,15 @@ module Retrospring docstring: "How many questions got asked", labels: %i[anonymous followers generated] ) + + QUESTIONS_ANSWERED = counter( + :retrospring_questions_answered_total, + docstring: "How many questions got answered" + ) + + COMMENTS_CREATED = counter( + :retrospring_comments_created_total, + docstring: "How many comments got created" + ) end end From 8a055341c8de09214789ec51c4c68f1e2f92ade2 Mon Sep 17 00:00:00 2001 From: Georg Gadinger Date: Tue, 14 Feb 2023 05:30:40 +0100 Subject: [PATCH 6/6] add metrics for sidekiq --- app/controllers/metrics_controller.rb | 24 ++++++++++++++- lib/retrospring/metrics.rb | 33 +++++++++++++++++++++ spec/controllers/metrics_controller_spec.rb | 3 -- 3 files changed, 56 insertions(+), 4 deletions(-) diff --git a/app/controllers/metrics_controller.rb b/app/controllers/metrics_controller.rb index dae673a6..257b8b7e 100644 --- a/app/controllers/metrics_controller.rb +++ b/app/controllers/metrics_controller.rb @@ -6,12 +6,34 @@ class MetricsController < ActionController::API include ActionController::MimeResponds def show + fetch_sidekiq_metrics + render plain: metrics end private + SIDEKIQ_STATS_METHODS = %i[ + processed + failed + scheduled_size + retry_size + dead_size + processes_size + ].freeze + + def fetch_sidekiq_metrics + stats = Sidekiq::Stats.new + SIDEKIQ_STATS_METHODS.each do |key| + Retrospring::Metrics::SIDEKIQ[key].set stats.public_send(key) + end + + stats.queues.each do |queue, value| + Retrospring::Metrics::SIDEKIQ[:queue_enqueued].set value, labels: { queue: } + end + end + def metrics - Prometheus::Client::Formats::Text.marshal(Prometheus::Client.registry) + Prometheus::Client::Formats::Text.marshal(Retrospring::Metrics::PROMETHEUS) end end diff --git a/lib/retrospring/metrics.rb b/lib/retrospring/metrics.rb index 7259f33f..7c52dcb6 100644 --- a/lib/retrospring/metrics.rb +++ b/lib/retrospring/metrics.rb @@ -42,5 +42,38 @@ module Retrospring :retrospring_comments_created_total, docstring: "How many comments got created" ) + + # metrics from Sidekiq::Stats.new + SIDEKIQ = { + processed: gauge( + :sidekiq_processed, + docstring: "Number of jobs processed by Sidekiq" + ), + failed: gauge( + :sidekiq_failed, + docstring: "Number of jobs that failed" + ), + scheduled_size: gauge( + :sidekiq_scheduled_jobs, + docstring: "Number of jobs that are enqueued" + ), + retry_size: gauge( + :sidekiq_retried_jobs, + docstring: "Number of jobs that are being retried" + ), + dead_size: gauge( + :sidekiq_dead_jobs, + docstring: "Number of jobs that are dead" + ), + processes_size: gauge( + :sidekiq_processes, + docstring: "Number of active Sidekiq processes" + ), + queue_enqueued: gauge( + :sidekiq_queues_enqueued, + docstring: "Number of enqueued jobs per queue", + labels: %i[queue] + ), + }.freeze end end diff --git a/spec/controllers/metrics_controller_spec.rb b/spec/controllers/metrics_controller_spec.rb index c278e308..89f4316e 100644 --- a/spec/controllers/metrics_controller_spec.rb +++ b/spec/controllers/metrics_controller_spec.rb @@ -7,9 +7,6 @@ describe MetricsController, type: :controller do subject { get :show } it "returns the metrics" do - # ensure we have at least a metric set - Retrospring::Metrics::VERSION_INFO.set 1 - expect(subject.body).to include "retrospring_version_info" end end