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

Author: sl0thentr0py

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,53 @@
 # 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::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] || {}
       @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 +58,15 @@ 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
+      sentry_attributes.merge(@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 sentry_attributes
+      sentry_attributes = {}
+      sentry_attributes["sentry.origin"] = @origin if @origin
+      sentry_attributes["sentry.message.template"] = template if has_parameters?
+      sentry_attributes
     end
 
     def parameters

sentry-ruby/lib/sentry/metric_event.rb

@@ -1,9 +1,15 @@
 # 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::TelemetryAttributes
+
+    DEFAULT_ATTRIBUTES = {}.freeze
+
+    attr_reader :name, :type, :value, :unit, :timestamp, :trace_id, :span_id, :attributes
+    attr_writer :trace_id, :span_id, :attributes
 
     def initialize(
       name:,
@@ -16,18 +22,14 @@ def initialize(
       @type = type
       @value = value
       @unit = unit
-      @attributes = attributes || {}
+      @attributes = attributes || DEFAULT_ATTRIBUTES
 
       @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 +44,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

@@ -92,18 +92,39 @@ 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
+
+      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
+
+      telemetry.attributes = default_attributes.merge(telemetry.attributes)
+
+      if configuration.send_default_pii && !user.empty?
+        user_attributes = {
+          "user.id" => user[:id],
+          "user.name" => user[:username],
+          "user.email" => user[:email]
+        }.compact
+
+        telemetry.attributes = user_attributes.merge(telemetry.attributes)
+      end
+
       telemetry
     end
 

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

@@ -0,0 +1,28 @@
+# frozen_string_literal: true
+
+require "json"
+
+module Sentry
+  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

sentry-ruby/spec/sentry/log_event_buffer_spec.rb

@@ -8,7 +8,6 @@
   let(:client) { double(Sentry::Client) }
   let(:log_event) do
     Sentry::LogEvent.new(
-      configuration: Sentry.configuration,
       level: :info,
       body: "Test message"
     )