From d13923bd2111be68d23abf06ea844f2d595c00a2 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 12 Mar 2024 13:52:04 +0100 Subject: [PATCH] Add Metrics API and aggregator (#2247) * new `Sentry::Metrics` module with 4 apis that map to the new 4 `Sentry::Metrics::Metric` classes * `increment` - simple counter * `distribution` - array of observations * `gauge` - statistics (last/min/max/sum/count) * `set` - unique values * new `Sentry::Metrics::Aggregator` that starts a thread that flushes pending metric buckets in 5 second intervals * buckets are a nested hash of timestamp (rolled to 10 second intervals) -> metric keys -> actual metric instance * there is a random `flush_shift` once per startup to create jittering * flushable buckets are sent in a new `statsd` type envelope that is not json so made a small change to the `Envelope::Item` * tag key/values are sanitized for unicode/special characters according to the two regexes Reference spec - https://develop.sentry.dev/sdk/metrics/ part of #2246 --- CHANGELOG.md | 32 ++ Gemfile | 2 - sentry-ruby/lib/sentry-ruby.rb | 12 + sentry-ruby/lib/sentry/configuration.rb | 6 + sentry-ruby/lib/sentry/envelope.rb | 2 +- sentry-ruby/lib/sentry/metrics.rb | 30 ++ sentry-ruby/lib/sentry/metrics/aggregator.rb | 184 +++++++++++ .../lib/sentry/metrics/configuration.rb | 17 + .../lib/sentry/metrics/counter_metric.rb | 25 ++ .../lib/sentry/metrics/distribution_metric.rb | 25 ++ .../lib/sentry/metrics/gauge_metric.rb | 35 +++ sentry-ruby/lib/sentry/metrics/metric.rb | 19 ++ sentry-ruby/lib/sentry/metrics/set_metric.rb | 28 ++ sentry-ruby/lib/sentry/scope.rb | 6 + sentry-ruby/lib/sentry/transaction.rb | 10 +- .../spec/sentry/metrics/aggregator_spec.rb | 295 ++++++++++++++++++ .../sentry/metrics/counter_metric_spec.rb | 25 ++ .../metrics/distribution_metric_spec.rb | 25 ++ .../spec/sentry/metrics/gauge_metric_spec.rb | 29 ++ .../spec/sentry/metrics/metric_spec.rb | 21 ++ .../spec/sentry/metrics/set_metric_spec.rb | 30 ++ sentry-ruby/spec/sentry/metrics_spec.rb | 85 +++++ sentry-ruby/spec/sentry/transaction_spec.rb | 12 + sentry-ruby/spec/sentry/transport_spec.rb | 24 ++ sentry-ruby/spec/sentry_spec.rb | 8 + 25 files changed, 979 insertions(+), 8 deletions(-) create mode 100644 sentry-ruby/lib/sentry/metrics.rb create mode 100644 sentry-ruby/lib/sentry/metrics/aggregator.rb create mode 100644 sentry-ruby/lib/sentry/metrics/configuration.rb create mode 100644 sentry-ruby/lib/sentry/metrics/counter_metric.rb create mode 100644 sentry-ruby/lib/sentry/metrics/distribution_metric.rb create mode 100644 sentry-ruby/lib/sentry/metrics/gauge_metric.rb create mode 100644 sentry-ruby/lib/sentry/metrics/metric.rb create mode 100644 sentry-ruby/lib/sentry/metrics/set_metric.rb create mode 100644 sentry-ruby/spec/sentry/metrics/aggregator_spec.rb create mode 100644 sentry-ruby/spec/sentry/metrics/counter_metric_spec.rb create mode 100644 sentry-ruby/spec/sentry/metrics/distribution_metric_spec.rb create mode 100644 sentry-ruby/spec/sentry/metrics/gauge_metric_spec.rb create mode 100644 sentry-ruby/spec/sentry/metrics/metric_spec.rb create mode 100644 sentry-ruby/spec/sentry/metrics/set_metric_spec.rb create mode 100644 sentry-ruby/spec/sentry/metrics_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 88b654cac..ee66bffa6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,6 +5,38 @@ - Add support for distributed tracing in `sentry-delayed_job` [#2233](https://github.com/getsentry/sentry-ruby/pull/2233) - Fix warning about default gems on Ruby 3.3.0 ([#2225](https://github.com/getsentry/sentry-ruby/pull/2225)) - Add `hint:` support to `Sentry::Rails::ErrorSubscriber` [#2235](https://github.com/getsentry/sentry-ruby/pull/2235) +- Add [Metrics](https://docs.sentry.io/product/metrics/) support + - Add main APIs and `Aggregator` thread [#2247](https://github.com/getsentry/sentry-ruby/pull/2247) + + The SDK now supports recording and aggregating metrics. A new thread will be started + for aggregation and will flush the pending data to Sentry every 5 seconds. + + To enable this behavior, use: + + ```ruby + Sentry.init do |config| + # ... + config.metrics.enabled = true + end + ``` + + And then in your application code, collect metrics as follows: + + ```ruby + # increment a simple counter + Sentry::Metrics.increment('button_click') + # set a value, unit and tags + Sentry::Metrics.increment('time', 5, unit: 'second', tags: { browser:' firefox' }) + + # distribution - get statistical aggregates from an array of observations + Sentry::Metrics.distribution('page_load', 15.0, unit: 'millisecond') + + # gauge - record statistical aggregates directly on the SDK, more space efficient + Sentry::Metrics.gauge('page_load', 15.0, unit: 'millisecond') + + # set - get unique counts of elements + Sentry::Metrics.set('user_view', 'jane') + ``` ### Bug Fixes diff --git a/Gemfile b/Gemfile index 093178f80..515440efe 100644 --- a/Gemfile +++ b/Gemfile @@ -9,8 +9,6 @@ ruby_version = Gem::Version.new(RUBY_VERSION) if ruby_version >= Gem::Version.new("2.7.0") gem "debug", github: "ruby/debug", platform: :ruby gem "irb" - # new release breaks on jruby - gem "io-console", "0.6.0" if ruby_version >= Gem::Version.new("3.0.0") gem "ruby-lsp-rspec" diff --git a/sentry-ruby/lib/sentry-ruby.rb b/sentry-ruby/lib/sentry-ruby.rb index 5c750f373..79417d40f 100644 --- a/sentry-ruby/lib/sentry-ruby.rb +++ b/sentry-ruby/lib/sentry-ruby.rb @@ -23,6 +23,7 @@ require "sentry/session_flusher" require "sentry/backpressure_monitor" require "sentry/cron/monitor_check_ins" +require "sentry/metrics" [ "sentry/rake", @@ -77,6 +78,10 @@ def exception_locals_tp # @return [BackpressureMonitor, nil] attr_reader :backpressure_monitor + # @!attribute [r] metrics_aggregator + # @return [Metrics::Aggregator, nil] + attr_reader :metrics_aggregator + ##### Patch Registration ##### # @!visibility private @@ -224,6 +229,7 @@ def init(&block) @background_worker = Sentry::BackgroundWorker.new(config) @session_flusher = config.session_tracking? ? Sentry::SessionFlusher.new(config, client) : nil @backpressure_monitor = config.enable_backpressure_handling ? Sentry::BackpressureMonitor.new(config, client) : nil + @metrics_aggregator = config.metrics.enabled ? Sentry::Metrics::Aggregator.new(config, client) : nil exception_locals_tp.enable if config.include_local_variables at_exit { close } end @@ -244,6 +250,12 @@ def close @backpressure_monitor = nil end + if @metrics_aggregator + @metrics_aggregator.flush(force: true) + @metrics_aggregator.kill + @metrics_aggregator = nil + end + if client = get_current_client client.transport.flush diff --git a/sentry-ruby/lib/sentry/configuration.rb b/sentry-ruby/lib/sentry/configuration.rb index e2ca1eef6..c2776459f 100644 --- a/sentry-ruby/lib/sentry/configuration.rb +++ b/sentry-ruby/lib/sentry/configuration.rb @@ -8,6 +8,7 @@ require "sentry/release_detector" require "sentry/transport/configuration" require "sentry/cron/configuration" +require "sentry/metrics/configuration" require "sentry/linecache" require "sentry/interfaces/stacktrace_builder" @@ -235,6 +236,10 @@ def capture_exception_frame_locals=(value) # @return [Cron::Configuration] attr_reader :cron + # Metrics related configuration. + # @return [Metrics::Configuration] + attr_reader :metrics + # Take a float between 0.0 and 1.0 as the sample rate for tracing events (transactions). # @return [Float, nil] attr_reader :traces_sample_rate @@ -386,6 +391,7 @@ def initialize @transport = Transport::Configuration.new @cron = Cron::Configuration.new + @metrics = Metrics::Configuration.new @gem_specs = Hash[Gem::Specification.map { |spec| [spec.name, spec.version.to_s] }] if Gem::Specification.respond_to?(:map) run_post_initialization_callbacks diff --git a/sentry-ruby/lib/sentry/envelope.rb b/sentry-ruby/lib/sentry/envelope.rb index de981d5f2..6d9ff56ff 100644 --- a/sentry-ruby/lib/sentry/envelope.rb +++ b/sentry-ruby/lib/sentry/envelope.rb @@ -19,7 +19,7 @@ def type end def to_s - [JSON.generate(@headers), JSON.generate(@payload)].join("\n") + [JSON.generate(@headers), @payload.is_a?(String) ? @payload : JSON.generate(@payload)].join("\n") end def serialize diff --git a/sentry-ruby/lib/sentry/metrics.rb b/sentry-ruby/lib/sentry/metrics.rb new file mode 100644 index 000000000..190c026aa --- /dev/null +++ b/sentry-ruby/lib/sentry/metrics.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'sentry/metrics/metric' +require 'sentry/metrics/counter_metric' +require 'sentry/metrics/distribution_metric' +require 'sentry/metrics/gauge_metric' +require 'sentry/metrics/set_metric' +require 'sentry/metrics/aggregator' + +module Sentry + module Metrics + class << self + def increment(key, value = 1.0, unit: 'none', tags: {}, timestamp: nil) + Sentry.metrics_aggregator&.add(:c, key, value, unit: unit, tags: tags, timestamp: timestamp) + end + + def distribution(key, value, unit: 'none', tags: {}, timestamp: nil) + Sentry.metrics_aggregator&.add(:d, key, value, unit: unit, tags: tags, timestamp: timestamp) + end + + def set(key, value, unit: 'none', tags: {}, timestamp: nil) + Sentry.metrics_aggregator&.add(:s, key, value, unit: unit, tags: tags, timestamp: timestamp) + end + + def gauge(key, value, unit: 'none', tags: {}, timestamp: nil) + Sentry.metrics_aggregator&.add(:g, key, value, unit: unit, tags: tags, timestamp: timestamp) + end + end + end +end diff --git a/sentry-ruby/lib/sentry/metrics/aggregator.rb b/sentry-ruby/lib/sentry/metrics/aggregator.rb new file mode 100644 index 000000000..f7d6b17b1 --- /dev/null +++ b/sentry-ruby/lib/sentry/metrics/aggregator.rb @@ -0,0 +1,184 @@ +# frozen_string_literal: true + +module Sentry + module Metrics + class Aggregator + include LoggingHelper + + FLUSH_INTERVAL = 5 + ROLLUP_IN_SECONDS = 10 + + KEY_SANITIZATION_REGEX = /[^a-zA-Z0-9_\/.-]+/ + VALUE_SANITIZATION_REGEX = /[^[[:word:]][[:digit:]][[:space:]]_:\/@\.{}\[\]$-]+/ + + METRIC_TYPES = { + c: CounterMetric, + d: DistributionMetric, + g: GaugeMetric, + s: SetMetric + } + + # exposed only for testing + attr_reader :thread, :buckets, :flush_shift + + def initialize(configuration, client) + @client = client + @logger = configuration.logger + + @default_tags = {} + @default_tags['release'] = configuration.release if configuration.release + @default_tags['environment'] = configuration.environment if configuration.environment + + @thread = nil + @exited = false + @mutex = Mutex.new + + # buckets are a nested hash of timestamp -> bucket keys -> Metric instance + @buckets = {} + + # the flush interval needs to be shifted once per startup to create jittering + @flush_shift = Random.rand * ROLLUP_IN_SECONDS + end + + def add(type, + key, + value, + unit: 'none', + tags: {}, + timestamp: nil) + return unless ensure_thread + return unless METRIC_TYPES.keys.include?(type) + + timestamp = timestamp.to_i if timestamp.is_a?(Time) + timestamp ||= Sentry.utc_now.to_i + + # this is integer division and thus takes the floor of the division + # and buckets into 10 second intervals + bucket_timestamp = (timestamp / ROLLUP_IN_SECONDS) * ROLLUP_IN_SECONDS + + serialized_tags = serialize_tags(get_updated_tags(tags)) + bucket_key = [type, key, unit, serialized_tags] + + @mutex.synchronize do + @buckets[bucket_timestamp] ||= {} + + if @buckets[bucket_timestamp][bucket_key] + @buckets[bucket_timestamp][bucket_key].add(value) + else + @buckets[bucket_timestamp][bucket_key] = METRIC_TYPES[type].new(value) + end + end + end + + def flush(force: false) + flushable_buckets = get_flushable_buckets!(force) + return if flushable_buckets.empty? + + payload = serialize_buckets(flushable_buckets) + envelope = Envelope.new + envelope.add_item( + { type: 'statsd', length: payload.bytesize }, + payload + ) + + Sentry.background_worker.perform do + @client.transport.send_envelope(envelope) + end + end + + def kill + log_debug('[Metrics::Aggregator] killing thread') + + @exited = true + @thread&.kill + end + + private + + def ensure_thread + return false if @exited + return true if @thread&.alive? + + @thread = Thread.new do + loop do + # TODO-neel-metrics use event for force flush later + sleep(FLUSH_INTERVAL) + flush + end + end + + true + rescue ThreadError + log_debug('[Metrics::Aggregator] thread creation failed') + @exited = true + false + end + + # important to sort for key consistency + def serialize_tags(tags) + tags.flat_map do |k, v| + if v.is_a?(Array) + v.map { |x| [k.to_s, x.to_s] } + else + [[k.to_s, v.to_s]] + end + end.sort + end + + def get_flushable_buckets!(force) + @mutex.synchronize do + flushable_buckets = {} + + if force + flushable_buckets = @buckets + @buckets = {} + else + cutoff = Sentry.utc_now.to_i - ROLLUP_IN_SECONDS - @flush_shift + flushable_buckets = @buckets.select { |k, _| k <= cutoff } + @buckets.reject! { |k, _| k <= cutoff } + end + + flushable_buckets + end + end + + # serialize buckets to statsd format + def serialize_buckets(buckets) + buckets.map do |timestamp, timestamp_buckets| + timestamp_buckets.map do |metric_key, metric| + type, key, unit, tags = metric_key + values = metric.serialize.join(':') + sanitized_tags = tags.map { |k, v| "#{sanitize_key(k)}:#{sanitize_value(v)}" }.join(',') + + "#{sanitize_key(key)}@#{unit}:#{values}|#{type}|\##{sanitized_tags}|T#{timestamp}" + end + end.flatten.join("\n") + end + + def sanitize_key(key) + key.gsub(KEY_SANITIZATION_REGEX, '_') + end + + def sanitize_value(value) + value.gsub(VALUE_SANITIZATION_REGEX, '') + end + + def get_transaction_name + scope = Sentry.get_current_scope + return nil unless scope && scope.transaction_name + return nil if scope.transaction_source_low_quality? + + scope.transaction_name + end + + def get_updated_tags(tags) + updated_tags = @default_tags.merge(tags) + + transaction_name = get_transaction_name + updated_tags['transaction'] = transaction_name if transaction_name + + updated_tags + end + end + end +end diff --git a/sentry-ruby/lib/sentry/metrics/configuration.rb b/sentry-ruby/lib/sentry/metrics/configuration.rb new file mode 100644 index 000000000..1c8b86be2 --- /dev/null +++ b/sentry-ruby/lib/sentry/metrics/configuration.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Sentry + module Metrics + class Configuration + # Enable metrics usage + # Starts a new {Sentry::Metrics::Aggregator} instance to aggregate metrics + # and a thread to aggregate flush every 5 seconds. + # @return [Boolean] + attr_accessor :enabled + + def initialize + @enabled = false + end + end + end +end diff --git a/sentry-ruby/lib/sentry/metrics/counter_metric.rb b/sentry-ruby/lib/sentry/metrics/counter_metric.rb new file mode 100644 index 000000000..6ef9d4d48 --- /dev/null +++ b/sentry-ruby/lib/sentry/metrics/counter_metric.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Sentry + module Metrics + class CounterMetric < Metric + attr_reader :value + + def initialize(value) + @value = value.to_f + end + + def add(value) + @value += value.to_f + end + + def serialize + [value] + end + + def weight + 1 + end + end + end +end diff --git a/sentry-ruby/lib/sentry/metrics/distribution_metric.rb b/sentry-ruby/lib/sentry/metrics/distribution_metric.rb new file mode 100644 index 000000000..eae9aff2a --- /dev/null +++ b/sentry-ruby/lib/sentry/metrics/distribution_metric.rb @@ -0,0 +1,25 @@ +# frozen_string_literal: true + +module Sentry + module Metrics + class DistributionMetric < Metric + attr_reader :value + + def initialize(value) + @value = [value.to_f] + end + + def add(value) + @value << value.to_f + end + + def serialize + value + end + + def weight + value.size + end + end + end +end diff --git a/sentry-ruby/lib/sentry/metrics/gauge_metric.rb b/sentry-ruby/lib/sentry/metrics/gauge_metric.rb new file mode 100644 index 000000000..f0ba98514 --- /dev/null +++ b/sentry-ruby/lib/sentry/metrics/gauge_metric.rb @@ -0,0 +1,35 @@ +# frozen_string_literal: true + +module Sentry + module Metrics + class GaugeMetric < Metric + attr_reader :last, :min, :max, :sum, :count + + def initialize(value) + value = value.to_f + @last = value + @min = value + @max = value + @sum = value + @count = 1 + end + + def add(value) + value = value.to_f + @last = value + @min = [@min, value].min + @max = [@max, value].max + @sum += value + @count += 1 + end + + def serialize + [last, min, max, sum, count] + end + + def weight + 5 + end + end + end +end diff --git a/sentry-ruby/lib/sentry/metrics/metric.rb b/sentry-ruby/lib/sentry/metrics/metric.rb new file mode 100644 index 000000000..dbf4a17e0 --- /dev/null +++ b/sentry-ruby/lib/sentry/metrics/metric.rb @@ -0,0 +1,19 @@ +# frozen_string_literal: true + +module Sentry + module Metrics + class Metric + def add(value) + raise NotImplementedError + end + + def serialize + raise NotImplementedError + end + + def weight + raise NotImplementedError + end + end + end +end diff --git a/sentry-ruby/lib/sentry/metrics/set_metric.rb b/sentry-ruby/lib/sentry/metrics/set_metric.rb new file mode 100644 index 000000000..02c5c26a6 --- /dev/null +++ b/sentry-ruby/lib/sentry/metrics/set_metric.rb @@ -0,0 +1,28 @@ +# frozen_string_literal: true + +require 'set' +require 'zlib' + +module Sentry + module Metrics + class SetMetric < Metric + attr_reader :value + + def initialize(value) + @value = Set[value] + end + + def add(value) + @value << value + end + + def serialize + value.map { |x| x.is_a?(String) ? Zlib.crc32(x) : x.to_i } + end + + def weight + value.size + end + end + end +end diff --git a/sentry-ruby/lib/sentry/scope.rb b/sentry-ruby/lib/sentry/scope.rb index 4983895ae..6e78c6e04 100644 --- a/sentry-ruby/lib/sentry/scope.rb +++ b/sentry-ruby/lib/sentry/scope.rb @@ -252,6 +252,12 @@ def transaction_source @transaction_sources.last end + # These are high cardinality and thus bad. + # @return [Boolean] + def transaction_source_low_quality? + transaction_source == :url + end + # Returns the associated Transaction object. # @return [Transaction, nil] def get_transaction diff --git a/sentry-ruby/lib/sentry/transaction.rb b/sentry-ruby/lib/sentry/transaction.rb index aeaaf6eb2..456b49e74 100644 --- a/sentry-ruby/lib/sentry/transaction.rb +++ b/sentry-ruby/lib/sentry/transaction.rb @@ -302,6 +302,11 @@ def start_profiler! profiler.start end + # These are high cardinality and thus bad + def source_low_quality? + source == :url + end + protected def init_span_recorder(limit = 1000) @@ -337,11 +342,6 @@ def populate_head_baggage @baggage = Baggage.new(items, mutable: false) end - # These are high cardinality and thus bad - def source_low_quality? - source == :url - end - class SpanRecorder attr_reader :max_length, :spans diff --git a/sentry-ruby/spec/sentry/metrics/aggregator_spec.rb b/sentry-ruby/spec/sentry/metrics/aggregator_spec.rb new file mode 100644 index 000000000..d95ffbde8 --- /dev/null +++ b/sentry-ruby/spec/sentry/metrics/aggregator_spec.rb @@ -0,0 +1,295 @@ +require 'spec_helper' + +RSpec.describe Sentry::Metrics::Aggregator do + let(:string_io) { StringIO.new } + + # fix at 3 second offset to check rollup + let(:fake_time) { Time.new(2024, 1, 1, 1, 1, 3) } + + before do + perform_basic_setup do |config| + config.metrics.enabled = true + config.enable_tracing = true + config.release = 'test-release' + config.environment = 'test' + config.logger = Logger.new(string_io) + end + end + + subject { Sentry.metrics_aggregator } + + describe '#add' do + it 'spawns new thread' do + expect { subject.add(:c, 'incr', 1) }.to change { Thread.list.count }.by(1) + expect(subject.thread).to be_a(Thread) + end + + it 'spawns only one thread' do + expect { subject.add(:c, 'incr', 1) }.to change { Thread.list.count }.by(1) + + expect(subject.thread).to receive(:alive?).and_call_original + expect { subject.add(:c, 'incr', 1) }.to change { Thread.list.count }.by(0) + end + + context 'when thread creation fails' do + before do + expect(Thread).to receive(:new).and_raise(ThreadError) + end + + it 'does not create new thread' do + expect { subject.add(:c, 'incr', 1) }.to change { Thread.list.count }.by(0) + end + + it 'noops' do + subject.add(:c, 'incr', 1) + expect(subject.buckets).to eq({}) + end + + it 'logs error' do + subject.add(:c, 'incr', 1) + expect(string_io.string).to match(/\[Metrics::Aggregator\] thread creation failed/) + end + end + + context 'when killed' do + before { subject.kill } + + it 'noops' do + subject.add(:c, 'incr', 1) + expect(subject.buckets).to eq({}) + end + + it 'does not create new thread' do + expect(Thread).not_to receive(:new) + expect { subject.add(:c, 'incr', 1) }.to change { Thread.list.count }.by(0) + end + end + + it 'does not add unsupported metric type' do + subject.add(:foo, 'foo', 1) + expect(subject.buckets).to eq({}) + end + + it 'has the correct bucket timestamp key rolled up to 10 seconds' do + allow(Time).to receive(:now).and_return(fake_time) + subject.add(:c, 'incr', 1) + expect(subject.buckets.keys.first).to eq(fake_time.to_i - 3) + end + + it 'has the correct bucket timestamp key rolled up to 10 seconds when passed explicitly' do + subject.add(:c, 'incr', 1, timestamp: fake_time + 9) + expect(subject.buckets.keys.first).to eq(fake_time.to_i + 7) + end + + it 'has the correct type in the bucket metric key' do + subject.add(:c, 'incr', 1) + type, _, _, _ = subject.buckets.values.first.keys.first + expect(type).to eq(:c) + end + + it 'has the correct key in the bucket metric key' do + subject.add(:c, 'incr', 1) + _, key, _, _ = subject.buckets.values.first.keys.first + expect(key).to eq('incr') + end + + it 'has the default unit \'none\' in the bucket metric key' do + subject.add(:c, 'incr', 1) + _, _, unit, _ = subject.buckets.values.first.keys.first + expect(unit).to eq('none') + end + + it 'has the correct custom unit in the bucket metric key' do + subject.add(:c, 'incr', 1, unit: 'second') + _, _, unit, _ = subject.buckets.values.first.keys.first + expect(unit).to eq('second') + end + + it 'has the correct default tags serialized in the bucket metric key' do + subject.add(:c, 'incr', 1) + _, _, _, tags = subject.buckets.values.first.keys.first + expect(tags).to eq([['environment', 'test'], ['release', 'test-release']]) + end + + it 'has the correct custom tags serialized in the bucket metric key' do + subject.add(:c, 'incr', 1, tags: { foo: 42 }) + _, _, _, tags = subject.buckets.values.first.keys.first + expect(tags).to include(['foo', '42']) + end + + it 'has the correct array value tags serialized in the bucket metric key' do + subject.add(:c, 'incr', 1, tags: { foo: [42, 43] }) + _, _, _, tags = subject.buckets.values.first.keys.first + expect(tags).to include(['foo', '42'], ['foo', '43']) + end + + context 'with running transaction' do + it 'has the transaction name in tags serialized in the bucket metric key' do + Sentry.get_current_scope.set_transaction_name('foo') + subject.add(:c, 'incr', 1) + _, _, _, tags = subject.buckets.values.first.keys.first + expect(tags).to include(['transaction', 'foo']) + end + + it 'does not has the low quality transaction name in tags serialized in the bucket metric key' do + Sentry.get_current_scope.set_transaction_name('foo', source: :url) + subject.add(:c, 'incr', 1) + _, _, _, tags = subject.buckets.values.first.keys.first + expect(tags).not_to include(['transaction', 'foo']) + end + end + + it 'creates a new CounterMetric instance if not existing' do + expect(Sentry::Metrics::CounterMetric).to receive(:new).and_call_original + subject.add(:c, 'incr', 1) + + metric = subject.buckets.values.first.values.first + expect(metric).to be_a(Sentry::Metrics::CounterMetric) + expect(metric.value).to eq(1.0) + end + + it 'reuses the existing CounterMetric instance' do + allow(Time).to receive(:now).and_return(fake_time) + + subject.add(:c, 'incr', 1) + metric = subject.buckets.values.first.values.first + expect(metric.value).to eq(1.0) + + expect(Sentry::Metrics::CounterMetric).not_to receive(:new) + expect(metric).to receive(:add).with(2).and_call_original + subject.add(:c, 'incr', 2) + expect(metric.value).to eq(3.0) + end + + it 'creates a new DistributionMetric instance if not existing' do + expect(Sentry::Metrics::DistributionMetric).to receive(:new).and_call_original + subject.add(:d, 'dist', 1) + + metric = subject.buckets.values.first.values.first + expect(metric).to be_a(Sentry::Metrics::DistributionMetric) + expect(metric.value).to eq([1.0]) + end + + it 'creates a new GaugeMetric instance if not existing' do + expect(Sentry::Metrics::GaugeMetric).to receive(:new).and_call_original + subject.add(:g, 'gauge', 1) + + metric = subject.buckets.values.first.values.first + expect(metric).to be_a(Sentry::Metrics::GaugeMetric) + expect(metric.serialize).to eq([1.0, 1.0, 1.0, 1.0, 1]) + end + + it 'creates a new SetMetric instance if not existing' do + expect(Sentry::Metrics::SetMetric).to receive(:new).and_call_original + subject.add(:s, 'set', 1) + + metric = subject.buckets.values.first.values.first + expect(metric).to be_a(Sentry::Metrics::SetMetric) + expect(metric.value).to eq(Set[1]) + end + end + + describe '#flush' do + context 'with empty buckets' do + it 'returns early and does nothing' do + expect(sentry_envelopes.count).to eq(0) + subject.flush + end + + it 'returns early and does nothing with force' do + expect(sentry_envelopes.count).to eq(0) + subject.flush(force: true) + end + end + + context 'with pending buckets' do + before do + allow(Time).to receive(:now).and_return(fake_time) + 10.times { subject.add(:c, 'incr', 1) } + 5.times { |i| subject.add(:d, 'dist', i, unit: 'second', tags: { "foö$-bar" => "snöwmän% 23{}" }) } + + allow(Time).to receive(:now).and_return(fake_time + 9) + 5.times { subject.add(:c, 'incr', 1) } + 5.times { |i| subject.add(:d, 'dist', i + 5, unit: 'second', tags: { "foö$-bar" => "snöwmän% 23{}" }) } + + expect(subject.buckets.keys).to eq([fake_time.to_i - 3, fake_time.to_i + 7]) + expect(subject.buckets.values[0].length).to eq(2) + expect(subject.buckets.values[1].length).to eq(2) + + # set the time such that the first set of metrics above are picked + allow(Time).to receive(:now).and_return(fake_time + 9 + subject.flush_shift) + end + + context 'without force' do + it 'updates the pending buckets in place' do + subject.flush + + expect(subject.buckets.keys).to eq([fake_time.to_i + 7]) + expect(subject.buckets.values[0].length).to eq(2) + end + + it 'calls the background worker' do + expect(Sentry.background_worker).to receive(:perform) + subject.flush + end + + it 'sends the flushable buckets in statsd envelope item with correct payload' do + subject.flush + + envelope = sentry_envelopes.first + expect(envelope.headers).to eq({}) + + item = envelope.items.first + expect(item.headers).to eq({ type: 'statsd', length: item.payload.bytesize }) + + incr, dist = item.payload.split("\n") + expect(incr).to eq("incr@none:10.0|c|#environment:test,release:test-release|T#{fake_time.to_i - 3}") + expect(dist).to eq("dist@second:0.0:1.0:2.0:3.0:4.0|d|" + + "#environment:test,fo_-bar:snöwmän 23{},release:test-release|" + + "T#{fake_time.to_i - 3}") + end + end + + context 'with force' do + it 'empties the pending buckets in place' do + subject.flush(force: true) + expect(subject.buckets).to eq({}) + end + + it 'calls the background worker' do + expect(Sentry.background_worker).to receive(:perform) + subject.flush(force: true) + end + + it 'sends all buckets in statsd envelope item with correct payload' do + subject.flush(force: true) + + envelope = sentry_envelopes.first + expect(envelope.headers).to eq({}) + + item = envelope.items.first + expect(item.headers).to eq({ type: 'statsd', length: item.payload.bytesize }) + + incr1, dist1, incr2, dist2 = item.payload.split("\n") + expect(incr1).to eq("incr@none:10.0|c|#environment:test,release:test-release|T#{fake_time.to_i - 3}") + expect(dist1).to eq("dist@second:0.0:1.0:2.0:3.0:4.0|d|" + + "#environment:test,fo_-bar:snöwmän 23{},release:test-release|" + + "T#{fake_time.to_i - 3}") + expect(incr2).to eq("incr@none:5.0|c|#environment:test,release:test-release|T#{fake_time.to_i + 7}") + expect(dist2).to eq("dist@second:5.0:6.0:7.0:8.0:9.0|d|" + + "#environment:test,fo_-bar:snöwmän 23{},release:test-release|" + + "T#{fake_time.to_i + 7}") + end + end + end + end + + describe '#kill' do + before { subject.add(:c, 'incr', 1) } + it 'logs message when killing the thread' do + expect(subject.thread).to receive(:kill) + subject.kill + expect(string_io.string).to match(/\[Metrics::Aggregator\] killing thread/) + end + end +end diff --git a/sentry-ruby/spec/sentry/metrics/counter_metric_spec.rb b/sentry-ruby/spec/sentry/metrics/counter_metric_spec.rb new file mode 100644 index 000000000..f92a0135f --- /dev/null +++ b/sentry-ruby/spec/sentry/metrics/counter_metric_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' + +RSpec.describe Sentry::Metrics::CounterMetric do + subject { described_class.new(1) } + before { subject.add(2) } + + describe '#add' do + it 'adds float value' do + subject.add(3.0) + expect(subject.value).to eq(6.0) + end + end + + describe '#serialize' do + it 'returns value in array' do + expect(subject.serialize).to eq([3.0]) + end + end + + describe '#weight' do + it 'returns fixed value of 1' do + expect(subject.weight).to eq(1) + end + end +end diff --git a/sentry-ruby/spec/sentry/metrics/distribution_metric_spec.rb b/sentry-ruby/spec/sentry/metrics/distribution_metric_spec.rb new file mode 100644 index 000000000..7e5b2f2d4 --- /dev/null +++ b/sentry-ruby/spec/sentry/metrics/distribution_metric_spec.rb @@ -0,0 +1,25 @@ +require 'spec_helper' + +RSpec.describe Sentry::Metrics::DistributionMetric do + subject { described_class.new(1) } + before { subject.add(2) } + + describe '#add' do + it 'appends float value to array' do + subject.add(3.0) + expect(subject.value).to eq([1.0, 2.0, 3.0]) + end + end + + describe '#serialize' do + it 'returns whole array' do + expect(subject.serialize).to eq([1.0, 2.0]) + end + end + + describe '#weight' do + it 'returns length of array' do + expect(subject.weight).to eq(2) + end + end +end diff --git a/sentry-ruby/spec/sentry/metrics/gauge_metric_spec.rb b/sentry-ruby/spec/sentry/metrics/gauge_metric_spec.rb new file mode 100644 index 000000000..44be08465 --- /dev/null +++ b/sentry-ruby/spec/sentry/metrics/gauge_metric_spec.rb @@ -0,0 +1,29 @@ +require 'spec_helper' + +RSpec.describe Sentry::Metrics::GaugeMetric do + subject { described_class.new(0) } + before { 9.times { |i| subject.add(i + 1) } } + + describe '#add' do + it 'appends float value to array' do + subject.add(11) + expect(subject.last).to eq(11.0) + expect(subject.min).to eq(0.0) + expect(subject.max).to eq(11.0) + expect(subject.sum).to eq(56.0) + expect(subject.count).to eq(11) + end + end + + describe '#serialize' do + it 'returns array of statistics' do + expect(subject.serialize).to eq([9.0, 0.0, 9.0, 45.0, 10]) + end + end + + describe '#weight' do + it 'returns fixed value of 5' do + expect(subject.weight).to eq(5) + end + end +end diff --git a/sentry-ruby/spec/sentry/metrics/metric_spec.rb b/sentry-ruby/spec/sentry/metrics/metric_spec.rb new file mode 100644 index 000000000..a5ed3fd93 --- /dev/null +++ b/sentry-ruby/spec/sentry/metrics/metric_spec.rb @@ -0,0 +1,21 @@ +require 'spec_helper' + +RSpec.describe Sentry::Metrics::Metric do + describe '#add' do + it 'raises not implemented error' do + expect { subject.add(1) }.to raise_error(NotImplementedError) + end + end + + describe '#serialize' do + it 'raises not implemented error' do + expect { subject.serialize }.to raise_error(NotImplementedError) + end + end + + describe '#weight' do + it 'raises not implemented error' do + expect { subject.weight }.to raise_error(NotImplementedError) + end + end +end diff --git a/sentry-ruby/spec/sentry/metrics/set_metric_spec.rb b/sentry-ruby/spec/sentry/metrics/set_metric_spec.rb new file mode 100644 index 000000000..fdf74dd0d --- /dev/null +++ b/sentry-ruby/spec/sentry/metrics/set_metric_spec.rb @@ -0,0 +1,30 @@ +require 'spec_helper' + +RSpec.describe Sentry::Metrics::SetMetric do + subject { described_class.new('foo') } + + before do + 2.times { subject.add('foo') } + 2.times { subject.add('bar') } + 2.times { subject.add(42) } + end + + describe '#add' do + it 'appends new value to set' do + subject.add('baz') + expect(subject.value).to eq(Set['foo', 'bar', 'baz', 42]) + end + end + + describe '#serialize' do + it 'returns array of hashed values' do + expect(subject.serialize).to eq([Zlib.crc32('foo'), Zlib.crc32('bar'), 42]) + end + end + + describe '#weight' do + it 'returns length of set' do + expect(subject.weight).to eq(3) + end + end +end diff --git a/sentry-ruby/spec/sentry/metrics_spec.rb b/sentry-ruby/spec/sentry/metrics_spec.rb new file mode 100644 index 000000000..04acc66b3 --- /dev/null +++ b/sentry-ruby/spec/sentry/metrics_spec.rb @@ -0,0 +1,85 @@ +require 'spec_helper' + +RSpec.describe Sentry::Metrics do + before do + perform_basic_setup do |config| + config.metrics.enabled = true + end + end + + let(:aggregator) { Sentry.metrics_aggregator } + let(:fake_time) { Time.new(2024, 1, 1, 1, 1, 3) } + + describe '.increment' do + it 'passes default value of 1.0 with only key' do + expect(aggregator).to receive(:add).with( + :c, + 'foo', + 1.0, + unit: 'none', + tags: {}, + timestamp: nil + ) + + described_class.increment('foo') + end + + it 'passes through args to aggregator' do + expect(aggregator).to receive(:add).with( + :c, + 'foo', + 5.0, + unit: 'second', + tags: { fortytwo: 42 }, + timestamp: fake_time + ) + + described_class.increment('foo', 5.0, unit: 'second', tags: { fortytwo: 42 }, timestamp: fake_time) + end + end + + describe '.distribution' do + it 'passes through args to aggregator' do + expect(aggregator).to receive(:add).with( + :d, + 'foo', + 5.0, + unit: 'second', + tags: { fortytwo: 42 }, + timestamp: fake_time + ) + + described_class.distribution('foo', 5.0, unit: 'second', tags: { fortytwo: 42 }, timestamp: fake_time) + end + end + + describe '.set' do + it 'passes through args to aggregator' do + expect(aggregator).to receive(:add).with( + :s, + 'foo', + 'jane', + unit: 'none', + tags: { fortytwo: 42 }, + timestamp: fake_time + ) + + described_class.set('foo', 'jane', tags: { fortytwo: 42 }, timestamp: fake_time) + end + end + + describe '.gauge' do + it 'passes through args to aggregator' do + expect(aggregator).to receive(:add).with( + :g, + 'foo', + 5.0, + unit: 'second', + tags: { fortytwo: 42 }, + timestamp: fake_time + ) + + described_class.gauge('foo', 5.0, unit: 'second', tags: { fortytwo: 42 }, timestamp: fake_time) + end + end +end diff --git a/sentry-ruby/spec/sentry/transaction_spec.rb b/sentry-ruby/spec/sentry/transaction_spec.rb index 49de98f88..6dafc1371 100644 --- a/sentry-ruby/spec/sentry/transaction_spec.rb +++ b/sentry-ruby/spec/sentry/transaction_spec.rb @@ -651,4 +651,16 @@ expect(subject.contexts).to eq({ foo: { bar: 42 } }) end end + + describe "#source_low_quality?" do + it "returns true when :url" do + subject.set_name('foo', source: :url) + expect(subject.source_low_quality?).to eq(true) + end + + it "returns false otherwise" do + subject.set_name('foo', source: :view) + expect(subject.source_low_quality?).to eq(false) + end + end end diff --git a/sentry-ruby/spec/sentry/transport_spec.rb b/sentry-ruby/spec/sentry/transport_spec.rb index d872d77b9..23bf3322a 100644 --- a/sentry-ruby/spec/sentry/transport_spec.rb +++ b/sentry-ruby/spec/sentry/transport_spec.rb @@ -178,6 +178,30 @@ end end + context "metrics/statsd item" do + let(:payload) do + "foo@none:10.0|c|#tag1:42,tag2:bar|T1709042970\n" + + "bar@second:0.3:0.1:0.9:49.8:100|g|#|T1709042980" + end + + let(:envelope) do + envelope = Sentry::Envelope.new + envelope.add_item( + { type: 'statsd', length: payload.bytesize }, + payload + ) + envelope + end + + it "adds raw payload to envelope item" do + result, _ = subject.serialize_envelope(envelope) + item = result.split("\n", 2).last + item_header, item_payload = item.split("\n", 2) + expect(JSON.parse(item_header)).to eq({ 'type' => 'statsd', 'length' => 93 }) + expect(item_payload).to eq(payload) + end + end + context "oversized event" do context "due to breadcrumb" do let(:event) { client.event_from_message("foo") } diff --git a/sentry-ruby/spec/sentry_spec.rb b/sentry-ruby/spec/sentry_spec.rb index 666e84416..846f8a6a1 100644 --- a/sentry-ruby/spec/sentry_spec.rb +++ b/sentry-ruby/spec/sentry_spec.rb @@ -1048,6 +1048,14 @@ expect(described_class.backpressure_monitor).to eq(nil) end + it "flushes and kills metrics aggregator" do + perform_basic_setup { |c| c.metrics.enabled = true } + expect(described_class.metrics_aggregator).to receive(:flush).with(force: true) + expect(described_class.metrics_aggregator).to receive(:kill) + described_class.close + expect(described_class.metrics_aggregator).to eq(nil) + end + it "flushes transport" do expect(described_class.get_current_client.transport).to receive(:flush) described_class.close