From 9955d00a10ad19c96dd9737718ae9e6ba3faaf45 Mon Sep 17 00:00:00 2001 From: Sean Rankine Date: Thu, 4 Jun 2026 19:36:15 +0100 Subject: [PATCH 1/5] Add service to publish submission counts to CloudWatch Publish SubmissionCount metrics grouped by form for KPI reporting. --- .../metrics/submission_count_service.rb | 63 +++++++++++++++ .../metrics/submission_count_service_spec.rb | 76 +++++++++++++++++++ 2 files changed, 139 insertions(+) create mode 100644 app/services/metrics/submission_count_service.rb create mode 100644 spec/services/metrics/submission_count_service_spec.rb diff --git a/app/services/metrics/submission_count_service.rb b/app/services/metrics/submission_count_service.rb new file mode 100644 index 000000000..722701ace --- /dev/null +++ b/app/services/metrics/submission_count_service.rb @@ -0,0 +1,63 @@ +module Metrics + class SubmissionCountService + METRIC_NAME = "SubmissionCount".freeze + METRICS_NAMESPACE = CloudWatchService::FORM_METRICS_NAMESPACE + REGION = CloudWatchService::REGION + BATCH_SIZE = 500 + + def publish_submission_counts + metric_count = 0 + + submission_counts_by_form_id.each_slice(BATCH_SIZE) do |batch| + cloudwatch_client.put_metric_data( + namespace: METRICS_NAMESPACE, + metric_data: batch.map { |(form_id, count)| metric_datum(form_id:, count:) }, + ) + metric_count += batch.size + end + + Rails.logger.info "Published #{metric_count} submission count metrics to CloudWatch" + rescue Aws::CloudWatch::Errors::ServiceError, + Aws::Errors::MissingCredentialsError => e + Sentry.capture_exception(e) + raise + end + + private + + def submission_counts_by_form_id + Submission.where(mode: "form").group(:form_id).count + end + + def metric_datum(form_id:, count:) + { + metric_name: METRIC_NAME, + dimensions: [ + environment_dimension, + form_id_dimension(form_id), + ], + value: count, + unit: "Count", + timestamp: Time.zone.now, + } + end + + def environment_dimension + { + name: "Environment", + value: Settings.forms_env.downcase, + } + end + + def form_id_dimension(form_id) + { + name: "FormId", + value: form_id.to_s, + } + end + + def cloudwatch_client + @cloudwatch_client ||= Aws::CloudWatch::Client.new(region: REGION) + end + end +end diff --git a/spec/services/metrics/submission_count_service_spec.rb b/spec/services/metrics/submission_count_service_spec.rb new file mode 100644 index 000000000..a10604930 --- /dev/null +++ b/spec/services/metrics/submission_count_service_spec.rb @@ -0,0 +1,76 @@ +require "rails_helper" +require "aws-sdk-cloudwatch" + +describe Metrics::SubmissionCountService do + subject(:service) { described_class.new } + + let(:forms_env) { "test" } + let(:cloud_watch_client) { Aws::CloudWatch::Client.new(stub_responses: true) } + let(:form_id) { 42 } + let(:other_form_id) { 99 } + + before do + allow(Settings).to receive(:forms_env).and_return(forms_env) + allow(Aws::CloudWatch::Client).to receive(:new).and_return(cloud_watch_client) + end + + around do |example| + travel_to(Time.zone.local(2026, 6, 3, 12, 0, 0)) do + example.run + end + end + + describe "#publish_submission_counts" do + before do + create_list(:submission, 2, form_id:) + create(:submission, form_id: other_form_id) + create(:submission, :preview, form_id:) + create(:submission, form_id: 123) + end + + it "publishes grouped submission counts to CloudWatch" do + expect(cloud_watch_client).to receive(:put_metric_data).with( + namespace: "Forms", + metric_data: contain_exactly( + metric_datum(form_id:, count: 2), + metric_datum(form_id: other_form_id, count: 1), + metric_datum(form_id: 123, count: 1), + ), + ) + + service.publish_submission_counts + end + + context "when CloudWatch returns an error" do + before do + allow(cloud_watch_client).to receive(:put_metric_data) + .and_raise(Aws::CloudWatch::Errors::ServiceError.new(nil, "CloudWatch error", nil)) + end + + it "captures the exception and re-raises" do + expect(Sentry).to receive(:capture_exception).with(instance_of(Aws::CloudWatch::Errors::ServiceError)) + + expect { service.publish_submission_counts }.to raise_error(Aws::CloudWatch::Errors::ServiceError) + end + end + end + + def metric_datum(form_id:, count:) + { + metric_name: "SubmissionCount", + dimensions: [ + { + name: "Environment", + value: forms_env, + }, + { + name: "FormId", + value: form_id.to_s, + }, + ], + value: count, + unit: "Count", + timestamp: Time.zone.local(2026, 6, 3, 12, 0, 0), + } + end +end From ca555eced8071c8863057894cdcd2b830df753c6 Mon Sep 17 00:00:00 2001 From: Sean Rankine Date: Thu, 4 Jun 2026 19:36:18 +0100 Subject: [PATCH 2/5] Add rake task to export submission counts to CloudWatch Add metrics:export_submission_counts for scheduled KPI publishing. --- lib/tasks/metrics.rake | 6 ++++++ spec/lib/tasks/metrics.rake_spec.rb | 17 +++++++++++++++++ 2 files changed, 23 insertions(+) create mode 100644 lib/tasks/metrics.rake create mode 100644 spec/lib/tasks/metrics.rake_spec.rb diff --git a/lib/tasks/metrics.rake b/lib/tasks/metrics.rake new file mode 100644 index 000000000..f9654ce3c --- /dev/null +++ b/lib/tasks/metrics.rake @@ -0,0 +1,6 @@ +namespace :metrics do + desc "Export submission counts to CloudWatch metrics grouped by form" + task export_submission_counts: :environment do + Metrics::SubmissionCountService.new.publish_submission_counts + end +end diff --git a/spec/lib/tasks/metrics.rake_spec.rb b/spec/lib/tasks/metrics.rake_spec.rb new file mode 100644 index 000000000..0a542d26d --- /dev/null +++ b/spec/lib/tasks/metrics.rake_spec.rb @@ -0,0 +1,17 @@ +require "rails_helper" + +RSpec.describe "metrics.rake", type: :task do + describe "metrics:export_submission_counts" do + subject(:task) do + Rake::Task["metrics:export_submission_counts"] + end + + it "publishes submission counts via Metrics::SubmissionCountService" do + service = instance_double(Metrics::SubmissionCountService) + allow(Metrics::SubmissionCountService).to receive(:new).and_return(service) + expect(service).to receive(:publish_submission_counts) + + task.invoke + end + end +end From 9494591dace0ee6a77a8cc5ef47a8b7941420208 Mon Sep 17 00:00:00 2001 From: Sean Rankine Date: Wed, 17 Jun 2026 15:07:49 +0100 Subject: [PATCH 3/5] wip --- .../metrics/submission_count_service.rb | 21 ++++- .../metrics/submission_count_service_spec.rb | 83 ++++++++++++++----- 2 files changed, 79 insertions(+), 25 deletions(-) diff --git a/app/services/metrics/submission_count_service.rb b/app/services/metrics/submission_count_service.rb index 722701ace..2d9db4f39 100644 --- a/app/services/metrics/submission_count_service.rb +++ b/app/services/metrics/submission_count_service.rb @@ -7,11 +7,12 @@ class SubmissionCountService def publish_submission_counts metric_count = 0 + form_names = latest_form_names_by_form_id submission_counts_by_form_id.each_slice(BATCH_SIZE) do |batch| cloudwatch_client.put_metric_data( namespace: METRICS_NAMESPACE, - metric_data: batch.map { |(form_id, count)| metric_datum(form_id:, count:) }, + metric_data: batch.map { |(form_id, count)| metric_datum(form_id:, form_name: form_names[form_id], count:) }, ) metric_count += batch.size end @@ -29,12 +30,21 @@ def submission_counts_by_form_id Submission.where(mode: "form").group(:form_id).count end - def metric_datum(form_id:, count:) + def latest_form_names_by_form_id + Submission + .where(mode: "form") + .order(Arel.sql("form_id, created_at DESC")) + .pluck(Arel.sql("DISTINCT ON (form_id) form_id"), Arel.sql("form_document->>'name'")) + .to_h + end + + def metric_datum(form_id:, form_name:, count:) { metric_name: METRIC_NAME, dimensions: [ environment_dimension, form_id_dimension(form_id), + form_name_dimension(form_name), ], value: count, unit: "Count", @@ -56,6 +66,13 @@ def form_id_dimension(form_id) } end + def form_name_dimension(form_name) + { + name: "FormName", + value: form_name.to_s, + } + end + def cloudwatch_client @cloudwatch_client ||= Aws::CloudWatch::Client.new(region: REGION) end diff --git a/spec/services/metrics/submission_count_service_spec.rb b/spec/services/metrics/submission_count_service_spec.rb index a10604930..6cfecc536 100644 --- a/spec/services/metrics/submission_count_service_spec.rb +++ b/spec/services/metrics/submission_count_service_spec.rb @@ -8,6 +8,9 @@ let(:cloud_watch_client) { Aws::CloudWatch::Client.new(stub_responses: true) } let(:form_id) { 42 } let(:other_form_id) { 99 } + let(:form_document) { build(:v2_form_document, form_id:, name: "Test Form") } + let(:other_form_document) { build(:v2_form_document, form_id: other_form_id, name: "Other Form") } + let(:third_form_document) { build(:v2_form_document, form_id: 123, name: "Third Form") } before do allow(Settings).to receive(:forms_env).and_return(forms_env) @@ -21,41 +24,71 @@ end describe "#publish_submission_counts" do - before do - create_list(:submission, 2, form_id:) - create(:submission, form_id: other_form_id) - create(:submission, :preview, form_id:) - create(:submission, form_id: 123) - end + context "with submissions for multiple forms" do + before do + create_list(:submission, 2, form_id:, form_document:) + create(:submission, form_id: other_form_id, form_document: other_form_document) + create(:submission, :preview, form_id:, form_document:) + create(:submission, form_id: 123, form_document: third_form_document) + end + + it "publishes grouped submission counts to CloudWatch" do + expect(cloud_watch_client).to receive(:put_metric_data).with( + namespace: "Forms", + metric_data: contain_exactly( + metric_datum(form_id:, form_name: "Test Form", count: 2), + metric_datum(form_id: other_form_id, form_name: "Other Form", count: 1), + metric_datum(form_id: 123, form_name: "Third Form", count: 1), + ), + ) + + service.publish_submission_counts + end + + context "when CloudWatch returns an error" do + before do + allow(cloud_watch_client).to receive(:put_metric_data) + .and_raise(Aws::CloudWatch::Errors::ServiceError.new(nil, "CloudWatch error", nil)) + end - it "publishes grouped submission counts to CloudWatch" do - expect(cloud_watch_client).to receive(:put_metric_data).with( - namespace: "Forms", - metric_data: contain_exactly( - metric_datum(form_id:, count: 2), - metric_datum(form_id: other_form_id, count: 1), - metric_datum(form_id: 123, count: 1), - ), - ) + it "captures the exception and re-raises" do + expect(Sentry).to receive(:capture_exception).with(instance_of(Aws::CloudWatch::Errors::ServiceError)) - service.publish_submission_counts + expect { service.publish_submission_counts }.to raise_error(Aws::CloudWatch::Errors::ServiceError) + end + end end - context "when CloudWatch returns an error" do + context "when a form has been renamed" do before do - allow(cloud_watch_client).to receive(:put_metric_data) - .and_raise(Aws::CloudWatch::Errors::ServiceError.new(nil, "CloudWatch error", nil)) + create( + :submission, + form_id:, + form_document: build(:v2_form_document, form_id:, name: "Older Form Name"), + created_at: 2.days.ago, + ) + create( + :submission, + form_id:, + form_document: build(:v2_form_document, form_id:, name: "Latest Form Name"), + created_at: 1.minute.ago, + ) end - it "captures the exception and re-raises" do - expect(Sentry).to receive(:capture_exception).with(instance_of(Aws::CloudWatch::Errors::ServiceError)) + it "uses the latest form name from the most recent submission" do + expect(cloud_watch_client).to receive(:put_metric_data).with( + namespace: "Forms", + metric_data: include( + metric_datum(form_id:, form_name: "Latest Form Name", count: 2), + ), + ) - expect { service.publish_submission_counts }.to raise_error(Aws::CloudWatch::Errors::ServiceError) + service.publish_submission_counts end end end - def metric_datum(form_id:, count:) + def metric_datum(form_id:, form_name:, count:) { metric_name: "SubmissionCount", dimensions: [ @@ -67,6 +100,10 @@ def metric_datum(form_id:, count:) name: "FormId", value: form_id.to_s, }, + { + name: "FormName", + value: form_name, + }, ], value: count, unit: "Count", From 76eb56dcf96b02c3d1bba42e889cfde644aa6d70 Mon Sep 17 00:00:00 2001 From: Sean Rankine Date: Thu, 18 Jun 2026 11:20:48 +0100 Subject: [PATCH 4/5] wip --- Gemfile | 2 + Gemfile.lock | 20 ++++ .../metrics/submission_count_service.rb | 88 ++++++++------- config/initializers/opentelemetry.rb | 4 + lib/tasks/metrics.rake | 2 +- .../metrics/submission_count_service_spec.rb | 103 +++++++++--------- 6 files changed, 127 insertions(+), 92 deletions(-) diff --git a/Gemfile b/Gemfile index fda0d60af..37fdda4f1 100644 --- a/Gemfile +++ b/Gemfile @@ -62,7 +62,9 @@ gem "lograge" # For distributed tracing and telemetry gem "opentelemetry-exporter-otlp", "~> 0.34.0" +gem "opentelemetry-exporter-otlp-metrics", "~> 0.10.0" gem "opentelemetry-instrumentation-all", "~> 0.94.0" +gem "opentelemetry-metrics-sdk", "~> 0.15.0" gem "opentelemetry-propagator-xray", "~> 0.27.0" gem "opentelemetry-sdk", "~> 1.12" diff --git a/Gemfile.lock b/Gemfile.lock index 532099365..b8292b33b 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -373,6 +373,15 @@ GEM opentelemetry-common (~> 0.20) opentelemetry-sdk (~> 1.10) opentelemetry-semantic_conventions + opentelemetry-exporter-otlp-metrics (0.10.0) + google-protobuf (>= 3.18, < 5.0) + googleapis-common-protos-types (~> 1.3) + opentelemetry-api (~> 1.1) + opentelemetry-common (~> 0.20) + opentelemetry-metrics-api (~> 0.2) + opentelemetry-metrics-sdk (~> 0.5) + opentelemetry-sdk (~> 1.2) + opentelemetry-semantic_conventions opentelemetry-helpers-mysql (0.6.0) opentelemetry-api (~> 1.7) opentelemetry-common (~> 0.21) @@ -527,6 +536,12 @@ GEM opentelemetry-helpers-sql-processor opentelemetry-instrumentation-base (~> 0.25) opentelemetry-semantic_conventions (>= 1.8.0) + opentelemetry-metrics-api (0.6.0) + opentelemetry-api (~> 1.0) + opentelemetry-metrics-sdk (0.15.0) + opentelemetry-api (~> 1.1) + opentelemetry-metrics-api (~> 0.2) + opentelemetry-sdk (~> 1.2) opentelemetry-propagator-xray (0.27.0) opentelemetry-api (~> 1.7) opentelemetry-registry (0.6.0) @@ -800,7 +815,9 @@ DEPENDENCIES omniauth-rails_csrf_protection omniauth_govuk_one_login! opentelemetry-exporter-otlp (~> 0.34.0) + opentelemetry-exporter-otlp-metrics (~> 0.10.0) opentelemetry-instrumentation-all (~> 0.94.0) + opentelemetry-metrics-sdk (~> 0.15.0) opentelemetry-propagator-xray (~> 0.27.0) opentelemetry-sdk (~> 1.12) pg (~> 1.6) @@ -947,6 +964,7 @@ CHECKSUMS opentelemetry-api (1.10.0) sha256=99ee7c829b18381c31a817ee9bf6a160d737542d99cb8da55d443336d266bfa9 opentelemetry-common (0.25.0) sha256=73915362e58d337fc92acbe1abfdaee1f725442527125fdb2af1420417f1149d opentelemetry-exporter-otlp (0.34.0) sha256=3b3cdf4329ba30f4389d849c7f13b8f9f983ecb4a030031c03997dffae1e2a60 + opentelemetry-exporter-otlp-metrics (0.10.0) sha256=d8cbff9b8a3391eb61486b8be9b6ad74e3b9306a3c60fb4c906b28bc857167c8 opentelemetry-helpers-mysql (0.6.0) sha256=7eeb5e6950c434775a8cf28b5fde4defc12e8b865c86479ce3119fcf593d9337 opentelemetry-helpers-sql (0.4.0) sha256=b10e8c3a2cca28a98af951bbb3e4efdc59e68b25ba0825e055574af543420afb opentelemetry-helpers-sql-processor (0.5.0) sha256=b199241bc9451fcbd9f00b2f454830af19d4ca27c2219ea379c9b0d53cd0e0f1 @@ -996,6 +1014,8 @@ CHECKSUMS opentelemetry-instrumentation-sidekiq (0.29.0) sha256=b1d2a0cb9041a5e14239fe7c94d99e3dd07f870e2759460ab63592d7cdd8aadc opentelemetry-instrumentation-sinatra (0.30.0) sha256=b67301153420f43264a0c68cdb3ca5bd77467cf5054e57b83a2bf891aaaa0361 opentelemetry-instrumentation-trilogy (0.69.0) sha256=0676dd720eeab284abfa52f273967442156fcac7084a1e1411373cf14ec026ad + opentelemetry-metrics-api (0.6.0) sha256=b9300821680a1370684098cb030c18423dd55909ea0206faadfa7bc47362df87 + opentelemetry-metrics-sdk (0.15.0) sha256=611a9cd9f473c461095c7401b8c25f9774160d286a1acbfcbf044da2972aeada opentelemetry-propagator-xray (0.27.0) sha256=753f756c7ad3146f182d428b06041084eecc77769edfd280f365e0bc09b9c4d1 opentelemetry-registry (0.6.0) sha256=5d3ed32ab9eee0fbdb30d4f0d0bb61ad11a4040b267b475ae815b80a8498a728 opentelemetry-sdk (1.12.0) sha256=a224abe0c59023d41cb7ac1c634d9d28843907efcd045ed1ae320796c48b864b diff --git a/app/services/metrics/submission_count_service.rb b/app/services/metrics/submission_count_service.rb index 2d9db4f39..96d648d77 100644 --- a/app/services/metrics/submission_count_service.rb +++ b/app/services/metrics/submission_count_service.rb @@ -1,80 +1,86 @@ module Metrics class SubmissionCountService + class ExportError < StandardError; end + METRIC_NAME = "SubmissionCount".freeze - METRICS_NAMESPACE = CloudWatchService::FORM_METRICS_NAMESPACE - REGION = CloudWatchService::REGION - BATCH_SIZE = 500 + METER_NAME = "forms-runner".freeze + METER_VERSION = "1.0".freeze + PERIOD = 5.minutes + + def initialize(meter_provider: OpenTelemetry.meter_provider) + @meter_provider = meter_provider + end def publish_submission_counts metric_count = 0 form_names = latest_form_names_by_form_id - submission_counts_by_form_id.each_slice(BATCH_SIZE) do |batch| - cloudwatch_client.put_metric_data( - namespace: METRICS_NAMESPACE, - metric_data: batch.map { |(form_id, count)| metric_datum(form_id:, form_name: form_names[form_id], count:) }, + submission_counts_by_form_id.each do |form_id, count| + submission_count_counter.add( + count, + attributes: metric_attributes(form_id:, form_name: form_names[form_id]), ) - metric_count += batch.size + metric_count += 1 end - Rails.logger.info "Published #{metric_count} submission count metrics to CloudWatch" - rescue Aws::CloudWatch::Errors::ServiceError, - Aws::Errors::MissingCredentialsError => e + export_metrics! + + Rails.logger.info "Published #{metric_count} submission count metrics via OpenTelemetry" + rescue ExportError => e Sentry.capture_exception(e) raise end private + attr_reader :meter_provider + def submission_counts_by_form_id - Submission.where(mode: "form").group(:form_id).count + submissions_in_period.group(:form_id).count end def latest_form_names_by_form_id - Submission - .where(mode: "form") + submissions_in_period .order(Arel.sql("form_id, created_at DESC")) .pluck(Arel.sql("DISTINCT ON (form_id) form_id"), Arel.sql("form_document->>'name'")) .to_h end - def metric_datum(form_id:, form_name:, count:) - { - metric_name: METRIC_NAME, - dimensions: [ - environment_dimension, - form_id_dimension(form_id), - form_name_dimension(form_name), - ], - value: count, - unit: "Count", - timestamp: Time.zone.now, - } + def submissions_in_period + Submission.where(mode: "form", created_at: period_range) end - def environment_dimension - { - name: "Environment", - value: Settings.forms_env.downcase, - } + def period_range + PERIOD.ago..Time.current end - def form_id_dimension(form_id) - { - name: "FormId", - value: form_id.to_s, - } + def submission_count_counter + @submission_count_counter ||= meter.create_counter( + METRIC_NAME, + unit: "1", + description: "Number of form submissions in the export period", + ) end - def form_name_dimension(form_name) + def meter + meter_provider.meter(METER_NAME, version: METER_VERSION) + end + + def metric_attributes(form_id:, form_name:) { - name: "FormName", - value: form_name.to_s, + "Environment" => Settings.forms_env.downcase, + "FormId" => form_id.to_s, + "FormName" => form_name.to_s, } end - def cloudwatch_client - @cloudwatch_client ||= Aws::CloudWatch::Client.new(region: REGION) + def export_metrics! + return if meter_provider.metric_readers.empty? + + result = meter_provider.force_flush + return if result == OpenTelemetry::SDK::Metrics::Export::SUCCESS + + raise ExportError, "Failed to export submission count metrics (status: #{result})" end end end diff --git a/config/initializers/opentelemetry.rb b/config/initializers/opentelemetry.rb index eda8872b3..f849e1cb1 100644 --- a/config/initializers/opentelemetry.rb +++ b/config/initializers/opentelemetry.rb @@ -1,8 +1,12 @@ require "opentelemetry/sdk" require "opentelemetry/instrumentation/all" +require "opentelemetry-metrics-sdk" +require "opentelemetry-exporter-otlp-metrics" return unless ENV["ENABLE_OTEL"] == "true" +ENV["OTEL_METRICS_EXPORTER"] ||= "otlp" + OpenTelemetry::SDK.configure do |c| instrumentation_config = { "OpenTelemetry::Instrumentation::Rack" => { untraced_endpoints: ["/up"] } } c.use_all(instrumentation_config) diff --git a/lib/tasks/metrics.rake b/lib/tasks/metrics.rake index f9654ce3c..6caf7c544 100644 --- a/lib/tasks/metrics.rake +++ b/lib/tasks/metrics.rake @@ -1,5 +1,5 @@ namespace :metrics do - desc "Export submission counts to CloudWatch metrics grouped by form" + desc "Export submission counts for the last 5 minutes as OpenTelemetry metrics grouped by form" task export_submission_counts: :environment do Metrics::SubmissionCountService.new.publish_submission_counts end diff --git a/spec/services/metrics/submission_count_service_spec.rb b/spec/services/metrics/submission_count_service_spec.rb index 6cfecc536..9c27a3dc5 100644 --- a/spec/services/metrics/submission_count_service_spec.rb +++ b/spec/services/metrics/submission_count_service_spec.rb @@ -1,11 +1,12 @@ require "rails_helper" -require "aws-sdk-cloudwatch" +require "opentelemetry-metrics-sdk" describe Metrics::SubmissionCountService do - subject(:service) { described_class.new } + subject(:service) { described_class.new(meter_provider:) } + let(:meter_provider) { OpenTelemetry::SDK::Metrics::MeterProvider.new } + let(:metric_exporter) { OpenTelemetry::SDK::Metrics::Export::InMemoryMetricPullExporter.new } let(:forms_env) { "test" } - let(:cloud_watch_client) { Aws::CloudWatch::Client.new(stub_responses: true) } let(:form_id) { 42 } let(:other_form_id) { 99 } let(:form_document) { build(:v2_form_document, form_id:, name: "Test Form") } @@ -14,7 +15,7 @@ before do allow(Settings).to receive(:forms_env).and_return(forms_env) - allow(Aws::CloudWatch::Client).to receive(:new).and_return(cloud_watch_client) + meter_provider.add_metric_reader(metric_exporter) end around do |example| @@ -24,7 +25,7 @@ end describe "#publish_submission_counts" do - context "with submissions for multiple forms" do + context "with submissions for multiple forms in the export period" do before do create_list(:submission, 2, form_id:, form_document:) create(:submission, form_id: other_form_id, form_document: other_form_document) @@ -32,40 +33,52 @@ create(:submission, form_id: 123, form_document: third_form_document) end - it "publishes grouped submission counts to CloudWatch" do - expect(cloud_watch_client).to receive(:put_metric_data).with( - namespace: "Forms", - metric_data: contain_exactly( - metric_datum(form_id:, form_name: "Test Form", count: 2), - metric_datum(form_id: other_form_id, form_name: "Other Form", count: 1), - metric_datum(form_id: 123, form_name: "Third Form", count: 1), - ), - ) - + it "publishes grouped submission counts for the period via OpenTelemetry" do service.publish_submission_counts + + expect(exported_data_points).to contain_exactly( + data_point(form_id:, form_name: "Test Form", count: 2), + data_point(form_id: other_form_id, form_name: "Other Form", count: 1), + data_point(form_id: 123, form_name: "Third Form", count: 1), + ) end - context "when CloudWatch returns an error" do + context "when metric export fails" do before do - allow(cloud_watch_client).to receive(:put_metric_data) - .and_raise(Aws::CloudWatch::Errors::ServiceError.new(nil, "CloudWatch error", nil)) + allow(meter_provider).to receive(:force_flush) + .and_return(OpenTelemetry::SDK::Metrics::Export::FAILURE) end it "captures the exception and re-raises" do - expect(Sentry).to receive(:capture_exception).with(instance_of(Aws::CloudWatch::Errors::ServiceError)) + expect(Sentry).to receive(:capture_exception).with(instance_of(Metrics::SubmissionCountService::ExportError)) - expect { service.publish_submission_counts }.to raise_error(Aws::CloudWatch::Errors::ServiceError) + expect { service.publish_submission_counts }.to raise_error(Metrics::SubmissionCountService::ExportError) end end end - context "when a form has been renamed" do + context "with submissions outside the export period" do + before do + create(:submission, form_id:, form_document:, created_at: 10.minutes.ago) + create(:submission, form_id:, form_document:, created_at: 4.minutes.ago) + end + + it "only counts submissions within the last 5 minutes" do + service.publish_submission_counts + + expect(exported_data_points).to contain_exactly( + data_point(form_id:, form_name: "Test Form", count: 1), + ) + end + end + + context "when a form has been renamed within the export period" do before do create( :submission, form_id:, form_document: build(:v2_form_document, form_id:, name: "Older Form Name"), - created_at: 2.days.ago, + created_at: 4.minutes.ago, ) create( :submission, @@ -75,39 +88,29 @@ ) end - it "uses the latest form name from the most recent submission" do - expect(cloud_watch_client).to receive(:put_metric_data).with( - namespace: "Forms", - metric_data: include( - metric_datum(form_id:, form_name: "Latest Form Name", count: 2), - ), - ) - + it "uses the latest form name from the most recent submission in the period" do service.publish_submission_counts + + expect(exported_data_points).to include( + data_point(form_id:, form_name: "Latest Form Name", count: 2), + ) end end end - def metric_datum(form_id:, form_name:, count:) - { - metric_name: "SubmissionCount", - dimensions: [ - { - name: "Environment", - value: forms_env, - }, - { - name: "FormId", - value: form_id.to_s, - }, - { - name: "FormName", - value: form_name, - }, - ], + def exported_data_points + metric_exporter.pull + metric_exporter.metric_snapshots.flat_map(&:data_points) + end + + def data_point(form_id:, form_name:, count:) + have_attributes( value: count, - unit: "Count", - timestamp: Time.zone.local(2026, 6, 3, 12, 0, 0), - } + attributes: { + "Environment" => forms_env, + "FormId" => form_id.to_s, + "FormName" => form_name, + }, + ) end end From 70c1c91c0287cd2e04fc3536aef21d77f1a24965 Mon Sep 17 00:00:00 2001 From: Sean Rankine Date: Thu, 18 Jun 2026 15:18:54 +0100 Subject: [PATCH 5/5] Record submission counts with OpenTelemetry counter Replace the periodic database aggregation rake task with an incrementing counter at submission creation time, and export metrics via PeriodicMetricReader. --- app/services/form_submission_service.rb | 2 + .../metrics/submission_count_service.rb | 86 ------------- app/services/metrics/submission_counter.rb | 44 +++++++ config/initializers/opentelemetry.rb | 10 +- lib/tasks/metrics.rake | 6 - spec/lib/tasks/metrics.rake_spec.rb | 17 --- spec/services/form_submission_service_spec.rb | 10 ++ .../metrics/submission_count_service_spec.rb | 116 ------------------ .../metrics/submission_counter_spec.rb | 88 +++++++++++++ 9 files changed, 152 insertions(+), 227 deletions(-) delete mode 100644 app/services/metrics/submission_count_service.rb create mode 100644 app/services/metrics/submission_counter.rb delete mode 100644 lib/tasks/metrics.rake delete mode 100644 spec/lib/tasks/metrics.rake_spec.rb delete mode 100644 spec/services/metrics/submission_count_service_spec.rb create mode 100644 spec/services/metrics/submission_counter_spec.rb diff --git a/app/services/form_submission_service.rb b/app/services/form_submission_service.rb index c2b15b6b1..9b3410bb7 100644 --- a/app/services/form_submission_service.rb +++ b/app/services/form_submission_service.rb @@ -104,6 +104,8 @@ def create_submission_record submission.deliveries.create!(delivery_schedule: :immediate) + Metrics::SubmissionCounter.record(form_id: form.id, form_name: form.name, mode:) + submission end diff --git a/app/services/metrics/submission_count_service.rb b/app/services/metrics/submission_count_service.rb deleted file mode 100644 index 96d648d77..000000000 --- a/app/services/metrics/submission_count_service.rb +++ /dev/null @@ -1,86 +0,0 @@ -module Metrics - class SubmissionCountService - class ExportError < StandardError; end - - METRIC_NAME = "SubmissionCount".freeze - METER_NAME = "forms-runner".freeze - METER_VERSION = "1.0".freeze - PERIOD = 5.minutes - - def initialize(meter_provider: OpenTelemetry.meter_provider) - @meter_provider = meter_provider - end - - def publish_submission_counts - metric_count = 0 - form_names = latest_form_names_by_form_id - - submission_counts_by_form_id.each do |form_id, count| - submission_count_counter.add( - count, - attributes: metric_attributes(form_id:, form_name: form_names[form_id]), - ) - metric_count += 1 - end - - export_metrics! - - Rails.logger.info "Published #{metric_count} submission count metrics via OpenTelemetry" - rescue ExportError => e - Sentry.capture_exception(e) - raise - end - - private - - attr_reader :meter_provider - - def submission_counts_by_form_id - submissions_in_period.group(:form_id).count - end - - def latest_form_names_by_form_id - submissions_in_period - .order(Arel.sql("form_id, created_at DESC")) - .pluck(Arel.sql("DISTINCT ON (form_id) form_id"), Arel.sql("form_document->>'name'")) - .to_h - end - - def submissions_in_period - Submission.where(mode: "form", created_at: period_range) - end - - def period_range - PERIOD.ago..Time.current - end - - def submission_count_counter - @submission_count_counter ||= meter.create_counter( - METRIC_NAME, - unit: "1", - description: "Number of form submissions in the export period", - ) - end - - def meter - meter_provider.meter(METER_NAME, version: METER_VERSION) - end - - def metric_attributes(form_id:, form_name:) - { - "Environment" => Settings.forms_env.downcase, - "FormId" => form_id.to_s, - "FormName" => form_name.to_s, - } - end - - def export_metrics! - return if meter_provider.metric_readers.empty? - - result = meter_provider.force_flush - return if result == OpenTelemetry::SDK::Metrics::Export::SUCCESS - - raise ExportError, "Failed to export submission count metrics (status: #{result})" - end - end -end diff --git a/app/services/metrics/submission_counter.rb b/app/services/metrics/submission_counter.rb new file mode 100644 index 000000000..f8565f06b --- /dev/null +++ b/app/services/metrics/submission_counter.rb @@ -0,0 +1,44 @@ +module Metrics + class SubmissionCounter + METRIC_NAME = "SubmissionCount".freeze + METER_NAME = "forms-runner".freeze + METER_VERSION = "1.0".freeze + + class << self + def record(form_id:, form_name:, mode:, meter_provider: OpenTelemetry.meter_provider) + return if mode.preview? + + counter(meter_provider).add( + 1, + attributes: metric_attributes(form_id:, form_name:), + ) + end + + private + + def counter(meter_provider) + counters[meter_provider] ||= meter(meter_provider).create_counter( + METRIC_NAME, + unit: "1", + description: "Number of form submissions", + ) + end + + def counters + @counters ||= {} + end + + def meter(meter_provider) + meter_provider.meter(METER_NAME, version: METER_VERSION) + end + + def metric_attributes(form_id:, form_name:) + { + "Environment" => Settings.forms_env.downcase, + "FormId" => form_id.to_s, + "FormName" => form_name.to_s, + } + end + end + end +end diff --git a/config/initializers/opentelemetry.rb b/config/initializers/opentelemetry.rb index f849e1cb1..c68fb935c 100644 --- a/config/initializers/opentelemetry.rb +++ b/config/initializers/opentelemetry.rb @@ -5,8 +5,6 @@ return unless ENV["ENABLE_OTEL"] == "true" -ENV["OTEL_METRICS_EXPORTER"] ||= "otlp" - OpenTelemetry::SDK.configure do |c| instrumentation_config = { "OpenTelemetry::Instrumentation::Rack" => { untraced_endpoints: ["/up"] } } c.use_all(instrumentation_config) @@ -16,6 +14,14 @@ c.id_generator = OpenTelemetry::Propagator::XRay::IDGenerator end + unless ENV.fetch("OTEL_METRICS_EXPORTER", "otlp") == "none" + c.add_metric_reader( + OpenTelemetry::SDK::Metrics::Export::PeriodicMetricReader.new( + exporter: OpenTelemetry::Exporter::OTLP::Metrics::MetricsExporter.new, + ), + ) + end + # Disable logging for Rake tasks to avoid cluttering output c.logger = Logger.new(File::NULL) if Rails.const_defined?(:Rake) && Rake.application.top_level_tasks.any? end diff --git a/lib/tasks/metrics.rake b/lib/tasks/metrics.rake deleted file mode 100644 index 6caf7c544..000000000 --- a/lib/tasks/metrics.rake +++ /dev/null @@ -1,6 +0,0 @@ -namespace :metrics do - desc "Export submission counts for the last 5 minutes as OpenTelemetry metrics grouped by form" - task export_submission_counts: :environment do - Metrics::SubmissionCountService.new.publish_submission_counts - end -end diff --git a/spec/lib/tasks/metrics.rake_spec.rb b/spec/lib/tasks/metrics.rake_spec.rb deleted file mode 100644 index 0a542d26d..000000000 --- a/spec/lib/tasks/metrics.rake_spec.rb +++ /dev/null @@ -1,17 +0,0 @@ -require "rails_helper" - -RSpec.describe "metrics.rake", type: :task do - describe "metrics:export_submission_counts" do - subject(:task) do - Rake::Task["metrics:export_submission_counts"] - end - - it "publishes submission counts via Metrics::SubmissionCountService" do - service = instance_double(Metrics::SubmissionCountService) - allow(Metrics::SubmissionCountService).to receive(:new).and_return(service) - expect(service).to receive(:publish_submission_counts) - - task.invoke - end - end -end diff --git a/spec/services/form_submission_service_spec.rb b/spec/services/form_submission_service_spec.rb index 158bdb406..c7149e4dc 100644 --- a/spec/services/form_submission_service_spec.rb +++ b/spec/services/form_submission_service_spec.rb @@ -102,6 +102,16 @@ expect(log_line["submission_reference"]).to eq(reference) end + it "records a submission count metric" do + expect(Metrics::SubmissionCounter).to receive(:record).with( + form_id: form.id, + form_name: form.name, + mode:, + ) + + service.submit + end + shared_examples "logging" do it "logs submission" do allow(LogEventService).to receive(:log_submit).once diff --git a/spec/services/metrics/submission_count_service_spec.rb b/spec/services/metrics/submission_count_service_spec.rb deleted file mode 100644 index 9c27a3dc5..000000000 --- a/spec/services/metrics/submission_count_service_spec.rb +++ /dev/null @@ -1,116 +0,0 @@ -require "rails_helper" -require "opentelemetry-metrics-sdk" - -describe Metrics::SubmissionCountService do - subject(:service) { described_class.new(meter_provider:) } - - let(:meter_provider) { OpenTelemetry::SDK::Metrics::MeterProvider.new } - let(:metric_exporter) { OpenTelemetry::SDK::Metrics::Export::InMemoryMetricPullExporter.new } - let(:forms_env) { "test" } - let(:form_id) { 42 } - let(:other_form_id) { 99 } - let(:form_document) { build(:v2_form_document, form_id:, name: "Test Form") } - let(:other_form_document) { build(:v2_form_document, form_id: other_form_id, name: "Other Form") } - let(:third_form_document) { build(:v2_form_document, form_id: 123, name: "Third Form") } - - before do - allow(Settings).to receive(:forms_env).and_return(forms_env) - meter_provider.add_metric_reader(metric_exporter) - end - - around do |example| - travel_to(Time.zone.local(2026, 6, 3, 12, 0, 0)) do - example.run - end - end - - describe "#publish_submission_counts" do - context "with submissions for multiple forms in the export period" do - before do - create_list(:submission, 2, form_id:, form_document:) - create(:submission, form_id: other_form_id, form_document: other_form_document) - create(:submission, :preview, form_id:, form_document:) - create(:submission, form_id: 123, form_document: third_form_document) - end - - it "publishes grouped submission counts for the period via OpenTelemetry" do - service.publish_submission_counts - - expect(exported_data_points).to contain_exactly( - data_point(form_id:, form_name: "Test Form", count: 2), - data_point(form_id: other_form_id, form_name: "Other Form", count: 1), - data_point(form_id: 123, form_name: "Third Form", count: 1), - ) - end - - context "when metric export fails" do - before do - allow(meter_provider).to receive(:force_flush) - .and_return(OpenTelemetry::SDK::Metrics::Export::FAILURE) - end - - it "captures the exception and re-raises" do - expect(Sentry).to receive(:capture_exception).with(instance_of(Metrics::SubmissionCountService::ExportError)) - - expect { service.publish_submission_counts }.to raise_error(Metrics::SubmissionCountService::ExportError) - end - end - end - - context "with submissions outside the export period" do - before do - create(:submission, form_id:, form_document:, created_at: 10.minutes.ago) - create(:submission, form_id:, form_document:, created_at: 4.minutes.ago) - end - - it "only counts submissions within the last 5 minutes" do - service.publish_submission_counts - - expect(exported_data_points).to contain_exactly( - data_point(form_id:, form_name: "Test Form", count: 1), - ) - end - end - - context "when a form has been renamed within the export period" do - before do - create( - :submission, - form_id:, - form_document: build(:v2_form_document, form_id:, name: "Older Form Name"), - created_at: 4.minutes.ago, - ) - create( - :submission, - form_id:, - form_document: build(:v2_form_document, form_id:, name: "Latest Form Name"), - created_at: 1.minute.ago, - ) - end - - it "uses the latest form name from the most recent submission in the period" do - service.publish_submission_counts - - expect(exported_data_points).to include( - data_point(form_id:, form_name: "Latest Form Name", count: 2), - ) - end - end - end - - def exported_data_points - metric_exporter.pull - metric_exporter.metric_snapshots.flat_map(&:data_points) - end - - def data_point(form_id:, form_name:, count:) - have_attributes( - value: count, - attributes: { - "Environment" => forms_env, - "FormId" => form_id.to_s, - "FormName" => form_name, - }, - ) - end -end diff --git a/spec/services/metrics/submission_counter_spec.rb b/spec/services/metrics/submission_counter_spec.rb new file mode 100644 index 000000000..b51179adb --- /dev/null +++ b/spec/services/metrics/submission_counter_spec.rb @@ -0,0 +1,88 @@ +require "rails_helper" +require "opentelemetry-metrics-sdk" + +describe Metrics::SubmissionCounter do + let(:meter_provider) { OpenTelemetry::SDK::Metrics::MeterProvider.new } + let(:metric_exporter) { OpenTelemetry::SDK::Metrics::Export::InMemoryMetricPullExporter.new } + let(:forms_env) { "test" } + let(:form_id) { 42 } + let(:form_name) { "Test Form" } + let(:mode) { Mode.new("form") } + + before do + allow(Settings).to receive(:forms_env).and_return(forms_env) + meter_provider.add_metric_reader(metric_exporter) + described_class.send(:counters).clear + end + + describe ".record" do + it "records a submission count metric" do + described_class.record(form_id:, form_name:, mode:, meter_provider:) + + expect(exported_data_points).to contain_exactly( + have_attributes( + value: 1, + attributes: { + "Environment" => forms_env, + "FormId" => form_id.to_s, + "FormName" => form_name, + }, + ), + ) + end + + context "when mode is preview" do + let(:mode) { Mode.new("preview-live") } + + it "does not record a metric" do + described_class.record(form_id:, form_name:, mode:, meter_provider:) + + expect(exported_data_points).to be_empty + end + end + + it "accumulates counts for the same form" do + 2.times { described_class.record(form_id:, form_name:, mode:, meter_provider:) } + + expect(exported_data_points).to contain_exactly( + have_attributes( + value: 2, + attributes: { + "Environment" => forms_env, + "FormId" => form_id.to_s, + "FormName" => form_name, + }, + ), + ) + end + + it "records separate counts per form" do + described_class.record(form_id:, form_name:, mode:, meter_provider:) + described_class.record(form_id: 99, form_name: "Other Form", mode:, meter_provider:) + + expect(exported_data_points).to contain_exactly( + have_attributes( + value: 1, + attributes: { + "Environment" => forms_env, + "FormId" => form_id.to_s, + "FormName" => form_name, + }, + ), + have_attributes( + value: 1, + attributes: { + "Environment" => forms_env, + "FormId" => "99", + "FormName" => "Other Form", + }, + ), + ) + end + end + + def exported_data_points + metric_exporter.pull + metric_exporter.metric_snapshots.flat_map(&:data_points) + end +end