From be9422ddbda95f5e7647d1064fe10a67f2e74d7c Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Fri, 16 Feb 2024 16:35:43 +0100 Subject: [PATCH 01/20] metrics wip --- sentry-ruby/lib/sentry/metrics.rb | 17 ++++ sentry-ruby/lib/sentry/metrics/aggregator.rb | 83 +++++++++++++++++++ .../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 | 30 +++++++ 7 files changed, 234 insertions(+) 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/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 diff --git a/sentry-ruby/lib/sentry/metrics.rb b/sentry-ruby/lib/sentry/metrics.rb new file mode 100644 index 000000000..da24fddca --- /dev/null +++ b/sentry-ruby/lib/sentry/metrics.rb @@ -0,0 +1,17 @@ +# 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' + +module Sentry + module Metrics + class << self + # TODO-neel-metrics define units, maybe symbols + def incr(key, value: 1.0, unit: 'none', tags: nil, timestamp: nil) + 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..7c3ee1861 --- /dev/null +++ b/sentry-ruby/lib/sentry/metrics/aggregator.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +module Sentry + module Metrics + class Aggregator + include LoggingHelper + + FLUSH_INTERVAL = 5 + ROLLUP_IN_SECONDS = 10 + + def initialize(configuration, client) + @client = client + @logger = configuration.logger + @release = configuration.release + @environment = configuration.environment + + @thread = nil + @exited = false + + @buckets = {} + end + + def add(type, + key, + value, + unit, + tags: nil, + timestamp: nil) + return unless ensure_thread + + 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(tags) + bucket_key = [type, key, unit, serialized_tags] + + # TODO lock and add to bucket + 42 + end + + def flush + # TODO + 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 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 + + def serialize_tags(tags) + # TODO support array tags + return [] unless tags + tags.map { |k, v| [k.to_s, v.to_s] } + 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..0250be8c5 --- /dev/null +++ b/sentry-ruby/lib/sentry/metrics/set_metric.rb @@ -0,0 +1,30 @@ +# frozen_string_literal: true + +require 'set' +require 'zlib' + +module Sentry + module Metrics + class SetMetric < Metric + attr_reader :value + + def initialize(value) + @value = Set[value.to_f] + end + + def add(value) + @value << value + end + + def serialize + value.map do |v| + x.is_a?(String) ? Zlib.crc32(x) : x.to_i + end + end + + def weight + value.size + end + end + end +end From ee59473b237acb7c8ff0e6a31a108af9279f87c6 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Wed, 21 Feb 2024 15:11:23 +0100 Subject: [PATCH 02/20] aggregator add impl --- sentry-ruby/lib/sentry/metrics/aggregator.rb | 44 ++++++++++++++++---- 1 file changed, 37 insertions(+), 7 deletions(-) diff --git a/sentry-ruby/lib/sentry/metrics/aggregator.rb b/sentry-ruby/lib/sentry/metrics/aggregator.rb index 7c3ee1861..d87919fa4 100644 --- a/sentry-ruby/lib/sentry/metrics/aggregator.rb +++ b/sentry-ruby/lib/sentry/metrics/aggregator.rb @@ -8,6 +8,13 @@ class Aggregator FLUSH_INTERVAL = 5 ROLLUP_IN_SECONDS = 10 + METRIC_TYPES = { + c: CounterMetric, + d: DistributionMetric, + g: GaugeMetric, + s: SetMetric + } + def initialize(configuration, client) @client = client @logger = configuration.logger @@ -16,8 +23,13 @@ def initialize(configuration, client) @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, @@ -34,19 +46,30 @@ def add(type, # 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(tags) bucket_key = [type, key, unit, serialized_tags] - # TODO lock and add to bucket - 42 + @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 - # TODO + @mutex.synchronize do + log_debug("[Metrics::Aggregator] current bucket state: #{@buckets}") + # TODO + end end def kill - log_debug("[Metrics::Aggregator] killing thread") + log_debug('[Metrics::Aggregator] killing thread') @exited = true @thread&.kill @@ -68,15 +91,22 @@ def ensure_thread true rescue ThreadError - log_debug("[Metrics::Aggregator] thread creation failed") + log_debug('[Metrics::Aggregator] thread creation failed') @exited = true false end def serialize_tags(tags) - # TODO support array tags return [] unless tags - tags.map { |k, v| [k.to_s, v.to_s] } + + # important to sort for key consistency + tags.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.flatten.sort end end end From 675292b2db65f6924afd61df54d699e737678244 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Wed, 21 Feb 2024 15:44:01 +0100 Subject: [PATCH 03/20] config and apis --- sentry-ruby/lib/sentry-ruby.rb | 13 +++++++++++++ sentry-ruby/lib/sentry/configuration.rb | 7 +++++++ sentry-ruby/lib/sentry/metrics.rb | 17 ++++++++++++++++- sentry-ruby/lib/sentry/metrics/aggregator.rb | 19 ++++++++----------- 4 files changed, 44 insertions(+), 12 deletions(-) diff --git a/sentry-ruby/lib/sentry-ruby.rb b/sentry-ruby/lib/sentry-ruby.rb index 5c750f373..9a201b108 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.enable_metrics ? Sentry::Metrics::Aggregator.new(config, client) : nil exception_locals_tp.enable if config.include_local_variables at_exit { close } end @@ -244,6 +250,13 @@ def close @backpressure_monitor = nil end + if @metrics_aggregator + # TODO-neel-metrics force flush? + @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..420516f8d 100644 --- a/sentry-ruby/lib/sentry/configuration.rb +++ b/sentry-ruby/lib/sentry/configuration.rb @@ -254,6 +254,12 @@ def capture_exception_frame_locals=(value) # @return [Boolean, nil] attr_reader :enable_tracing + # 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 :enable_metrics + # Send diagnostic client reports about dropped events, true by default # tries to attach to an existing envelope max once every 30s # @return [Boolean] @@ -383,6 +389,7 @@ def initialize self.rack_env_whitelist = RACK_ENV_WHITELIST_DEFAULT self.traces_sampler = nil self.enable_tracing = nil + self.enable_metrics = false @transport = Transport::Configuration.new @cron = Cron::Configuration.new diff --git a/sentry-ruby/lib/sentry/metrics.rb b/sentry-ruby/lib/sentry/metrics.rb index da24fddca..26b7c70e9 100644 --- a/sentry-ruby/lib/sentry/metrics.rb +++ b/sentry-ruby/lib/sentry/metrics.rb @@ -5,12 +5,27 @@ 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 # TODO-neel-metrics define units, maybe symbols - def incr(key, value: 1.0, unit: 'none', tags: nil, timestamp: nil) + + def incr(key, value = 1.0, unit = 'none', tags: {}, timestamp: nil) + Sentry.metrics_aggregator&.add(:c, key, value, unit, tags: tags, timestamp: timestamp) + end + + def distribution(key, value, unit = 'none', tags: {}, timestamp: nil) + Sentry.metrics_aggregator&.add(:d, key, value, unit, tags: tags, timestamp: timestamp) + end + + def set(key, value, unit = 'none', tags: {}, timestamp: nil) + Sentry.metrics_aggregator&.add(:s, key, value, unit, tags: tags, timestamp: timestamp) + end + + def gauge(key, value, unit = 'none', tags: {}, timestamp: nil) + Sentry.metrics_aggregator&.add(:g, key, value, unit, tags: tags, timestamp: timestamp) end end end diff --git a/sentry-ruby/lib/sentry/metrics/aggregator.rb b/sentry-ruby/lib/sentry/metrics/aggregator.rb index d87919fa4..111aeede0 100644 --- a/sentry-ruby/lib/sentry/metrics/aggregator.rb +++ b/sentry-ruby/lib/sentry/metrics/aggregator.rb @@ -18,8 +18,7 @@ class Aggregator def initialize(configuration, client) @client = client @logger = configuration.logger - @release = configuration.release - @environment = configuration.environment + @default_tags = { 'release' => configuration.release, 'environment' => configuration.environment } @thread = nil @exited = false @@ -36,7 +35,7 @@ def add(type, key, value, unit, - tags: nil, + tags: {}, timestamp: nil) return unless ensure_thread @@ -47,7 +46,7 @@ def add(type, # and buckets into 10 second intervals bucket_timestamp = (timestamp / ROLLUP_IN_SECONDS) * ROLLUP_IN_SECONDS - serialized_tags = serialize_tags(tags) + serialized_tags = serialize_tags(tags.merge(@default_tags)) bucket_key = [type, key, unit, serialized_tags] @mutex.synchronize do @@ -61,7 +60,7 @@ def add(type, end end - def flush + def flush(force: false) @mutex.synchronize do log_debug("[Metrics::Aggregator] current bucket state: #{@buckets}") # TODO @@ -96,17 +95,15 @@ def ensure_thread false end + # important to sort for key consistency def serialize_tags(tags) - return [] unless tags - - # important to sort for key consistency - tags.map do |k, v| + 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] + [[k.to_s, v.to_s]] end - end.flatten.sort + end.sort end end end From 73e5e047fa2db952880bdef4ae6157bd10eabf09 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Thu, 22 Feb 2024 15:41:28 +0100 Subject: [PATCH 04/20] encode statsd format and sanitization --- sentry-ruby/lib/sentry-ruby.rb | 1 - sentry-ruby/lib/sentry/metrics/aggregator.rb | 53 ++++++++++++++++++-- 2 files changed, 49 insertions(+), 5 deletions(-) diff --git a/sentry-ruby/lib/sentry-ruby.rb b/sentry-ruby/lib/sentry-ruby.rb index 9a201b108..7dbb12d39 100644 --- a/sentry-ruby/lib/sentry-ruby.rb +++ b/sentry-ruby/lib/sentry-ruby.rb @@ -251,7 +251,6 @@ def close end if @metrics_aggregator - # TODO-neel-metrics force flush? @metrics_aggregator.flush(force: true) @metrics_aggregator.kill @metrics_aggregator = nil diff --git a/sentry-ruby/lib/sentry/metrics/aggregator.rb b/sentry-ruby/lib/sentry/metrics/aggregator.rb index 111aeede0..a65ba6e6c 100644 --- a/sentry-ruby/lib/sentry/metrics/aggregator.rb +++ b/sentry-ruby/lib/sentry/metrics/aggregator.rb @@ -8,6 +8,9 @@ class Aggregator 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, @@ -61,10 +64,14 @@ def add(type, end def flush(force: false) - @mutex.synchronize do - log_debug("[Metrics::Aggregator] current bucket state: #{@buckets}") - # TODO - end + log_debug("[Metrics::Aggregator] current bucket state: #{@buckets}") + + flushable_buckets = get_flushable_buckets!(force) + return if flushable_buckets.empty? + + payload = serialize_buckets(flushable_buckets) + log_debug("[Metrics::Aggregator] flushing buckets: #{flushable_buckets}") + log_debug("[Metrics::Aggregator] payload: #{payload}") end def kill @@ -105,6 +112,44 @@ def serialize_tags(tags) 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 end end end From 9569bf94f954261568ad7075ea3f49f48b359ace Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Thu, 22 Feb 2024 17:07:19 +0100 Subject: [PATCH 05/20] capture envelope --- sentry-ruby/lib/sentry/envelope.rb | 11 ++++++----- sentry-ruby/lib/sentry/metrics/aggregator.rb | 13 ++++++++++++- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/sentry-ruby/lib/sentry/envelope.rb b/sentry-ruby/lib/sentry/envelope.rb index de981d5f2..7b6272229 100644 --- a/sentry-ruby/lib/sentry/envelope.rb +++ b/sentry-ruby/lib/sentry/envelope.rb @@ -7,11 +7,12 @@ class Item STACKTRACE_FRAME_LIMIT_ON_OVERSIZED_PAYLOAD = 500 MAX_SERIALIZED_PAYLOAD_SIZE = 1024 * 1000 - attr_accessor :headers, :payload + attr_accessor :headers, :payload, :is_json - def initialize(headers, payload) + def initialize(headers, payload, is_json: true) @headers = headers @payload = payload + @is_json = is_json end def type @@ -19,7 +20,7 @@ def type end def to_s - [JSON.generate(@headers), JSON.generate(@payload)].join("\n") + [JSON.generate(@headers), @is_json ? JSON.generate(@payload) : @payload].join("\n") end def serialize @@ -78,8 +79,8 @@ def initialize(headers = {}) @items = [] end - def add_item(headers, payload) - @items << Item.new(headers, payload) + def add_item(headers, payload, is_json: true) + @items << Item.new(headers, payload, is_json: is_json) end def item_types diff --git a/sentry-ruby/lib/sentry/metrics/aggregator.rb b/sentry-ruby/lib/sentry/metrics/aggregator.rb index a65ba6e6c..884ec751c 100644 --- a/sentry-ruby/lib/sentry/metrics/aggregator.rb +++ b/sentry-ruby/lib/sentry/metrics/aggregator.rb @@ -70,8 +70,19 @@ def flush(force: false) return if flushable_buckets.empty? payload = serialize_buckets(flushable_buckets) + envelope = Envelope.new + envelope.add_item( + { type: 'statsd', length: payload.bytesize }, + payload, + is_json: false + ) + log_debug("[Metrics::Aggregator] flushing buckets: #{flushable_buckets}") log_debug("[Metrics::Aggregator] payload: #{payload}") + + Sentry.background_worker.perform do + @client.transport.send_envelope(envelope) + end end def kill @@ -136,7 +147,7 @@ def serialize_buckets(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(',') + sanitized_tags = tags.map { |k, v| "#{sanitize_key(k)}:#{sanitize_value(v)}" }.join(',') "#{sanitize_key(key)}@#{unit}:#{values}|#{type}|\##{sanitized_tags}|T#{timestamp}" end From 8efde86cb8f5546ec61907b47ed7c8e860a51062 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 26 Feb 2024 15:14:18 +0100 Subject: [PATCH 06/20] fix set --- sentry-ruby/lib/sentry/metrics/set_metric.rb | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/sentry-ruby/lib/sentry/metrics/set_metric.rb b/sentry-ruby/lib/sentry/metrics/set_metric.rb index 0250be8c5..02c5c26a6 100644 --- a/sentry-ruby/lib/sentry/metrics/set_metric.rb +++ b/sentry-ruby/lib/sentry/metrics/set_metric.rb @@ -9,7 +9,7 @@ class SetMetric < Metric attr_reader :value def initialize(value) - @value = Set[value.to_f] + @value = Set[value] end def add(value) @@ -17,9 +17,7 @@ def add(value) end def serialize - value.map do |v| - x.is_a?(String) ? Zlib.crc32(x) : x.to_i - end + value.map { |x| x.is_a?(String) ? Zlib.crc32(x) : x.to_i } end def weight From 512408cde43b119cf082391767fb8ce39f76c96c Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 26 Feb 2024 16:12:36 +0100 Subject: [PATCH 07/20] add transaction name to tags --- sentry-ruby/lib/sentry/metrics/aggregator.rb | 19 ++++++++++++++++++- sentry-ruby/lib/sentry/transaction.rb | 10 +++++----- 2 files changed, 23 insertions(+), 6 deletions(-) diff --git a/sentry-ruby/lib/sentry/metrics/aggregator.rb b/sentry-ruby/lib/sentry/metrics/aggregator.rb index 884ec751c..056e9cf43 100644 --- a/sentry-ruby/lib/sentry/metrics/aggregator.rb +++ b/sentry-ruby/lib/sentry/metrics/aggregator.rb @@ -49,7 +49,7 @@ def add(type, # and buckets into 10 second intervals bucket_timestamp = (timestamp / ROLLUP_IN_SECONDS) * ROLLUP_IN_SECONDS - serialized_tags = serialize_tags(tags.merge(@default_tags)) + serialized_tags = serialize_tags(get_updated_tags(tags)) bucket_key = [type, key, unit, serialized_tags] @mutex.synchronize do @@ -161,6 +161,23 @@ def sanitize_key(key) def sanitize_value(value) value.gsub(VALUE_SANITIZATION_REGEX, '') end + + def get_transaction_name + transaction = Sentry.get_current_scope&.get_transaction + return nil unless transaction + return nil if transaction.source_low_quality? + + 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/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 From bdac0667e93014021594d60bd5f1ec83bbd8f2b4 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 27 Feb 2024 15:35:55 +0100 Subject: [PATCH 08/20] Specs --- sentry-ruby/spec/sentry/transaction_spec.rb | 12 ++++++++++ sentry-ruby/spec/sentry/transport_spec.rb | 25 +++++++++++++++++++++ sentry-ruby/spec/sentry_spec.rb | 8 +++++++ 3 files changed, 45 insertions(+) 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..ec1ec3f91 100644 --- a/sentry-ruby/spec/sentry/transport_spec.rb +++ b/sentry-ruby/spec/sentry/transport_spec.rb @@ -178,6 +178,31 @@ 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, + is_json: false + ) + 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..3e04a31f9 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.enable_metrics = 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 From 3f51c15a0fb6ef1c86298a2dd08da35106a98007 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 27 Feb 2024 15:47:21 +0100 Subject: [PATCH 09/20] changelog --- CHANGELOG.md | 32 ++++++++++++++++++++++++++++++++ 1 file changed, 32 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 88b654cac..dba8d7a30 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.enable_metrics = true + end + ``` + + And then in your application code, collect metrics as follows: + + ```ruby + # increment a simple counter + Sentry::Metrics.incr('button_click') + # set a value, unit and tags + Sentry::Metrics.incr('time', 5, 'second', tags: { browser:' firefox' }) + + # distribution - get statistical aggregates from an array of observations + Sentry::Metrics.distribution('page_load', 15.0, 'millisecond') + + # gauge - record statistical aggregates directly on the SDK, more space efficient + Sentry::Metrics.gauge('page_load', 15.0, 'millisecond') + + # set - get unique counts of elements + Sentry::Metrics.set('user_view', 'jane') + ``` ### Bug Fixes From c1217cd450adb5343ed287aa92b8a6d45c5bb91d Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 27 Feb 2024 16:42:55 +0100 Subject: [PATCH 10/20] metric specs --- CHANGELOG.md | 6 +- sentry-ruby/lib/sentry/metrics.rb | 18 +- sentry-ruby/lib/sentry/metrics/aggregator.rb | 18 +- .../spec/sentry/metrics/aggregator_spec.rb | 299 ++++++++++++++++++ .../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 +++++ 10 files changed, 535 insertions(+), 21 deletions(-) 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 dba8d7a30..bd8bd017b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,13 +26,13 @@ # increment a simple counter Sentry::Metrics.incr('button_click') # set a value, unit and tags - Sentry::Metrics.incr('time', 5, 'second', tags: { browser:' firefox' }) + Sentry::Metrics.incr('time', 5, unit: 'second', tags: { browser:' firefox' }) # distribution - get statistical aggregates from an array of observations - Sentry::Metrics.distribution('page_load', 15.0, 'millisecond') + 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, 'millisecond') + Sentry::Metrics.gauge('page_load', 15.0, unit: 'millisecond') # set - get unique counts of elements Sentry::Metrics.set('user_view', 'jane') diff --git a/sentry-ruby/lib/sentry/metrics.rb b/sentry-ruby/lib/sentry/metrics.rb index 26b7c70e9..ab927b1c5 100644 --- a/sentry-ruby/lib/sentry/metrics.rb +++ b/sentry-ruby/lib/sentry/metrics.rb @@ -10,22 +10,20 @@ module Sentry module Metrics class << self - # TODO-neel-metrics define units, maybe symbols - - def incr(key, value = 1.0, unit = 'none', tags: {}, timestamp: nil) - Sentry.metrics_aggregator&.add(:c, key, value, unit, tags: tags, timestamp: timestamp) + def incr(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, tags: tags, timestamp: timestamp) + 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, tags: tags, timestamp: timestamp) + 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, tags: tags, timestamp: timestamp) + 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 diff --git a/sentry-ruby/lib/sentry/metrics/aggregator.rb b/sentry-ruby/lib/sentry/metrics/aggregator.rb index 056e9cf43..3cc75bda8 100644 --- a/sentry-ruby/lib/sentry/metrics/aggregator.rb +++ b/sentry-ruby/lib/sentry/metrics/aggregator.rb @@ -18,10 +18,16 @@ class Aggregator s: SetMetric } + # exposed only for testing + attr_reader :thread, :buckets, :flush_shift + def initialize(configuration, client) @client = client @logger = configuration.logger - @default_tags = { 'release' => configuration.release, 'environment' => configuration.environment } + + @default_tags = {} + @default_tags['release'] = configuration.release if configuration.release + @default_tags['environment'] = configuration.environment if configuration.environment @thread = nil @exited = false @@ -37,10 +43,11 @@ def initialize(configuration, client) def add(type, key, value, - unit, + 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 @@ -64,8 +71,6 @@ def add(type, end def flush(force: false) - log_debug("[Metrics::Aggregator] current bucket state: #{@buckets}") - flushable_buckets = get_flushable_buckets!(force) return if flushable_buckets.empty? @@ -77,9 +82,6 @@ def flush(force: false) is_json: false ) - log_debug("[Metrics::Aggregator] flushing buckets: #{flushable_buckets}") - log_debug("[Metrics::Aggregator] payload: #{payload}") - Sentry.background_worker.perform do @client.transport.send_envelope(envelope) end @@ -100,7 +102,7 @@ def ensure_thread @thread = Thread.new do loop do - # TODO use event for force flush later + # TODO-neel-metrics use event for force flush later sleep(FLUSH_INTERVAL) flush end 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..abe6d00b5 --- /dev/null +++ b/sentry-ruby/spec/sentry/metrics/aggregator_spec.rb @@ -0,0 +1,299 @@ +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.enable_metrics = 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 + let(:transaction) { Sentry.start_transaction(name: 'foo', source: :view) } + before { Sentry.get_current_scope.set_span(transaction) } + + it 'has the transaction name in tags serialized in the bucket metric key' do + 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 + transaction.set_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 }) + expect(item.is_json).to eq(false) + + 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 }) + expect(item.is_json).to eq(false) + + 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..c74e84fa1 --- /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.enable_metrics = true + end + end + + let(:aggregator) { Sentry.metrics_aggregator } + let(:fake_time) { Time.new(2024, 1, 1, 1, 1, 3) } + + describe '.incr' 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.incr('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.incr('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 From 8f9dabbf5be758631e05dedf98793ab967f69317 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Thu, 29 Feb 2024 14:25:19 +0100 Subject: [PATCH 11/20] incr -> increment --- CHANGELOG.md | 4 ++-- sentry-ruby/lib/sentry/metrics.rb | 2 +- sentry-ruby/spec/sentry/metrics_spec.rb | 6 +++--- 3 files changed, 6 insertions(+), 6 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index bd8bd017b..5532575f6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,9 +24,9 @@ ```ruby # increment a simple counter - Sentry::Metrics.incr('button_click') + Sentry::Metrics.increment('button_click') # set a value, unit and tags - Sentry::Metrics.incr('time', 5, unit: 'second', tags: { browser:' firefox' }) + 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') diff --git a/sentry-ruby/lib/sentry/metrics.rb b/sentry-ruby/lib/sentry/metrics.rb index ab927b1c5..190c026aa 100644 --- a/sentry-ruby/lib/sentry/metrics.rb +++ b/sentry-ruby/lib/sentry/metrics.rb @@ -10,7 +10,7 @@ module Sentry module Metrics class << self - def incr(key, value = 1.0, unit: 'none', tags: {}, timestamp: nil) + 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 diff --git a/sentry-ruby/spec/sentry/metrics_spec.rb b/sentry-ruby/spec/sentry/metrics_spec.rb index c74e84fa1..1ca8ee7a0 100644 --- a/sentry-ruby/spec/sentry/metrics_spec.rb +++ b/sentry-ruby/spec/sentry/metrics_spec.rb @@ -10,7 +10,7 @@ let(:aggregator) { Sentry.metrics_aggregator } let(:fake_time) { Time.new(2024, 1, 1, 1, 1, 3) } - describe '.incr' do + describe '.increment' do it 'passes default value of 1.0 with only key' do expect(aggregator).to receive(:add).with( :c, @@ -21,7 +21,7 @@ timestamp: nil ) - described_class.incr('foo') + described_class.increment('foo') end it 'passes through args to aggregator' do @@ -34,7 +34,7 @@ timestamp: fake_time ) - described_class.incr('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 From ff3814fefb47cf9c4b29f6007b7a8f5b53fdbdd3 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 5 Mar 2024 14:20:32 +0100 Subject: [PATCH 12/20] Move config to separate metrics obj --- CHANGELOG.md | 2 +- sentry-ruby/lib/sentry-ruby.rb | 2 +- sentry-ruby/lib/sentry/configuration.rb | 13 ++++++------- sentry-ruby/lib/sentry/metrics/configuration.rb | 17 +++++++++++++++++ .../spec/sentry/metrics/aggregator_spec.rb | 2 +- sentry-ruby/spec/sentry/metrics_spec.rb | 2 +- sentry-ruby/spec/sentry_spec.rb | 2 +- 7 files changed, 28 insertions(+), 12 deletions(-) create mode 100644 sentry-ruby/lib/sentry/metrics/configuration.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 5532575f6..ee66bffa6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ ```ruby Sentry.init do |config| # ... - config.enable_metrics = true + config.metrics.enabled = true end ``` diff --git a/sentry-ruby/lib/sentry-ruby.rb b/sentry-ruby/lib/sentry-ruby.rb index 7dbb12d39..79417d40f 100644 --- a/sentry-ruby/lib/sentry-ruby.rb +++ b/sentry-ruby/lib/sentry-ruby.rb @@ -229,7 +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.enable_metrics ? Sentry::Metrics::Aggregator.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 diff --git a/sentry-ruby/lib/sentry/configuration.rb b/sentry-ruby/lib/sentry/configuration.rb index 420516f8d..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 @@ -254,12 +259,6 @@ def capture_exception_frame_locals=(value) # @return [Boolean, nil] attr_reader :enable_tracing - # 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 :enable_metrics - # Send diagnostic client reports about dropped events, true by default # tries to attach to an existing envelope max once every 30s # @return [Boolean] @@ -389,10 +388,10 @@ def initialize self.rack_env_whitelist = RACK_ENV_WHITELIST_DEFAULT self.traces_sampler = nil self.enable_tracing = nil - self.enable_metrics = false @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/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/spec/sentry/metrics/aggregator_spec.rb b/sentry-ruby/spec/sentry/metrics/aggregator_spec.rb index abe6d00b5..0b0a4dee4 100644 --- a/sentry-ruby/spec/sentry/metrics/aggregator_spec.rb +++ b/sentry-ruby/spec/sentry/metrics/aggregator_spec.rb @@ -8,7 +8,7 @@ before do perform_basic_setup do |config| - config.enable_metrics = true + config.metrics.enabled = true config.enable_tracing = true config.release = 'test-release' config.environment = 'test' diff --git a/sentry-ruby/spec/sentry/metrics_spec.rb b/sentry-ruby/spec/sentry/metrics_spec.rb index 1ca8ee7a0..04acc66b3 100644 --- a/sentry-ruby/spec/sentry/metrics_spec.rb +++ b/sentry-ruby/spec/sentry/metrics_spec.rb @@ -3,7 +3,7 @@ RSpec.describe Sentry::Metrics do before do perform_basic_setup do |config| - config.enable_metrics = true + config.metrics.enabled = true end end diff --git a/sentry-ruby/spec/sentry_spec.rb b/sentry-ruby/spec/sentry_spec.rb index 3e04a31f9..846f8a6a1 100644 --- a/sentry-ruby/spec/sentry_spec.rb +++ b/sentry-ruby/spec/sentry_spec.rb @@ -1049,7 +1049,7 @@ end it "flushes and kills metrics aggregator" do - perform_basic_setup { |c| c.enable_metrics = true } + 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 From f7163ef77f2348fa94fb606b1aa77c8931d3ac1d Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 5 Mar 2024 16:26:02 +0100 Subject: [PATCH 13/20] Use scope name/source, not transaction --- sentry-ruby/lib/sentry/metrics/aggregator.rb | 8 ++++---- sentry-ruby/lib/sentry/scope.rb | 6 ++++++ sentry-ruby/spec/sentry/metrics/aggregator_spec.rb | 6 ++---- 3 files changed, 12 insertions(+), 8 deletions(-) diff --git a/sentry-ruby/lib/sentry/metrics/aggregator.rb b/sentry-ruby/lib/sentry/metrics/aggregator.rb index 3cc75bda8..695f7c912 100644 --- a/sentry-ruby/lib/sentry/metrics/aggregator.rb +++ b/sentry-ruby/lib/sentry/metrics/aggregator.rb @@ -165,11 +165,11 @@ def sanitize_value(value) end def get_transaction_name - transaction = Sentry.get_current_scope&.get_transaction - return nil unless transaction - return nil if transaction.source_low_quality? + scope = Sentry.get_current_scope + return nil unless scope && scope.transaction_name + return nil if scope.transaction_source_low_quality? - transaction.name + scope.transaction_name end def get_updated_tags(tags) 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/spec/sentry/metrics/aggregator_spec.rb b/sentry-ruby/spec/sentry/metrics/aggregator_spec.rb index 0b0a4dee4..6f1f24c28 100644 --- a/sentry-ruby/spec/sentry/metrics/aggregator_spec.rb +++ b/sentry-ruby/spec/sentry/metrics/aggregator_spec.rb @@ -124,17 +124,15 @@ end context 'with running transaction' do - let(:transaction) { Sentry.start_transaction(name: 'foo', source: :view) } - before { Sentry.get_current_scope.set_span(transaction) } - 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 - transaction.set_name('foo', source: :url) + 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']) From ff9792fcfd4c6bc6cc82db46db041b28440cd65c Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 11 Mar 2024 19:44:47 +0100 Subject: [PATCH 14/20] Remove is_json for envelope and use string check --- sentry-ruby/lib/sentry/envelope.rb | 11 +++++------ sentry-ruby/lib/sentry/metrics/aggregator.rb | 3 +-- sentry-ruby/spec/sentry/metrics/aggregator_spec.rb | 2 -- sentry-ruby/spec/sentry/transport_spec.rb | 3 +-- 4 files changed, 7 insertions(+), 12 deletions(-) diff --git a/sentry-ruby/lib/sentry/envelope.rb b/sentry-ruby/lib/sentry/envelope.rb index 7b6272229..6d9ff56ff 100644 --- a/sentry-ruby/lib/sentry/envelope.rb +++ b/sentry-ruby/lib/sentry/envelope.rb @@ -7,12 +7,11 @@ class Item STACKTRACE_FRAME_LIMIT_ON_OVERSIZED_PAYLOAD = 500 MAX_SERIALIZED_PAYLOAD_SIZE = 1024 * 1000 - attr_accessor :headers, :payload, :is_json + attr_accessor :headers, :payload - def initialize(headers, payload, is_json: true) + def initialize(headers, payload) @headers = headers @payload = payload - @is_json = is_json end def type @@ -20,7 +19,7 @@ def type end def to_s - [JSON.generate(@headers), @is_json ? JSON.generate(@payload) : @payload].join("\n") + [JSON.generate(@headers), @payload.is_a?(String) ? @payload : JSON.generate(@payload)].join("\n") end def serialize @@ -79,8 +78,8 @@ def initialize(headers = {}) @items = [] end - def add_item(headers, payload, is_json: true) - @items << Item.new(headers, payload, is_json: is_json) + def add_item(headers, payload) + @items << Item.new(headers, payload) end def item_types diff --git a/sentry-ruby/lib/sentry/metrics/aggregator.rb b/sentry-ruby/lib/sentry/metrics/aggregator.rb index 695f7c912..f7d6b17b1 100644 --- a/sentry-ruby/lib/sentry/metrics/aggregator.rb +++ b/sentry-ruby/lib/sentry/metrics/aggregator.rb @@ -78,8 +78,7 @@ def flush(force: false) envelope = Envelope.new envelope.add_item( { type: 'statsd', length: payload.bytesize }, - payload, - is_json: false + payload ) Sentry.background_worker.perform do diff --git a/sentry-ruby/spec/sentry/metrics/aggregator_spec.rb b/sentry-ruby/spec/sentry/metrics/aggregator_spec.rb index 6f1f24c28..d95ffbde8 100644 --- a/sentry-ruby/spec/sentry/metrics/aggregator_spec.rb +++ b/sentry-ruby/spec/sentry/metrics/aggregator_spec.rb @@ -241,7 +241,6 @@ item = envelope.items.first expect(item.headers).to eq({ type: 'statsd', length: item.payload.bytesize }) - expect(item.is_json).to eq(false) 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}") @@ -270,7 +269,6 @@ item = envelope.items.first expect(item.headers).to eq({ type: 'statsd', length: item.payload.bytesize }) - expect(item.is_json).to eq(false) 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}") diff --git a/sentry-ruby/spec/sentry/transport_spec.rb b/sentry-ruby/spec/sentry/transport_spec.rb index ec1ec3f91..23bf3322a 100644 --- a/sentry-ruby/spec/sentry/transport_spec.rb +++ b/sentry-ruby/spec/sentry/transport_spec.rb @@ -188,8 +188,7 @@ envelope = Sentry::Envelope.new envelope.add_item( { type: 'statsd', length: payload.bytesize }, - payload, - is_json: false + payload ) envelope end From 9ba1a4ea23a3ab353fe400fe0a36b0e4d39c149d Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 11 Mar 2024 19:46:21 +0100 Subject: [PATCH 15/20] trigger ci From 478a78b16ee6c85318b9efd973d69216f3e0eb88 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 11 Mar 2024 19:48:46 +0100 Subject: [PATCH 16/20] trigger ci From 0e87bab9abd91e53fc7fe9f295a3ee560281ae39 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 11 Mar 2024 19:53:22 +0100 Subject: [PATCH 17/20] remove io-console pin --- Gemfile | 2 -- 1 file changed, 2 deletions(-) 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" From e6bbb90f93967b68702e493ecb8b6dd4881a9307 Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Mon, 4 Mar 2024 15:57:05 +0100 Subject: [PATCH 18/20] Add Sentry::Metrics.timing API to measure blocks --- CHANGELOG.md | 6 +++ sentry-ruby/lib/sentry/metrics.rb | 17 ++++++ sentry-ruby/lib/sentry/metrics/timing.rb | 43 +++++++++++++++ .../spec/sentry/metrics/timing_spec.rb | 54 +++++++++++++++++++ sentry-ruby/spec/sentry/metrics_spec.rb | 25 +++++++++ 5 files changed, 145 insertions(+) create mode 100644 sentry-ruby/lib/sentry/metrics/timing.rb create mode 100644 sentry-ruby/spec/sentry/metrics/timing_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index ee66bffa6..480ab8720 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -7,6 +7,7 @@ - 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) + - Add `Sentry::Metrics.timing` API for measuring block duration [#2254](https://github.com/getsentry/sentry-ruby/pull/2254) 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. @@ -36,6 +37,11 @@ # set - get unique counts of elements Sentry::Metrics.set('user_view', 'jane') + + # timing - measure duration of code block, defaults to seconds + Sentry::Metrics.timing('how_long') { sleep(1) } + # timing - measure duration of code block in other duraton units + Sentry::Metrics.timing('how_long_ms', unit: 'millisecond') { sleep(0.5) } ``` ### Bug Fixes diff --git a/sentry-ruby/lib/sentry/metrics.rb b/sentry-ruby/lib/sentry/metrics.rb index 190c026aa..c8d043c51 100644 --- a/sentry-ruby/lib/sentry/metrics.rb +++ b/sentry-ruby/lib/sentry/metrics.rb @@ -5,10 +5,15 @@ require 'sentry/metrics/distribution_metric' require 'sentry/metrics/gauge_metric' require 'sentry/metrics/set_metric' +require 'sentry/metrics/timing' require 'sentry/metrics/aggregator' module Sentry module Metrics + DURATION_UNITS = %w[nanosecond microsecond millisecond second minute hour day week] + INFORMATION_UNITS = %w[bit byte kilobyte kibibyte megabyte mebibyte gigabyte gibibyte terabyte tebibyte petabyte pebibyte exabyte exbibyte] + FRACTIONAL_UNITS = %w[ratio percent] + 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) @@ -25,6 +30,18 @@ def set(key, value, unit: 'none', tags: {}, timestamp: nil) def gauge(key, value, unit: 'none', tags: {}, timestamp: nil) Sentry.metrics_aggregator&.add(:g, key, value, unit: unit, tags: tags, timestamp: timestamp) end + + def timing(key, unit: 'second', tags: {}, timestamp: nil, &block) + return unless Sentry.metrics_aggregator + return unless block_given? + return unless DURATION_UNITS.include?(unit) + + start = Timing.send(unit.to_sym) + yield + value = Timing.send(unit.to_sym) - start + + Sentry.metrics_aggregator.add(:d, key, value, unit: unit, tags: tags, timestamp: timestamp) + end end end end diff --git a/sentry-ruby/lib/sentry/metrics/timing.rb b/sentry-ruby/lib/sentry/metrics/timing.rb new file mode 100644 index 000000000..510434583 --- /dev/null +++ b/sentry-ruby/lib/sentry/metrics/timing.rb @@ -0,0 +1,43 @@ +# frozen_string_literal: true + +module Sentry + module Metrics + module Timing + class << self + def nanosecond + time = Sentry.utc_now + time.to_i * (10 ** 9) + time.nsec + end + + def microsecond + time = Sentry.utc_now + time.to_i * (10 ** 6) + time.usec + end + + def millisecond + Sentry.utc_now.to_i * (10 ** 3) + end + + def second + Sentry.utc_now.to_i + end + + def minute + Sentry.utc_now.to_i / 60.0 + end + + def hour + Sentry.utc_now.to_i / 3600.0 + end + + def day + Sentry.utc_now.to_i / (3600.0 * 24.0) + end + + def week + Sentry.utc_now.to_i / (3600.0 * 24.0 * 7.0) + end + end + end + end +end diff --git a/sentry-ruby/spec/sentry/metrics/timing_spec.rb b/sentry-ruby/spec/sentry/metrics/timing_spec.rb new file mode 100644 index 000000000..8142fbb4d --- /dev/null +++ b/sentry-ruby/spec/sentry/metrics/timing_spec.rb @@ -0,0 +1,54 @@ +require 'spec_helper' + +RSpec.describe Sentry::Metrics::Timing do + let(:fake_time) { Time.new(2024, 1, 2, 3, 4, 5) } + before { allow(Time).to receive(:now).and_return(fake_time) } + + describe '.nanosecond' do + it 'returns nanoseconds' do + expect(described_class.nanosecond).to eq(fake_time.to_i * 10 ** 9) + end + end + + describe '.microsecond' do + it 'returns microseconds' do + expect(described_class.microsecond).to eq(fake_time.to_i * 10 ** 6) + end + end + + describe '.millisecond' do + it 'returns milliseconds' do + expect(described_class.millisecond).to eq(fake_time.to_i * 10 ** 3) + end + end + + describe '.second' do + it 'returns seconds' do + expect(described_class.second).to eq(fake_time.to_i) + end + end + + describe '.minute' do + it 'returns minutes' do + expect(described_class.minute).to eq(fake_time.to_i / 60.0) + end + end + + describe '.hour' do + it 'returns hours' do + expect(described_class.hour).to eq(fake_time.to_i / 3600.0) + end + end + + describe '.day' do + it 'returns days' do + expect(described_class.day).to eq(fake_time.to_i / (3600.0 * 24.0)) + end + end + + describe '.week' do + it 'returns weeks' do + expect(described_class.week).to eq(fake_time.to_i / (3600.0 * 24.0 * 7.0)) + end + end +end diff --git a/sentry-ruby/spec/sentry/metrics_spec.rb b/sentry-ruby/spec/sentry/metrics_spec.rb index 04acc66b3..9989876a4 100644 --- a/sentry-ruby/spec/sentry/metrics_spec.rb +++ b/sentry-ruby/spec/sentry/metrics_spec.rb @@ -82,4 +82,29 @@ described_class.gauge('foo', 5.0, unit: 'second', tags: { fortytwo: 42 }, timestamp: fake_time) end end + + describe '.timing' do + it 'does nothing without a block' do + expect(aggregator).not_to receive(:add) + described_class.timing('foo') + end + + it 'does nothing with a non-duration unit' do + expect(aggregator).not_to receive(:add) + described_class.timing('foo', unit: 'ratio') { } + end + + it 'measures time taken as distribution and passes through args to aggregator' do + expect(aggregator).to receive(:add).with( + :d, + 'foo', + an_instance_of(Integer), + unit: 'millisecond', + tags: { fortytwo: 42 }, + timestamp: fake_time + ) + + described_class.timing('foo', unit: 'millisecond', tags: { fortytwo: 42 }, timestamp: fake_time) { sleep(0.1) } + end + end end From 116318bfe2b2145f96f4c6a66e541cc0fd8d1edf Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 5 Mar 2024 16:15:54 +0100 Subject: [PATCH 19/20] Metric summaries on span * add a `LocalAggregator` instance on spans to duplicate metrics on the span as a gauge metric * proxy the main aggregator add calls to the local aggregator if a span is running * start a `metrics.timing` span in the `Sentry::Metrics.timing` API --- CHANGELOG.md | 2 + sentry-ruby/lib/sentry/metrics.rb | 15 ++-- sentry-ruby/lib/sentry/metrics/aggregator.rb | 24 +++++- .../lib/sentry/metrics/local_aggregator.rb | 53 ++++++++++++ sentry-ruby/lib/sentry/span.rb | 17 +++- sentry-ruby/lib/sentry/transaction_event.rb | 5 ++ sentry-ruby/spec/sentry/client_spec.rb | 10 +++ .../spec/sentry/metrics/aggregator_spec.rb | 51 ++++++++++++ .../sentry/metrics/local_aggregator_spec.rb | 83 +++++++++++++++++++ sentry-ruby/spec/sentry/metrics_spec.rb | 25 ++++++ sentry-ruby/spec/sentry/span_spec.rb | 10 +++ 11 files changed, 285 insertions(+), 10 deletions(-) create mode 100644 sentry-ruby/lib/sentry/metrics/local_aggregator.rb create mode 100644 sentry-ruby/spec/sentry/metrics/local_aggregator_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 480ab8720..5ca04651d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,7 @@ - Add [Metrics](https://docs.sentry.io/product/metrics/) support - Add main APIs and `Aggregator` thread [#2247](https://github.com/getsentry/sentry-ruby/pull/2247) - Add `Sentry::Metrics.timing` API for measuring block duration [#2254](https://github.com/getsentry/sentry-ruby/pull/2254) + - Add metric summaries on spans [#2255](https://github.com/getsentry/sentry-ruby/pull/2255) 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. @@ -39,6 +40,7 @@ Sentry::Metrics.set('user_view', 'jane') # timing - measure duration of code block, defaults to seconds + # will also automatically create a `metric.timing` span Sentry::Metrics.timing('how_long') { sleep(1) } # timing - measure duration of code block in other duraton units Sentry::Metrics.timing('how_long_ms', unit: 'millisecond') { sleep(0.5) } diff --git a/sentry-ruby/lib/sentry/metrics.rb b/sentry-ruby/lib/sentry/metrics.rb index c8d043c51..83048420e 100644 --- a/sentry-ruby/lib/sentry/metrics.rb +++ b/sentry-ruby/lib/sentry/metrics.rb @@ -14,6 +14,8 @@ module Metrics INFORMATION_UNITS = %w[bit byte kilobyte kibibyte megabyte mebibyte gigabyte gibibyte terabyte tebibyte petabyte pebibyte exabyte exbibyte] FRACTIONAL_UNITS = %w[ratio percent] + OP_NAME = 'metric.timing' + 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) @@ -32,15 +34,18 @@ def gauge(key, value, unit: 'none', tags: {}, timestamp: nil) end def timing(key, unit: 'second', tags: {}, timestamp: nil, &block) - return unless Sentry.metrics_aggregator return unless block_given? return unless DURATION_UNITS.include?(unit) - start = Timing.send(unit.to_sym) - yield - value = Timing.send(unit.to_sym) - start + Sentry.with_child_span(op: OP_NAME, description: key) do |span| + tags.each { |k, v| span.set_tag(k, v.is_a?(Array) ? v.join(', ') : v.to_s) } if span + + start = Timing.send(unit.to_sym) + yield + value = Timing.send(unit.to_sym) - start - Sentry.metrics_aggregator.add(:d, key, value, unit: unit, tags: tags, timestamp: timestamp) + Sentry.metrics_aggregator&.add(:d, 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 index f7d6b17b1..35ff66cab 100644 --- a/sentry-ruby/lib/sentry/metrics/aggregator.rb +++ b/sentry-ruby/lib/sentry/metrics/aggregator.rb @@ -59,15 +59,23 @@ def add(type, serialized_tags = serialize_tags(get_updated_tags(tags)) bucket_key = [type, key, unit, serialized_tags] - @mutex.synchronize do + added = @mutex.synchronize do @buckets[bucket_timestamp] ||= {} - if @buckets[bucket_timestamp][bucket_key] - @buckets[bucket_timestamp][bucket_key].add(value) + if (metric = @buckets[bucket_timestamp][bucket_key]) + old_weight = metric.weight + metric.add(value) + metric.weight - old_weight else - @buckets[bucket_timestamp][bucket_key] = METRIC_TYPES[type].new(value) + metric = METRIC_TYPES[type].new(value) + @buckets[bucket_timestamp][bucket_key] = metric + metric.weight end end + + # for sets, we pass on if there was a new entry to the local gauge + local_value = type == :s ? added : value + process_span_aggregator(bucket_key, local_value) end def flush(force: false) @@ -179,6 +187,14 @@ def get_updated_tags(tags) updated_tags end + + def process_span_aggregator(key, value) + scope = Sentry.get_current_scope + return nil unless scope && scope.span + return nil if scope.transaction_source_low_quality? + + scope.span.metrics_local_aggregator.add(key, value) + end end end end diff --git a/sentry-ruby/lib/sentry/metrics/local_aggregator.rb b/sentry-ruby/lib/sentry/metrics/local_aggregator.rb new file mode 100644 index 000000000..99d50a5b1 --- /dev/null +++ b/sentry-ruby/lib/sentry/metrics/local_aggregator.rb @@ -0,0 +1,53 @@ +# frozen_string_literal: true + +module Sentry + module Metrics + class LocalAggregator + # exposed only for testing + attr_reader :buckets + + def initialize + @buckets = {} + end + + def add(key, value) + if @buckets[key] + @buckets[key].add(value) + else + @buckets[key] = GaugeMetric.new(value) + end + end + + def to_hash + return nil if @buckets.empty? + + @buckets.map do |bucket_key, metric| + type, key, unit, tags = bucket_key + + payload_key = "#{type}:#{key}@#{unit}" + payload_value = { + tags: deserialize_tags(tags), + min: metric.min, + max: metric.max, + count: metric.count, + sum: metric.sum + } + + [payload_key, payload_value] + end.to_h + end + + private + + def deserialize_tags(tags) + tags.inject({}) do |h, tag| + k, v = tag + old = h[k] + # make it an array if key repeats + h[k] = old ? (old.is_a?(Array) ? old << v : [old, v]) : v + h + end + end + end + end +end diff --git a/sentry-ruby/lib/sentry/span.rb b/sentry-ruby/lib/sentry/span.rb index 3cddb49a6..69374b496 100644 --- a/sentry-ruby/lib/sentry/span.rb +++ b/sentry-ruby/lib/sentry/span.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require "securerandom" +require "sentry/metrics/local_aggregator" module Sentry class Span @@ -149,7 +150,7 @@ def to_baggage # @return [Hash] def to_hash - { + hash = { trace_id: @trace_id, span_id: @span_id, parent_span_id: @parent_span_id, @@ -161,6 +162,11 @@ def to_hash tags: @tags, data: @data } + + summary = metrics_summary + hash[:_metrics_summary] = summary if summary + + hash end # Returns the span's context that can be used to embed in an Event. @@ -268,5 +274,14 @@ def set_data(key, value) def set_tag(key, value) @tags[key] = value end + + # Collects gauge metrics on the span for metric summaries. + def metrics_local_aggregator + @metrics_local_aggregator ||= Sentry::Metrics::LocalAggregator.new + end + + def metrics_summary + @metrics_local_aggregator&.to_hash + end end end diff --git a/sentry-ruby/lib/sentry/transaction_event.rb b/sentry-ruby/lib/sentry/transaction_event.rb index 20d25445e..d2e39dc1f 100644 --- a/sentry-ruby/lib/sentry/transaction_event.rb +++ b/sentry-ruby/lib/sentry/transaction_event.rb @@ -17,6 +17,9 @@ class TransactionEvent < Event # @return [Hash, nil] attr_accessor :profile + # @return [Hash, nil] + attr_accessor :metrics_summary + def initialize(transaction:, **options) super(**options) @@ -29,6 +32,7 @@ def initialize(transaction:, **options) self.tags = transaction.tags self.dynamic_sampling_context = transaction.get_baggage.dynamic_sampling_context self.measurements = transaction.measurements + self.metrics_summary = transaction.metrics_summary finished_spans = transaction.span_recorder.spans.select { |span| span.timestamp && span != transaction } self.spans = finished_spans.map(&:to_hash) @@ -49,6 +53,7 @@ def to_hash data[:spans] = @spans.map(&:to_hash) if @spans data[:start_timestamp] = @start_timestamp data[:measurements] = @measurements + data[:_metrics_summary] = @metrics_summary if @metrics_summary data end diff --git a/sentry-ruby/spec/sentry/client_spec.rb b/sentry-ruby/spec/sentry/client_spec.rb index 33c9ff4a5..5f95cf831 100644 --- a/sentry-ruby/spec/sentry/client_spec.rb +++ b/sentry-ruby/spec/sentry/client_spec.rb @@ -195,6 +195,16 @@ def sentry_context event = subject.event_from_transaction(transaction) expect(event.contexts).to include({ foo: { bar: 42 } }) end + + it 'adds metric summary on transaction if any' do + key = [:c, 'incr', 'none', []] + transaction.metrics_local_aggregator.add(key, 10) + hash = subject.event_from_transaction(transaction).to_hash + + expect(hash[:_metrics_summary]).to eq({ + 'c:incr@none' => { count: 1, max: 10.0, min: 10.0, sum: 10.0, tags: {} } + }) + end end describe "#event_from_exception" do diff --git a/sentry-ruby/spec/sentry/metrics/aggregator_spec.rb b/sentry-ruby/spec/sentry/metrics/aggregator_spec.rb index d95ffbde8..785f11cb9 100644 --- a/sentry-ruby/spec/sentry/metrics/aggregator_spec.rb +++ b/sentry-ruby/spec/sentry/metrics/aggregator_spec.rb @@ -187,6 +187,57 @@ expect(metric).to be_a(Sentry::Metrics::SetMetric) expect(metric.value).to eq(Set[1]) end + + describe 'local aggregation for span metric summaries' do + it 'does nothing without an active scope span' do + expect_any_instance_of(Sentry::Metrics::LocalAggregator).not_to receive(:add) + subject.add(:c, 'incr', 1) + end + + context 'with running transaction and active span' do + let(:span) { Sentry.start_transaction } + + before do + Sentry.get_current_scope.set_span(span) + Sentry.get_current_scope.set_transaction_name('metric', source: :view) + end + + it 'does nothing if transaction name is low quality' do + expect_any_instance_of(Sentry::Metrics::LocalAggregator).not_to receive(:add) + + Sentry.get_current_scope.set_transaction_name('/123', source: :url) + subject.add(:c, 'incr', 1) + end + + it 'proxies bucket key and value to local aggregator' do + expect(span.metrics_local_aggregator).to receive(:add).with( + array_including(:c, 'incr', 'none'), + 1 + ) + subject.add(:c, 'incr', 1) + end + + context 'for set metrics' do + before { subject.add(:s, 'set', 'foo') } + + it 'proxies bucket key and value 0 when existing element' do + expect(span.metrics_local_aggregator).to receive(:add).with( + array_including(:s, 'set', 'none'), + 0 + ) + subject.add(:s, 'set', 'foo') + end + + it 'proxies bucket key and value 1 when new element' do + expect(span.metrics_local_aggregator).to receive(:add).with( + array_including(:s, 'set', 'none'), + 1 + ) + subject.add(:s, 'set', 'bar') + end + end + end + end end describe '#flush' do diff --git a/sentry-ruby/spec/sentry/metrics/local_aggregator_spec.rb b/sentry-ruby/spec/sentry/metrics/local_aggregator_spec.rb new file mode 100644 index 000000000..5c5ab0583 --- /dev/null +++ b/sentry-ruby/spec/sentry/metrics/local_aggregator_spec.rb @@ -0,0 +1,83 @@ +require 'spec_helper' + +RSpec.describe Sentry::Metrics::LocalAggregator do + let(:tags) { [['foo', 1], ['foo', 2], ['bar', 'baz']] } + let(:key) { [:c, 'incr', 'second', tags] } + let(:key2) { [:s, 'set', 'none', []] } + + describe '#add' do + it 'creates new GaugeMetric and adds it to bucket if key not existing' do + expect(Sentry::Metrics::GaugeMetric).to receive(:new).with(10).and_call_original + + subject.add(key, 10) + + metric = subject.buckets[key] + expect(metric).to be_a(Sentry::Metrics::GaugeMetric) + expect(metric.last).to eq(10.0) + expect(metric.min).to eq(10.0) + expect(metric.max).to eq(10.0) + expect(metric.sum).to eq(10.0) + expect(metric.count).to eq(1) + end + + it 'adds value to existing GaugeMetric' do + subject.add(key, 10) + + metric = subject.buckets[key] + expect(metric).to be_a(Sentry::Metrics::GaugeMetric) + expect(metric).to receive(:add).with(20).and_call_original + expect(Sentry::Metrics::GaugeMetric).not_to receive(:new) + + subject.add(key, 20) + expect(metric.last).to eq(20.0) + expect(metric.min).to eq(10.0) + expect(metric.max).to eq(20.0) + expect(metric.sum).to eq(30.0) + expect(metric.count).to eq(2) + end + end + + describe '#to_hash' do + it 'returns nil if empty buckets' do + expect(subject.to_hash).to eq(nil) + end + + context 'with filled buckets' do + before do + subject.add(key, 10) + subject.add(key, 20) + subject.add(key2, 1) + end + + it 'has the correct payload keys in the hash' do + expect(subject.to_hash.keys).to eq([ + 'c:incr@second', + 's:set@none' + ]) + end + + it 'has the tags deserialized correctly with array values' do + expect(subject.to_hash['c:incr@second'][:tags]).to eq({ + 'foo' => [1, 2], + 'bar' => 'baz' + }) + end + + it 'has the correct gauge metric values' do + expect(subject.to_hash['c:incr@second']).to include({ + min: 10.0, + max: 20.0, + count: 2, + sum: 30.0 + }) + + expect(subject.to_hash['s:set@none']).to include({ + min: 1.0, + max: 1.0, + count: 1, + sum: 1.0 + }) + end + end + end +end diff --git a/sentry-ruby/spec/sentry/metrics_spec.rb b/sentry-ruby/spec/sentry/metrics_spec.rb index 9989876a4..094aee347 100644 --- a/sentry-ruby/spec/sentry/metrics_spec.rb +++ b/sentry-ruby/spec/sentry/metrics_spec.rb @@ -106,5 +106,30 @@ described_class.timing('foo', unit: 'millisecond', tags: { fortytwo: 42 }, timestamp: fake_time) { sleep(0.1) } end + + context 'with running transaction' do + let(:transaction) { transaction = Sentry.start_transaction(name: 'metrics') } + + before do + perform_basic_setup do |config| + config.enable_tracing = true + config.metrics.enabled = true + end + + Sentry.get_current_scope.set_span(transaction) + end + + it 'starts a span' do + expect(Sentry).to receive(:with_child_span).with(op: Sentry::Metrics::OP_NAME, description: 'foo').and_call_original + + described_class.timing('foo') { sleep(0.1) } + end + + it 'has the correct tags on the new span' do + described_class.timing('foo', tags: { a: 42, b: [1, 2] }) { sleep(0.1) } + span = transaction.span_recorder.spans.last + expect(span.tags).to eq(a: '42', b: '1, 2') + end + end end end diff --git a/sentry-ruby/spec/sentry/span_spec.rb b/sentry-ruby/spec/sentry/span_spec.rb index 96d1f378b..3e61c8058 100644 --- a/sentry-ruby/spec/sentry/span_spec.rb +++ b/sentry-ruby/spec/sentry/span_spec.rb @@ -70,6 +70,16 @@ expect(hash[:trace_id].length).to eq(32) expect(hash[:span_id].length).to eq(16) end + + it 'has metric summary if present' do + key = [:c, 'incr', 'none', []] + subject.metrics_local_aggregator.add(key, 10) + + hash = subject.to_hash + expect(hash[:_metrics_summary]).to eq({ + 'c:incr@none' => { count: 1, max: 10.0, min: 10.0, sum: 10.0, tags: {} } + }) + end end describe "#to_sentry_trace" do From 9a7abafc7bd771413f05783ec0862edc3ece102d Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 12 Mar 2024 14:37:27 +0100 Subject: [PATCH 20/20] Add config.metrics.before_emit callback (#2258) --- CHANGELOG.md | 13 ++++++++ sentry-ruby/lib/sentry/configuration.rb | 6 ---- sentry-ruby/lib/sentry/metrics/aggregator.rb | 6 +++- .../lib/sentry/metrics/configuration.rb | 23 +++++++++++++ .../sentry/utils/argument_checking_helper.rb | 6 ++++ .../spec/sentry/metrics/aggregator_spec.rb | 32 +++++++++++++++++++ .../spec/sentry/metrics/configuration_spec.rb | 11 +++++++ 7 files changed, 90 insertions(+), 7 deletions(-) create mode 100644 sentry-ruby/spec/sentry/metrics/configuration_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index 5ca04651d..a5ffc0b6f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -9,6 +9,7 @@ - Add main APIs and `Aggregator` thread [#2247](https://github.com/getsentry/sentry-ruby/pull/2247) - Add `Sentry::Metrics.timing` API for measuring block duration [#2254](https://github.com/getsentry/sentry-ruby/pull/2254) - Add metric summaries on spans [#2255](https://github.com/getsentry/sentry-ruby/pull/2255) + - Add `config.metrics.before_emit` callback [#2258](https://github.com/getsentry/sentry-ruby/pull/2258) 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. @@ -44,6 +45,18 @@ Sentry::Metrics.timing('how_long') { sleep(1) } # timing - measure duration of code block in other duraton units Sentry::Metrics.timing('how_long_ms', unit: 'millisecond') { sleep(0.5) } + + # add a before_emit callback to filter keys or update tags + Sentry.init do |config| + # ... + config.metrics.enabled = true + config.metrics.before_emit = lambda do |key, tags| + return nil if key == 'foo' + tags[:bar] = 42 + tags.delete(:baz) + true + end + end ``` ### Bug Fixes diff --git a/sentry-ruby/lib/sentry/configuration.rb b/sentry-ruby/lib/sentry/configuration.rb index c2776459f..ba39bd8e6 100644 --- a/sentry-ruby/lib/sentry/configuration.rb +++ b/sentry-ruby/lib/sentry/configuration.rb @@ -576,12 +576,6 @@ def error_messages private - def check_callable!(name, value) - unless value == nil || value.respond_to?(:call) - raise ArgumentError, "#{name} must be callable (or nil to disable)" - end - end - def init_dsn(dsn_string) return if dsn_string.nil? || dsn_string.empty? diff --git a/sentry-ruby/lib/sentry/metrics/aggregator.rb b/sentry-ruby/lib/sentry/metrics/aggregator.rb index 35ff66cab..18b1af8f7 100644 --- a/sentry-ruby/lib/sentry/metrics/aggregator.rb +++ b/sentry-ruby/lib/sentry/metrics/aggregator.rb @@ -24,6 +24,7 @@ class Aggregator def initialize(configuration, client) @client = client @logger = configuration.logger + @before_emit = configuration.metrics.before_emit @default_tags = {} @default_tags['release'] = configuration.release if configuration.release @@ -55,8 +56,11 @@ def add(type, # 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 + updated_tags = get_updated_tags(tags) - serialized_tags = serialize_tags(get_updated_tags(tags)) + return if @before_emit && !@before_emit.call(key, updated_tags) + + serialized_tags = serialize_tags(updated_tags) bucket_key = [type, key, unit, serialized_tags] added = @mutex.synchronize do diff --git a/sentry-ruby/lib/sentry/metrics/configuration.rb b/sentry-ruby/lib/sentry/metrics/configuration.rb index 1c8b86be2..e45b43bd5 100644 --- a/sentry-ruby/lib/sentry/metrics/configuration.rb +++ b/sentry-ruby/lib/sentry/metrics/configuration.rb @@ -3,15 +3,38 @@ module Sentry module Metrics class Configuration + include ArgumentCheckingHelper + # 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 + # Optional Proc, called before emitting a metric to the aggregator. + # Use it to filter keys (return false/nil) or update tags. + # Make sure to return true at the end. + # + # @example + # config.metrics.before_emit = lambda do |key, tags| + # return nil if key == 'foo' + # tags[:bar] = 42 + # tags.delete(:baz) + # true + # end + # + # @return [Proc, nil] + attr_reader :before_emit + def initialize @enabled = false end + + def before_emit=(value) + check_callable!("metrics.before_emit", value) + + @before_emit = value + end end end end diff --git a/sentry-ruby/lib/sentry/utils/argument_checking_helper.rb b/sentry-ruby/lib/sentry/utils/argument_checking_helper.rb index e00eb5fc2..343e01dad 100644 --- a/sentry-ruby/lib/sentry/utils/argument_checking_helper.rb +++ b/sentry-ruby/lib/sentry/utils/argument_checking_helper.rb @@ -15,5 +15,11 @@ def check_argument_includes!(argument, values) raise ArgumentError, "expect the argument to be one of #{values.map(&:inspect).join(' or ')}, got #{argument.inspect}" end end + + def check_callable!(name, value) + unless value == nil || value.respond_to?(:call) + raise ArgumentError, "#{name} must be callable (or nil to disable)" + end + end end end diff --git a/sentry-ruby/spec/sentry/metrics/aggregator_spec.rb b/sentry-ruby/spec/sentry/metrics/aggregator_spec.rb index 785f11cb9..1edbfe215 100644 --- a/sentry-ruby/spec/sentry/metrics/aggregator_spec.rb +++ b/sentry-ruby/spec/sentry/metrics/aggregator_spec.rb @@ -188,6 +188,38 @@ expect(metric.value).to eq(Set[1]) end + describe 'with before_emit callback' do + 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) + + config.metrics.before_emit = lambda do |key, tags| + return nil if key == 'foo' + tags[:add_tag] = 42 + tags.delete(:remove_tag) + true + end + end + end + + it 'does not emit metric with filtered key' do + expect(Sentry::Metrics::CounterMetric).not_to receive(:new) + subject.add(:c, 'foo', 1) + expect(subject.buckets).to eq({}) + end + + it 'updates the tags according to the callback' do + subject.add(:c, 'bar', 1, tags: { remove_tag: 99 }) + _, _, _, tags = subject.buckets.values.first.keys.first + expect(tags).not_to include(['remove_tag', '99']) + expect(tags).to include(['add_tag', '42']) + end + end + describe 'local aggregation for span metric summaries' do it 'does nothing without an active scope span' do expect_any_instance_of(Sentry::Metrics::LocalAggregator).not_to receive(:add) diff --git a/sentry-ruby/spec/sentry/metrics/configuration_spec.rb b/sentry-ruby/spec/sentry/metrics/configuration_spec.rb new file mode 100644 index 000000000..2c7e22fbe --- /dev/null +++ b/sentry-ruby/spec/sentry/metrics/configuration_spec.rb @@ -0,0 +1,11 @@ +require 'spec_helper' + +RSpec.describe Sentry::Metrics::Configuration do + describe '#before_emit=' do + it 'raises error when setting before_emit to anything other than callable or nil' do + subject.before_emit = -> { } + subject.before_emit = nil + expect { subject.before_emit = true }.to raise_error(ArgumentError, 'metrics.before_emit must be callable (or nil to disable)') + end + end +end