Unify Logs and Metrics implementations (#2826)

* Unify Logs and Metrics implementations

* Make `LogEvent` go through `scope.apply_to_telemetry`
* Populate default and user attributes in `scope.apply_to_telemetry`
* Simplify LogEvent to be more simpler with serialization logic, remove
  all unnecessary fields

* Reduce hash allocations

* Update sentry-ruby/lib/sentry/log_event.rb

Co-authored-by: Peter Solnica <[email protected]>

* Fix transform_values

* Optimize scope attribute setting

---------

Co-authored-by: Peter Solnica <[email protected]>

Author: sl0thentr0py

CHANGELOG.md

@@ -43,6 +43,10 @@
   ```
 - Prevent SDK crash when SDK logging fails ([#2817](https://github.com/getsentry/sentry-ruby/pull/2817))
 
+### Internal
+
+- Unify Logs and Metrics implementations ([#2826](https://github.com/getsentry/sentry-ruby/pull/2826))
+
 ## 6.2.0
 
 ### Features

sentry-ruby/lib/sentry/client.rb

@@ -107,7 +107,7 @@ def capture_event(event, scope, hint = {})
     # @return [LogEvent]
     def buffer_log_event(event, scope)
       return unless event.is_a?(LogEvent)
-      @log_event_buffer.add_event(scope.apply_to_event(event))
+      @log_event_buffer.add_event(scope.apply_to_telemetry(event))
       event
     end
 

sentry-ruby/lib/sentry/log_event.rb

@@ -1,129 +1,52 @@
 # frozen_string_literal: true
 
-require "json"
+require "sentry/utils/telemetry_attributes"
 
 module Sentry
   # Event type that represents a log entry with its attributes
   #
   # @see https://develop.sentry.dev/sdk/telemetry/logs/#log-envelope-item-payload
   class LogEvent
+    include Sentry::Utils::TelemetryAttributes
+
     TYPE = "log"
 
     DEFAULT_PARAMETERS = [].freeze
-    DEFAULT_ATTRIBUTES = {}.freeze
-
-    SERIALIZEABLE_ATTRIBUTES = %i[
-      level
-      body
-      timestamp
-      environment
-      release
-      server_name
-      trace_id
-      attributes
-      contexts
-    ]
-
-    SENTRY_ATTRIBUTES = {
-      "sentry.trace.parent_span_id" => :parent_span_id,
-      "sentry.environment" => :environment,
-      "sentry.release" => :release,
-      "sentry.address" => :server_name,
-      "sentry.sdk.name" => :sdk_name,
-      "sentry.sdk.version" => :sdk_version,
-      "sentry.message.template" => :template,
-      "sentry.origin" => :origin
-    }
 
     PARAMETER_PREFIX = "sentry.message.parameter"
 
-    USER_ATTRIBUTES = {
-      "user.id" => :user_id,
-      "user.name" => :user_username,
-      "user.email" => :user_email
-    }
-
     LEVELS = %i[trace debug info warn error fatal].freeze
 
-    attr_accessor :level, :body, :template, :attributes, :user, :origin
-
-    attr_reader :configuration, *(SERIALIZEABLE_ATTRIBUTES - %i[level body attributes])
-
-    SERIALIZERS = %i[
-      attributes
-      body
-      level
-      parent_span_id
-      sdk_name
-      sdk_version
-      template
-      timestamp
-      trace_id
-      user_id
-      user_username
-      user_email
-    ].map { |name| [name, :"serialize_#{name}"] }.to_h
+    attr_accessor :level, :body, :template, :attributes, :origin, :trace_id, :span_id
+    attr_reader :timestamp
 
     TOKEN_REGEXP = /%\{(\w+)\}/
 
-    def initialize(configuration: Sentry.configuration, **options)
-      @configuration = configuration
+    def initialize(**options)
       @type = TYPE
-      @server_name = configuration.server_name
-      @environment = configuration.environment
-      @release = configuration.release
       @timestamp = Sentry.utc_now
       @level = options.fetch(:level)
       @body = options[:body]
       @template = @body if is_template?
-      @attributes = options[:attributes] || DEFAULT_ATTRIBUTES
-      @user = options[:user] || {}
+      @attributes = options[:attributes] || {}
       @origin = options[:origin]
-      @contexts = {}
+      @trace_id = nil
+      @span_id = nil
     end
 
     def to_h
-      SERIALIZEABLE_ATTRIBUTES.each_with_object({}) do |name, memo|
-        memo[name] = serialize(name)
-      end
+      {
+        level: level.to_s,
+        timestamp: timestamp.to_f,
+        trace_id: @trace_id,
+        span_id: @span_id,
+        body: serialize_body,
+        attributes: serialize_attributes
+      }.compact
     end
 
     private
 
-    def serialize(name)
-      serializer = SERIALIZERS[name]
-
-      if serializer
-        __send__(serializer)
-      else
-        public_send(name)
-      end
-    end
-
-    def serialize_level
-      level.to_s
-    end
-
-    def serialize_sdk_name
-      Sentry.sdk_meta["name"]
-    end
-
-    def serialize_sdk_version
-      Sentry.sdk_meta["version"]
-    end
-
-    def serialize_timestamp
-      timestamp.to_f
-    end
-
-    def serialize_trace_id
-      contexts.dig(:trace, :trace_id)
-    end
-
-    def serialize_parent_span_id
-      contexts.dig(:trace, :parent_span_id)
-    end
-
     def serialize_body
       if parameters.empty?
         body
@@ -134,61 +57,14 @@ def serialize_body
       end
     end
 
-    def serialize_user_id
-      user[:id]
-    end
-
-    def serialize_user_username
-      user[:username]
-    end
-
-    def serialize_user_email
-      user[:email]
-    end
-
-    def serialize_template
-      template if has_parameters?
-    end
-
     def serialize_attributes
-      hash = {}
-
-      attributes.each do |key, value|
-        hash[key] = attribute_hash(value)
-      end
-
-      SENTRY_ATTRIBUTES.each do |key, name|
-        if (value = serialize(name))
-          hash[key] = attribute_hash(value)
-        end
-      end
-
-      USER_ATTRIBUTES.each do |key, name|
-        if (value = serialize(name))
-          hash[key] = value
-        end
-      end
-
-      hash
+      populate_sentry_attributes!
+      @attributes.transform_values! { |v| attribute_hash(v) }
     end
 
-    def attribute_hash(value)
-      case value
-      when String
-        { value: value, type: "string" }
-      when TrueClass, FalseClass
-        { value: value, type: "boolean" }
-      when Integer
-        { value: value, type: "integer" }
-      when Float
-        { value: value, type: "double" }
-      else
-        begin
-          { value: JSON.generate(value), type: "string" }
-        rescue
-          { value: value, type: "string" }
-        end
-      end
+    def populate_sentry_attributes!
+      @attributes["sentry.origin"] ||= @origin if @origin
+      @attributes["sentry.message.template"] ||= template if has_parameters?
     end
 
     def parameters

sentry-ruby/lib/sentry/metric_event.rb

@@ -1,9 +1,13 @@
 # frozen_string_literal: true
 
+require "sentry/utils/telemetry_attributes"
+
 module Sentry
   class MetricEvent
-    attr_reader :name, :type, :value, :unit, :timestamp, :trace_id, :span_id, :attributes, :user
-    attr_writer :trace_id, :span_id, :attributes, :user
+    include Sentry::Utils::TelemetryAttributes
+
+    attr_reader :name, :type, :value, :unit, :timestamp, :trace_id, :span_id, :attributes
+    attr_writer :trace_id, :span_id, :attributes
 
     def initialize(
       name:,
@@ -21,13 +25,9 @@ def initialize(
       @timestamp = Sentry.utc_now
       @trace_id = nil
       @span_id = nil
-      @user = {}
     end
 
     def to_h
-      populate_default_attributes!
-      populate_user_attributes!
-
       {
         name: @name,
         type: @type,
@@ -42,44 +42,8 @@ def to_h
 
     private
 
-    def populate_default_attributes!
-      configuration = Sentry.configuration
-      return unless configuration
-
-      default_attributes = {
-        "sentry.environment" => configuration.environment,
-        "sentry.release" => configuration.release,
-        "sentry.sdk.name" => Sentry.sdk_meta["name"],
-        "sentry.sdk.version" => Sentry.sdk_meta["version"],
-        "server.address" => configuration.server_name
-      }.compact
-
-      @attributes = default_attributes.merge(@attributes)
-    end
-
-    def populate_user_attributes!
-      return unless @user
-      return unless Sentry.initialized? && Sentry.configuration.send_default_pii
-
-      user_attributes = {
-        "user.id" => @user[:id],
-        "user.name" => @user[:username],
-        "user.email" => @user[:email]
-      }.compact
-
-      @attributes = user_attributes.merge(@attributes)
-    end
-
     def serialize_attributes
-      @attributes.transform_values do |v|
-        case v
-        when Integer then { type: "integer", value: v }
-        when Float then { type: "double", value: v }
-        when TrueClass, FalseClass then { type: "boolean", value: v }
-        when String then { type: "string", value: v }
-        else { type: "string", value: v.to_s }
-        end
-      end
+      @attributes.transform_values! { |v| attribute_hash(v) }
     end
   end
 end

sentry-ruby/lib/sentry/scope.rb

@@ -46,7 +46,7 @@ def clear
     # @param hint [Hash] the hint data that'll be passed to event processors.
     # @return [Event]
     def apply_to_event(event, hint = nil)
-      unless event.is_a?(CheckInEvent) || event.is_a?(LogEvent)
+      unless event.is_a?(CheckInEvent)
         event.tags = tags.merge(event.tags)
         event.user = user.merge(event.user)
         event.extra = extra.merge(event.extra)
@@ -60,10 +60,6 @@ def apply_to_event(event, hint = nil)
         event.attachments = attachments
       end
 
-      if event.is_a?(LogEvent)
-        event.user = user.merge(event.user)
-      end
-
       if span
         event.contexts[:trace] ||= span.get_trace_context
 
@@ -92,18 +88,31 @@ def apply_to_event(event, hint = nil)
     # A leaner version of apply_to_event that applies to
     # lightweight payloads like Logs and Metrics.
     #
-    # Only adds trace_id, span_id and user from the scope.
+    # Adds trace_id, span_id, user from the scope and default attributes from configuration.
     #
-    # @param telemetry [MetricEvent]
-    # @return [MetricEvent]
+    # @param telemetry [MetricEvent, LogEvent] the telemetry event to apply scope context to
+    # @return [MetricEvent, LogEvent] the telemetry event with scope context applied
     def apply_to_telemetry(telemetry)
       # TODO-neel when new scope set_attribute api is added: add them here
-      telemetry.user = user.merge(telemetry.user)
-
       trace_context = span ? span.get_trace_context : propagation_context.get_trace_context
       telemetry.trace_id = trace_context[:trace_id]
       telemetry.span_id = trace_context[:span_id]
 
+      configuration = Sentry.configuration
+      return telemetry unless configuration
+
+      telemetry.attributes["sentry.sdk.name"] ||= Sentry.sdk_meta["name"]
+      telemetry.attributes["sentry.sdk.version"] ||= Sentry.sdk_meta["version"]
+      telemetry.attributes["sentry.environment"] ||= configuration.environment if configuration.environment
+      telemetry.attributes["sentry.release"] ||= configuration.release if configuration.release
+      telemetry.attributes["server.address"] ||= configuration.server_name if configuration.server_name
+
+      if configuration.send_default_pii && !user.empty?
+        telemetry.attributes["user.id"] ||= user[:id] if user[:id]
+        telemetry.attributes["user.name"] ||= user[:username] if user[:username]
+        telemetry.attributes["user.email"] ||= user[:email] if user[:email]
+      end
+
       telemetry
     end
 

sentry-ruby/lib/sentry/utils/telemetry_attributes.rb

@@ -0,0 +1,30 @@
+# frozen_string_literal: true
+
+require "json"
+
+module Sentry
+  module Utils
+    module TelemetryAttributes
+      private
+
+      def attribute_hash(value)
+        case value
+        when String
+          { value: value, type: "string" }
+        when TrueClass, FalseClass
+          { value: value, type: "boolean" }
+        when Integer
+          { value: value, type: "integer" }
+        when Float
+          { value: value, type: "double" }
+        else
+          begin
+            { value: JSON.generate(value), type: "string" }
+          rescue
+            { value: value, type: "string" }
+          end
+        end
+      end
+    end
+  end
+end