From 5c96b67593326e8aebc1cb66b8ae9eec457820cb Mon Sep 17 00:00:00 2001 From: Neel Shah Date: Tue, 4 Feb 2025 14:34:30 +0100 Subject: [PATCH] Deprecate config.metrics and Sentry::Metrics --- CHANGELOG.md | 22 + sentry-ruby/lib/sentry-ruby.rb | 11 +- sentry-ruby/lib/sentry/configuration.rb | 3 +- .../lib/sentry/cron/monitor_check_ins.rb | 6 +- sentry-ruby/lib/sentry/envelope/item.rb | 1 - .../sentry/interfaces/stacktrace_builder.rb | 8 - sentry-ruby/lib/sentry/metrics.rb | 49 +- sentry-ruby/lib/sentry/metrics/aggregator.rb | 248 --------- .../lib/sentry/metrics/configuration.rb | 41 +- .../lib/sentry/metrics/counter_metric.rb | 25 - .../lib/sentry/metrics/distribution_metric.rb | 25 - .../lib/sentry/metrics/gauge_metric.rb | 35 -- .../lib/sentry/metrics/local_aggregator.rb | 53 -- sentry-ruby/lib/sentry/metrics/metric.rb | 19 - sentry-ruby/lib/sentry/metrics/set_metric.rb | 28 - sentry-ruby/lib/sentry/metrics/timing.rb | 51 -- sentry-ruby/lib/sentry/span.rb | 13 - sentry-ruby/lib/sentry/transaction_event.rb | 5 +- sentry-ruby/spec/sentry/client_spec.rb | 12 - sentry-ruby/spec/sentry/envelope/item_spec.rb | 2 - .../interfaces/stacktrace_builder_spec.rb | 13 - .../spec/sentry/metrics/aggregator_spec.rb | 487 ------------------ .../spec/sentry/metrics/configuration_spec.rb | 17 +- .../sentry/metrics/counter_metric_spec.rb | 27 - .../metrics/distribution_metric_spec.rb | 27 - .../spec/sentry/metrics/gauge_metric_spec.rb | 31 -- .../sentry/metrics/local_aggregator_spec.rb | 85 --- .../spec/sentry/metrics/metric_spec.rb | 23 - .../spec/sentry/metrics/set_metric_spec.rb | 32 -- .../spec/sentry/metrics/timing_spec.rb | 56 -- sentry-ruby/spec/sentry/metrics_spec.rb | 140 +---- sentry-ruby/spec/sentry/span_spec.rb | 10 - sentry-ruby/spec/sentry/transport_spec.rb | 24 - sentry-ruby/spec/sentry_spec.rb | 8 - 34 files changed, 73 insertions(+), 1564 deletions(-) delete mode 100644 sentry-ruby/lib/sentry/metrics/aggregator.rb delete mode 100644 sentry-ruby/lib/sentry/metrics/counter_metric.rb delete mode 100644 sentry-ruby/lib/sentry/metrics/distribution_metric.rb delete mode 100644 sentry-ruby/lib/sentry/metrics/gauge_metric.rb delete mode 100644 sentry-ruby/lib/sentry/metrics/local_aggregator.rb delete mode 100644 sentry-ruby/lib/sentry/metrics/metric.rb delete mode 100644 sentry-ruby/lib/sentry/metrics/set_metric.rb delete mode 100644 sentry-ruby/lib/sentry/metrics/timing.rb delete mode 100644 sentry-ruby/spec/sentry/metrics/aggregator_spec.rb delete mode 100644 sentry-ruby/spec/sentry/metrics/counter_metric_spec.rb delete mode 100644 sentry-ruby/spec/sentry/metrics/distribution_metric_spec.rb delete mode 100644 sentry-ruby/spec/sentry/metrics/gauge_metric_spec.rb delete mode 100644 sentry-ruby/spec/sentry/metrics/local_aggregator_spec.rb delete mode 100644 sentry-ruby/spec/sentry/metrics/metric_spec.rb delete mode 100644 sentry-ruby/spec/sentry/metrics/set_metric_spec.rb delete mode 100644 sentry-ruby/spec/sentry/metrics/timing_spec.rb diff --git a/CHANGELOG.md b/CHANGELOG.md index c80a5ba70..603c96f2d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,28 @@ - Revert "[rails] support string errors in error reporter (#2464)" ([#2533](https://github.com/getsentry/sentry-ruby/pull/2533)) - Removed unnecessary warning about missing `stackprof` when Vernier is configured as the profiler ([#2537](https://github.com/getsentry/sentry-ruby/pull/2537)) +### Miscellaneous + +- Deprecate all Metrics related APIs [#2539](https://github.com/getsentry/sentry-ruby/pull/2539) + + Sentry [no longer has the Metrics Beta offering](https://sentry.zendesk.com/hc/en-us/articles/26369339769883-Metrics-Beta-Ended-on-October-7th) so + all the following APIs linked to Metrics have been deprecated and will be removed in the next major. + + ```ruby + Sentry.init do |config| + # ... + config.metrics.enabled = true + config.metrics.enable_code_locations = true + config.metrics.before_emit = lambda {} + end + + Sentry::Metrics.increment('button_click') + Sentry::Metrics.distribution('page_load', 15.0, unit: 'millisecond') + Sentry::Metrics.gauge('page_load', 15.0, unit: 'millisecond') + Sentry::Metrics.set('user_view', 'jane') + Sentry::Metrics.timing('how_long') { sleep(1) } + ``` + ## 5.22.3 ### Bug Fixes diff --git a/sentry-ruby/lib/sentry-ruby.rb b/sentry-ruby/lib/sentry-ruby.rb index fb7116c48..4360cf0a2 100644 --- a/sentry-ruby/lib/sentry-ruby.rb +++ b/sentry-ruby/lib/sentry-ruby.rb @@ -83,7 +83,8 @@ def exception_locals_tp attr_reader :backpressure_monitor # @!attribute [r] metrics_aggregator - # @return [Metrics::Aggregator, nil] + # @deprecated + # @return [nil] attr_reader :metrics_aggregator ##### Patch Registration ##### @@ -241,7 +242,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 + @metrics_aggregator = nil exception_locals_tp.enable if config.include_local_variables at_exit { close } end @@ -262,12 +263,6 @@ 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.flush diff --git a/sentry-ruby/lib/sentry/configuration.rb b/sentry-ruby/lib/sentry/configuration.rb index 1b67975f7..8c7743093 100644 --- a/sentry-ruby/lib/sentry/configuration.rb +++ b/sentry-ruby/lib/sentry/configuration.rb @@ -245,6 +245,7 @@ def capture_exception_frame_locals=(value) attr_reader :cron # Metrics related configuration. + # @deprecated It will be removed in the next major release. # @return [Metrics::Configuration] attr_reader :metrics @@ -450,7 +451,7 @@ def initialize @transport = Transport::Configuration.new @cron = Cron::Configuration.new - @metrics = Metrics::Configuration.new + @metrics = Metrics::Configuration.new(self.logger) @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/cron/monitor_check_ins.rb b/sentry-ruby/lib/sentry/cron/monitor_check_ins.rb index df7445286..314733cfb 100644 --- a/sentry-ruby/lib/sentry/cron/monitor_check_ins.rb +++ b/sentry-ruby/lib/sentry/cron/monitor_check_ins.rb @@ -14,12 +14,12 @@ def perform(*args, **opts) :in_progress, monitor_config: monitor_config) - start = Metrics::Timing.duration_start + start = Process.clock_gettime(Process::CLOCK_MONOTONIC) begin # need to do this on ruby <= 2.6 sadly ret = method(:perform).super_method.arity == 0 ? super() : super - duration = Metrics::Timing.duration_end(start) + duration = Process.clock_gettime(Process::CLOCK_MONOTONIC) - start Sentry.capture_check_in(slug, :ok, @@ -29,7 +29,7 @@ def perform(*args, **opts) ret rescue Exception - duration = Metrics::Timing.duration_end(start) + duration = Process.clock_gettime(Process::CLOCK_MONOTONIC) - start Sentry.capture_check_in(slug, :error, diff --git a/sentry-ruby/lib/sentry/envelope/item.rb b/sentry-ruby/lib/sentry/envelope/item.rb index e1539bf6c..38ee15117 100644 --- a/sentry-ruby/lib/sentry/envelope/item.rb +++ b/sentry-ruby/lib/sentry/envelope/item.rb @@ -18,7 +18,6 @@ def self.data_category(type) when "session", "attachment", "transaction", "profile", "span" then type when "sessions" then "session" when "check_in" then "monitor" - when "statsd", "metric_meta" then "metric_bucket" when "event" then "error" when "client_report" then "internal" else "default" diff --git a/sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb b/sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb index d2d0758ec..3f4a7f090 100644 --- a/sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb +++ b/sentry-ruby/lib/sentry/interfaces/stacktrace_builder.rb @@ -75,14 +75,6 @@ def build(backtrace:, &frame_callback) StacktraceInterface.new(frames: frames) end - # Get the code location hash for a single line for where metrics where added. - # @return [Hash] - def metrics_code_location(unparsed_line) - parsed_line = Backtrace::Line.parse(unparsed_line) - frame = convert_parsed_line_into_frame(parsed_line) - frame.to_hash.reject { |k, _| %i[project_root in_app].include?(k) } - end - private def convert_parsed_line_into_frame(line) diff --git a/sentry-ruby/lib/sentry/metrics.rb b/sentry-ruby/lib/sentry/metrics.rb index bcab324f4..56a5363d0 100644 --- a/sentry-ruby/lib/sentry/metrics.rb +++ b/sentry-ruby/lib/sentry/metrics.rb @@ -1,55 +1,14 @@ # 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/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] - - OP_NAME = "metric.timing" - SPAN_ORIGIN = "auto.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) - 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 - - def timing(key, unit: "second", tags: {}, timestamp: nil, &block) - return unless block_given? - return yield unless DURATION_UNITS.include?(unit) + def method_missing(m, *args, &block) + return unless Sentry.initialized? - result, value = Sentry.with_child_span(op: OP_NAME, description: key, origin: SPAN_ORIGIN) 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) - result = yield - value = Timing.send(unit.to_sym) - start - - [result, value] + Sentry.logger.warn(LOGGER_PROGNAME) do + "`Sentry::Metrics` is now deprecated and will be removed in the next major." end - - Sentry.metrics_aggregator&.add(:d, key, value, unit: unit, tags: tags, timestamp: timestamp) - result end end end diff --git a/sentry-ruby/lib/sentry/metrics/aggregator.rb b/sentry-ruby/lib/sentry/metrics/aggregator.rb deleted file mode 100644 index 12ad54f69..000000000 --- a/sentry-ruby/lib/sentry/metrics/aggregator.rb +++ /dev/null @@ -1,248 +0,0 @@ -# frozen_string_literal: true - -module Sentry - module Metrics - class Aggregator < ThreadedPeriodicWorker - FLUSH_INTERVAL = 5 - ROLLUP_IN_SECONDS = 10 - - # this is how far removed from user code in the backtrace we are - # when we record code locations - DEFAULT_STACKLEVEL = 4 - - KEY_SANITIZATION_REGEX = /[^a-zA-Z0-9_\-.]+/ - UNIT_SANITIZATION_REGEX = /[^a-zA-Z0-9_]+/ - TAG_KEY_SANITIZATION_REGEX = /[^a-zA-Z0-9_\-.\/]+/ - - TAG_VALUE_SANITIZATION_MAP = { - "\n" => "\\n", - "\r" => "\\r", - "\t" => "\\t", - "\\" => "\\\\", - "|" => "\\u{7c}", - "," => "\\u{2c}" - } - - METRIC_TYPES = { - c: CounterMetric, - d: DistributionMetric, - g: GaugeMetric, - s: SetMetric - } - - # exposed only for testing - attr_reader :client, :thread, :buckets, :flush_shift, :code_locations - - def initialize(configuration, client) - super(configuration.logger, FLUSH_INTERVAL) - @client = client - @before_emit = configuration.metrics.before_emit - @enable_code_locations = configuration.metrics.enable_code_locations - @stacktrace_builder = configuration.stacktrace_builder - - @default_tags = {} - @default_tags["release"] = configuration.release if configuration.release - @default_tags["environment"] = configuration.environment if configuration.environment - - @mutex = Mutex.new - - # 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 - - # a nested hash of timestamp (start of day) -> meta keys -> frame - @code_locations = {} - end - - def add(type, - key, - value, - unit: "none", - tags: {}, - timestamp: nil, - stacklevel: nil) - return unless ensure_thread - return unless METRIC_TYPES.keys.include?(type) - - updated_tags = get_updated_tags(tags) - return if @before_emit && !@before_emit.call(key, updated_tags) - - timestamp ||= Sentry.utc_now - - # this is integer division and thus takes the floor of the division - # and buckets into 10 second intervals - bucket_timestamp = (timestamp.to_i / ROLLUP_IN_SECONDS) * ROLLUP_IN_SECONDS - - serialized_tags = serialize_tags(updated_tags) - bucket_key = [type, key, unit, serialized_tags] - - added = @mutex.synchronize do - record_code_location(type, key, unit, timestamp, stacklevel: stacklevel) if @enable_code_locations - process_bucket(bucket_timestamp, bucket_key, type, value) - 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) - flushable_buckets = get_flushable_buckets!(force) - code_locations = get_code_locations! - return if flushable_buckets.empty? && code_locations.empty? - - envelope = Envelope.new - - unless flushable_buckets.empty? - payload = serialize_buckets(flushable_buckets) - envelope.add_item( - { type: "statsd", length: payload.bytesize }, - payload - ) - end - - unless code_locations.empty? - code_locations.each do |timestamp, locations| - payload = serialize_locations(timestamp, locations) - envelope.add_item( - { type: "metric_meta", content_type: "application/json" }, - payload - ) - end - end - - @client.capture_envelope(envelope) - end - - alias_method :run, :flush - - private - - # 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 - - def get_code_locations! - @mutex.synchronize do - code_locations = @code_locations - @code_locations = {} - code_locations - 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_tag_key(k)}:#{sanitize_tag_value(v)}" }.join(",") - - "#{sanitize_key(key)}@#{sanitize_unit(unit)}:#{values}|#{type}|\##{sanitized_tags}|T#{timestamp}" - end - end.flatten.join("\n") - end - - def serialize_locations(timestamp, locations) - mapping = locations.map do |meta_key, location| - type, key, unit = meta_key - mri = "#{type}:#{sanitize_key(key)}@#{sanitize_unit(unit)}" - - # note this needs to be an array but it really doesn't serve a purpose right now - [mri, [location.merge(type: "location")]] - end.to_h - - { timestamp: timestamp, mapping: mapping } - end - - def sanitize_key(key) - key.gsub(KEY_SANITIZATION_REGEX, "_") - end - - def sanitize_unit(unit) - unit.gsub(UNIT_SANITIZATION_REGEX, "") - end - - def sanitize_tag_key(key) - key.gsub(TAG_KEY_SANITIZATION_REGEX, "") - end - - def sanitize_tag_value(value) - value.chars.map { |c| TAG_VALUE_SANITIZATION_MAP[c] || c }.join - 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 - - 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 - - def process_bucket(timestamp, key, type, value) - @buckets[timestamp] ||= {} - - if (metric = @buckets[timestamp][key]) - old_weight = metric.weight - metric.add(value) - metric.weight - old_weight - else - metric = METRIC_TYPES[type].new(value) - @buckets[timestamp][key] = metric - metric.weight - end - end - - def record_code_location(type, key, unit, timestamp, stacklevel: nil) - meta_key = [type, key, unit] - start_of_day = Time.utc(timestamp.year, timestamp.month, timestamp.day).to_i - - @code_locations[start_of_day] ||= {} - @code_locations[start_of_day][meta_key] ||= @stacktrace_builder.metrics_code_location(caller[stacklevel || DEFAULT_STACKLEVEL]) - end - end - end -end diff --git a/sentry-ruby/lib/sentry/metrics/configuration.rb b/sentry-ruby/lib/sentry/metrics/configuration.rb index 84681f31e..5dc6a5518 100644 --- a/sentry-ruby/lib/sentry/metrics/configuration.rb +++ b/sentry-ruby/lib/sentry/metrics/configuration.rb @@ -4,43 +4,16 @@ module Sentry module Metrics class Configuration include ArgumentCheckingHelper + include LoggingHelper - # 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 - - # Enable code location reporting. - # Will be sent once per day. - # True by default. - # @return [Boolean] - attr_accessor :enable_code_locations - - # 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 - @enable_code_locations = true + def initialize(logger) + @logger = logger end - def before_emit=(value) - check_callable!("metrics.before_emit", value) - - @before_emit = value + def method_missing(m, *args, &block) + log_warn <<~MSG + `config.metrics` is now deprecated and will be removed in the next major. + MSG end end end diff --git a/sentry-ruby/lib/sentry/metrics/counter_metric.rb b/sentry-ruby/lib/sentry/metrics/counter_metric.rb deleted file mode 100644 index 6ef9d4d48..000000000 --- a/sentry-ruby/lib/sentry/metrics/counter_metric.rb +++ /dev/null @@ -1,25 +0,0 @@ -# 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 deleted file mode 100644 index eae9aff2a..000000000 --- a/sentry-ruby/lib/sentry/metrics/distribution_metric.rb +++ /dev/null @@ -1,25 +0,0 @@ -# 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 deleted file mode 100644 index f0ba98514..000000000 --- a/sentry-ruby/lib/sentry/metrics/gauge_metric.rb +++ /dev/null @@ -1,35 +0,0 @@ -# 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/local_aggregator.rb b/sentry-ruby/lib/sentry/metrics/local_aggregator.rb deleted file mode 100644 index 99d50a5b1..000000000 --- a/sentry-ruby/lib/sentry/metrics/local_aggregator.rb +++ /dev/null @@ -1,53 +0,0 @@ -# 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/metrics/metric.rb b/sentry-ruby/lib/sentry/metrics/metric.rb deleted file mode 100644 index dbf4a17e0..000000000 --- a/sentry-ruby/lib/sentry/metrics/metric.rb +++ /dev/null @@ -1,19 +0,0 @@ -# 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 deleted file mode 100644 index b38af2743..000000000 --- a/sentry-ruby/lib/sentry/metrics/set_metric.rb +++ /dev/null @@ -1,28 +0,0 @@ -# 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/metrics/timing.rb b/sentry-ruby/lib/sentry/metrics/timing.rb deleted file mode 100644 index 6d4d9b66d..000000000 --- a/sentry-ruby/lib/sentry/metrics/timing.rb +++ /dev/null @@ -1,51 +0,0 @@ -# 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 - - def duration_start - Process.clock_gettime(Process::CLOCK_MONOTONIC) - end - - def duration_end(start) - Process.clock_gettime(Process::CLOCK_MONOTONIC) - start - end - end - end - end -end diff --git a/sentry-ruby/lib/sentry/span.rb b/sentry-ruby/lib/sentry/span.rb index 256d8bd2f..a877e0ecb 100644 --- a/sentry-ruby/lib/sentry/span.rb +++ b/sentry-ruby/lib/sentry/span.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require "securerandom" -require "sentry/metrics/local_aggregator" module Sentry class Span @@ -187,9 +186,6 @@ def to_hash origin: @origin } - summary = metrics_summary - hash[:_metrics_summary] = summary if summary - hash end @@ -306,14 +302,5 @@ def set_tag(key, value) def set_origin(origin) @origin = origin 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 a1a8767d1..b01fde5f6 100644 --- a/sentry-ruby/lib/sentry/transaction_event.rb +++ b/sentry-ruby/lib/sentry/transaction_event.rb @@ -17,7 +17,8 @@ class TransactionEvent < Event # @return [Hash, nil] attr_accessor :profile - # @return [Hash, nil] + # @return [nil] + # @deprecated attr_accessor :metrics_summary def initialize(transaction:, **options) @@ -32,7 +33,6 @@ 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) @@ -53,7 +53,6 @@ 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 e069ccc5b..85bed8de5 100644 --- a/sentry-ruby/spec/sentry/client_spec.rb +++ b/sentry-ruby/spec/sentry/client_spec.rb @@ -101,7 +101,6 @@ def sentry_context let(:envelope) do envelope = Sentry::Envelope.new({ env_header: 1 }) envelope.add_item({ type: 'event' }, { payload: 'test' }) - envelope.add_item({ type: 'statsd' }, { payload: 'test2' }) envelope.add_item({ type: 'transaction' }, { payload: 'test3' }) envelope end @@ -151,7 +150,6 @@ def sentry_context end.to raise_error(Sentry::ExternalError) expect(subject.transport).to have_recorded_lost_event(:network_error, 'error') - expect(subject.transport).to have_recorded_lost_event(:network_error, 'metric_bucket') expect(subject.transport).to have_recorded_lost_event(:network_error, 'transaction') end end @@ -276,16 +274,6 @@ 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/envelope/item_spec.rb b/sentry-ruby/spec/sentry/envelope/item_spec.rb index ea9cf1019..dee701804 100644 --- a/sentry-ruby/spec/sentry/envelope/item_spec.rb +++ b/sentry-ruby/spec/sentry/envelope/item_spec.rb @@ -12,8 +12,6 @@ ['span', 'span'], ['profile', 'profile'], ['check_in', 'monitor'], - ['statsd', 'metric_bucket'], - ['metric_meta', 'metric_bucket'], ['event', 'error'], ['client_report', 'internal'], ['unknown', 'default'] diff --git a/sentry-ruby/spec/sentry/interfaces/stacktrace_builder_spec.rb b/sentry-ruby/spec/sentry/interfaces/stacktrace_builder_spec.rb index fd89360a9..c56268a2e 100644 --- a/sentry-ruby/spec/sentry/interfaces/stacktrace_builder_spec.rb +++ b/sentry-ruby/spec/sentry/interfaces/stacktrace_builder_spec.rb @@ -99,17 +99,4 @@ end end end - - describe '#metrics_code_location' do - it 'builds metrics code location hash for line' do - hash = subject.metrics_code_location(backtrace.first) - - expect(hash[:filename]).to match(/stacktrace_test_fixture.rb/) - expect(hash[:function]).to eq("bar") - expect(hash[:lineno]).to eq(8) - expect(hash[:pre_context]).to eq(["end\n", "\n", "def bar\n"]) - expect(hash[:context_line]).to eq(" baz\n") - expect(hash[:post_context]).to eq(["end\n", nil, nil]) - end - end end diff --git a/sentry-ruby/spec/sentry/metrics/aggregator_spec.rb b/sentry-ruby/spec/sentry/metrics/aggregator_spec.rb deleted file mode 100644 index b54c6a0d9..000000000 --- a/sentry-ruby/spec/sentry/metrics/aggregator_spec.rb +++ /dev/null @@ -1,487 +0,0 @@ -# frozen_string_literal: true - -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 include("[#{described_class.name}] 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 - - 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) - 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 - - describe 'code location reporting' do - it 'does not record location if off' do - perform_basic_setup do |config| - config.metrics.enabled = true - config.metrics.enable_code_locations = false - end - - subject.add(:c, 'incr', 1) - expect(subject.code_locations).to eq({}) - end - - it 'records the code location with a timestamp for the day' do - subject.add(:c, 'incr', 1, unit: 'second', stacklevel: 3) - - timestamp = Time.now.utc - start_of_day = Time.utc(timestamp.year, timestamp.month, timestamp.day).to_i - expect(subject.code_locations.keys.first).to eq(start_of_day) - end - - it 'has the code location keyed with mri (metric resource identifier) from type/key/unit' do - subject.add(:c, 'incr', 1, unit: 'second', stacklevel: 3) - mri = subject.code_locations.values.first.keys.first - expect(mri).to eq([:c, 'incr', 'second']) - end - - it 'has the code location information in the hash' do - subject.add(:c, 'incr', 1, unit: 'second', stacklevel: 3) - - location = subject.code_locations.values.first.values.first - expect(location).to include(:abs_path, :filename, :pre_context, :context_line, :post_context, :lineno) - expect(location[:abs_path]).to match(/aggregator_spec.rb/) - expect(location[:filename]).to match(/aggregator_spec.rb/) - expect(location[:context_line]).to include("subject.add(:c, 'incr', 1, unit: 'second', stacklevel: 3)") - end - - it 'does not add code location for the same mri twice' do - subject.add(:c, 'incr', 1, unit: 'second', stacklevel: 3) - subject.add(:c, 'incr', 1, unit: 'second', stacklevel: 3) - expect(subject.code_locations.values.first.size).to eq(1) - end - - it 'adds code location for different mris twice' do - subject.add(:c, 'incr', 1, unit: 'second', stacklevel: 3) - subject.add(:c, 'incr', 1, unit: 'none', stacklevel: 3) - expect(subject.code_locations.values.first.size).to eq(2) - end - end - end - - describe '#flush' do - context 'with empty buckets and empty locations' 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, 'disöt', 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, 'disöt', 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 'empties the pending code locations in place' do - subject.flush - expect(subject.code_locations).to eq({}) - end - - it 'captures the envelope' do - expect(subject.client).to receive(:capture_envelope) - 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("dis_t@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 - - it 'sends the pending code locations in metric_meta envelope item with correct payload' do - subject.flush - - envelope = sentry_envelopes.first - expect(envelope.headers).to eq({}) - - item = envelope.items.last - expect(item.headers).to eq({ type: 'metric_meta', content_type: 'application/json' }) - expect(item.payload[:timestamp]).to be_a(Integer) - - mapping = item.payload[:mapping] - expect(mapping.keys).to eq(['c:incr@none', 'd:dis_t@second']) - - location_1 = mapping['c:incr@none'].first - expect(location_1[:type]).to eq('location') - expect(location_1).to include(:abs_path, :filename, :lineno) - - location_2 = mapping['d:dis_t@second'].first - expect(location_2[:type]).to eq('location') - expect(location_2).to include(:abs_path, :filename, :lineno) - 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 'captures the envelope' 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("dis_t@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("dis_t@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 - - context 'sanitization' do - it 'sanitizes the metric key' do - subject.add(:c, 'foo.disöt_12-bar', 1) - subject.flush(force: true) - - sanitized_key = 'foo.dis_t_12-bar' - statsd, metrics_meta = sentry_envelopes.first.items.map(&:payload) - expect(statsd).to include(sanitized_key) - expect(metrics_meta[:mapping].keys.first).to include(sanitized_key) - end - - it 'sanitizes the metric unit' do - subject.add(:c, 'incr', 1, unit: 'disöt_12-/.test') - subject.flush(force: true) - - sanitized_unit = '@dist_12test' - statsd, metrics_meta = sentry_envelopes.first.items.map(&:payload) - expect(statsd).to include(sanitized_unit) - expect(metrics_meta[:mapping].keys.first).to include(sanitized_unit) - end - - it 'sanitizes tag keys and values' do - tags = { "get.foö-$bar/12" => "hello!\n\r\t\\ 42 this | or , that" } - subject.add(:c, 'incr', 1, tags: tags) - subject.flush(force: true) - - sanitized_tags = "get.fo-bar/12:hello!\\n\\r\\t\\\\ 42 this \\u{7c} or \\u{2c} that" - statsd = sentry_envelopes.first.items.first.payload - expect(statsd).to include(sanitized_tags) - 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 include("[#{described_class.name}] thread killed") - end - end -end diff --git a/sentry-ruby/spec/sentry/metrics/configuration_spec.rb b/sentry-ruby/spec/sentry/metrics/configuration_spec.rb index 3d04ad3ea..87611c353 100644 --- a/sentry-ruby/spec/sentry/metrics/configuration_spec.rb +++ b/sentry-ruby/spec/sentry/metrics/configuration_spec.rb @@ -3,11 +3,18 @@ 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)') + let(:stringio) { StringIO.new } + subject { described_class.new(::Logger.new(stringio)) } + + %i[enabled enable_code_locations before_emit].each do |method| + describe "#{method}=" do + it 'logs deprecation warning' do + subject.send("#{method}=", true) + + expect(stringio.string).to include( + "WARN -- sentry: `config.metrics` is now deprecated and will be removed in the next major." + ) + end 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 deleted file mode 100644 index f1c480b45..000000000 --- a/sentry-ruby/spec/sentry/metrics/counter_metric_spec.rb +++ /dev/null @@ -1,27 +0,0 @@ -# frozen_string_literal: true - -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 deleted file mode 100644 index e8ac709f3..000000000 --- a/sentry-ruby/spec/sentry/metrics/distribution_metric_spec.rb +++ /dev/null @@ -1,27 +0,0 @@ -# frozen_string_literal: true - -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 deleted file mode 100644 index fce3141c6..000000000 --- a/sentry-ruby/spec/sentry/metrics/gauge_metric_spec.rb +++ /dev/null @@ -1,31 +0,0 @@ -# frozen_string_literal: true - -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/local_aggregator_spec.rb b/sentry-ruby/spec/sentry/metrics/local_aggregator_spec.rb deleted file mode 100644 index 3056808da..000000000 --- a/sentry-ruby/spec/sentry/metrics/local_aggregator_spec.rb +++ /dev/null @@ -1,85 +0,0 @@ -# frozen_string_literal: true - -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/metric_spec.rb b/sentry-ruby/spec/sentry/metrics/metric_spec.rb deleted file mode 100644 index ccb4fa222..000000000 --- a/sentry-ruby/spec/sentry/metrics/metric_spec.rb +++ /dev/null @@ -1,23 +0,0 @@ -# frozen_string_literal: true - -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 deleted file mode 100644 index 2cd01da83..000000000 --- a/sentry-ruby/spec/sentry/metrics/set_metric_spec.rb +++ /dev/null @@ -1,32 +0,0 @@ -# frozen_string_literal: true - -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/timing_spec.rb b/sentry-ruby/spec/sentry/metrics/timing_spec.rb deleted file mode 100644 index 3b2ee2235..000000000 --- a/sentry-ruby/spec/sentry/metrics/timing_spec.rb +++ /dev/null @@ -1,56 +0,0 @@ -# frozen_string_literal: true - -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 e50c55074..5cb8a79f7 100644 --- a/sentry-ruby/spec/sentry/metrics_spec.rb +++ b/sentry-ruby/spec/sentry/metrics_spec.rb @@ -3,136 +3,32 @@ require 'spec_helper' RSpec.describe Sentry::Metrics do + let(:stringio) { StringIO.new } + before do perform_basic_setup do |config| - config.metrics.enabled = true + config.logger = ::Logger.new(stringio) 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 - - 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) - result = described_class.timing('foo', unit: 'ratio') { 42 } - expect(result).to eq(42) - 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 - ) - - result = described_class.timing('foo', unit: 'millisecond', tags: { fortytwo: 42 }, timestamp: fake_time) { sleep(0.1); 42 } - expect(result).to eq(42) - 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', origin: Sentry::Metrics::SPAN_ORIGIN).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') + %i[increment distribution set gauge timing distribution].each do |method| + describe ".#{method}" do + it 'logs deprecation warning' do + described_class.send( + method, + 'foo', + 5.0, + unit: 'second', + tags: { fortytwo: 42 }, + timestamp: fake_time + ) + + expect(stringio.string).to include( + "WARN -- sentry: `Sentry::Metrics` is now deprecated and will be removed in the next major." + ) end end end diff --git a/sentry-ruby/spec/sentry/span_spec.rb b/sentry-ruby/spec/sentry/span_spec.rb index 0d1fd225c..636d2a605 100644 --- a/sentry-ruby/spec/sentry/span_spec.rb +++ b/sentry-ruby/spec/sentry/span_spec.rb @@ -77,16 +77,6 @@ expect(hash[:span_id].length).to eq(16) expect(hash[:origin]).to eq('manual') 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 diff --git a/sentry-ruby/spec/sentry/transport_spec.rb b/sentry-ruby/spec/sentry/transport_spec.rb index 059a8109b..a6c3703e9 100644 --- a/sentry-ruby/spec/sentry/transport_spec.rb +++ b/sentry-ruby/spec/sentry/transport_spec.rb @@ -213,30 +213,6 @@ 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 9a188e9f3..90dc726c9 100644 --- a/sentry-ruby/spec/sentry_spec.rb +++ b/sentry-ruby/spec/sentry_spec.rb @@ -1139,14 +1139,6 @@ 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).to receive(:flush) described_class.close