UX: Keep the 50 most recent envs rather than the first 50 (#103)

* UX: Keep the 50 most recent envs rather than the first 50

* store envs in a list for each message

* New settings

* Rename settings

* slice one character at a time

* Add documentation to readme

* Add default value

* This should be a clearer copy

README.md

@@ -44,7 +44,10 @@ Logster can be configured using `Logster.config`:
 - `Logster.config.rate_limit_error_reporting` : controls automatic 1 minute rate limiting for JS error reporting.
 - `Logster.config.web_title` : `<title>` tag for logster error page.
 - `Logster.config.enable_custom_patterns_via_ui` : enable the settings page (`/settings`) where you can add suppression and grouping patterns.
-- `Logster.config.maximum_message_size_bytes` : specify a size in bytes that a message cannot exceed. Note this isn't 100% accurate, meaning a message may still grow above the limit, but it shouldn't grow by more than, say, 2000 bytes.
+- `Logster.config.allow_grouping` : Enable grouping of similar messages into one messages with an array of `env` of the grouped messages. Similar messages are messages that have identical backtraces, severity and log message.
+- `Logster.config.maximum_message_size_bytes` : set a maximum size for messages. Default value is 10,000. If a message size exceeds this limit, Logster will first remove all occurrences of `gems_dir` (more on this config below) from the backtrace and computes the size again; if the message is still above the limit, Logster will iteratively remove a line from the end of the backtrace until the size becomes below the limit, or one line remains. At this point if the message size is still above the limit, Logster will remove as many as character as needed to bring the size below the limit. However, before all of this if Logster figures out that removing the whole backtrace will not bring the size below the limit, it'll give up early and not attempt to reduce the size at all. So it's not recommended to set this config to a really low value e.g. less than 2000.
+- `Logster.config.max_env_bytes` : set a maximum size for `env`. Default value is 1000. In case `env` is an array of hashes, this limit applies to the individual hashes in the array rather than the whole array. If an `env` hash exceeds this limit, Logster will take the biggest subset of key-value pairs whose size is below the limit. If the hash has a key with the name `time`, it will always be included.
+- `Logster.config.max_env_count_per_message` : default value is 50. Logster can merge messages that have the same backtrace, severity and log message into one grouping message that have many `env` hashes. This config specifies the maximum number of `env` hashes a grouping message is allowed to keep. If this limit is reached and a new similar message is created and it needs to be merged, Logster will remove the oldest `env` hash from the grouping message and adds the new one.
 - `Logster.config.project_directories` : This should be an array of hashes that map paths on the local filesystem to GitHub repository URLs. If this feature is enabled, Logster will parse backtraces and try to construct a GitHub URL to the exact file and line number for each line in the backtrace. For a Rails app, the config may look like this: `Logster.config.project_directories = [{ path: Rails.root.to_s, url: "https://github.com/<your_org>/<your_repo>" }]`. The GitHub links that are constructed will use the `master` branch. If you want Logster to use the `application_version` attribute from the `env` tab so that the GitHub links point to the exact version of the app when the log message is created, add `main_app: true` key to the hash.
 - `Logster.config.enable_backtrace_links` : Enable/disable the backtrace links feature.
 - `Logster.config.gems_dir` : The value of this config is `Gem.dir + "/gems/"` by default. You probably don't need to change this config, but it's available in case your app gems are installed in a different directory. An example where this config is needed is Logster [demo site](http://logster.info/logs/): [https://github.com/discourse/logster/blob/master/website/sample.rb#L77](https://github.com/discourse/logster/blob/master/website/sample.rb#L77).

lib/logster/base_store.rb

@@ -18,7 +18,7 @@ def save(message)
     end
 
     # Modify the saved message to the given one (identified by message.key) and bump it to the top of the latest list
-    def replace_and_bump(message, save_env: true)
+    def replace_and_bump(message)
       not_implemented
     end
 
@@ -199,15 +199,17 @@ def report(severity, progname, msg, opts = {})
         similar = get(key, load_env: false) if key
       end
 
+      message.drop_redundant_envs(Logster.config.max_env_count_per_message)
+      message.apply_env_size_limit(Logster.config.max_env_bytes)
       if similar
-        if similar.count < Logster::MAX_GROUPING_LENGTH
-          similar.env = get_env(similar.key) || {}
-        end
-        save_env = similar.merge_similar_message(message)
-
-        replace_and_bump(similar, save_env: save_env)
+        similar.merge_similar_message(message)
+        replace_and_bump(similar)
         similar
       else
+        message.apply_message_size_limit(
+          Logster.config.maximum_message_size_bytes,
+          gems_dir: Logster.config.gems_dir
+        )
         save message
         message
       end

lib/logster/configuration.rb

@@ -15,7 +15,9 @@ class Configuration
       :maximum_message_size_bytes,
       :project_directories,
       :enable_backtrace_links,
-      :gems_dir
+      :gems_dir,
+      :max_env_bytes,
+      :max_env_count_per_message
     )
 
     attr_writer :subdirectory
@@ -29,7 +31,9 @@ def initialize
       @enable_custom_patterns_via_ui = false
       @rate_limit_error_reporting = true
       @enable_js_error_reporting = true
-      @maximum_message_size_bytes = 60_000
+      @maximum_message_size_bytes = 10_000
+      @max_env_bytes = 1000
+      @max_env_count_per_message = 50
       @project_directories = []
       @enable_backtrace_links = true
       @gems_dir = Gem.dir + "/gems/"

lib/logster/message.rb

@@ -4,8 +4,6 @@
 require 'securerandom'
 
 module Logster
-
-  MAX_GROUPING_LENGTH = 50
   MAX_MESSAGE_LENGTH = 600
 
   class Message
@@ -22,9 +20,10 @@ class Message
       hostname
       process_id
       application_version
+      time
     }
 
-    attr_accessor :timestamp, :severity, :progname, :key, :backtrace, :count, :protected, :first_timestamp
+    attr_accessor :timestamp, :severity, :progname, :key, :backtrace, :count, :protected, :first_timestamp, :env_buffer
     attr_reader :message, :env
 
     def initialize(severity, progname, message, timestamp = nil, key = nil, count: 1)
@@ -37,6 +36,7 @@ def initialize(severity, progname, message, timestamp = nil, key = nil, count: 1
       @count = count || 1
       @protected = false
       @first_timestamp = nil
+      @env_buffer = []
     end
 
     def to_h(exclude_env: false)
@@ -82,7 +82,6 @@ def self.from_json(json)
     end
 
     def env=(env)
-      @env_json = nil
       @env = self.class.scrub_params(env)
     end
 
@@ -94,12 +93,18 @@ def populate_from_env(env)
       env ||= {}
       if Array === env
         env = env.map do |single_env|
-          self.class.default_env.merge(single_env)
+          single_env = self.class.default_env.merge(single_env)
+          if !single_env.key?("time") && !single_env.key?(:time)
+            single_env["time"] = @timestamp || get_timestamp
+          end
+          single_env
         end
       else
         env = self.class.default_env.merge(env)
+        if !env.key?("time") && !env.key?(:time)
+          env["time"] = @timestamp || get_timestamp
+        end
       end
-      @env_json = nil
       @env = Message.populate_from_env(env)
     end
 
@@ -146,31 +151,24 @@ def is_similar?(other)
     def merge_similar_message(other)
       self.first_timestamp ||= self.timestamp
       self.timestamp = [self.timestamp, other.timestamp].max
-
       self.count += other.count || 1
-      return false if self.count > Logster::MAX_GROUPING_LENGTH
-
-      size = self.to_json(exclude_env: true).bytesize + self.env_json.bytesize
-      extra_env_size = other.env_json.bytesize
-      return false if size + extra_env_size > Logster.config.maximum_message_size_bytes
 
-      other_env = JSON.load JSON.fast_generate other.env
-      if Hash === other_env && !other_env.key?("time")
-        other_env["time"] = other.timestamp
-      end
-      if Hash === self.env && !self.env.key?("time")
-        self.env["time"] = self.first_timestamp
+      if Hash === other.env && !other.env.key?("time") && !other.env.key?(:time)
+        other.env["time"] = other.timestamp
       end
 
-      if Array === self.env
-        Array === other_env ? self.env.concat(other_env) : self.env << other_env
+      if Array === other.env
+        env_buffer.unshift(*other.env)
       else
-        Array === other_env ? self.env = [self.env, *other_env] : self.env = [self.env, other_env]
+        env_buffer.unshift(other.env)
       end
-      @env_json = nil
       true
     end
 
+    def has_env_buffer?
+      env_buffer.size > 0
+    end
+
     def self.populate_from_env(env)
       if Array === env
         env.map do |single_env|
@@ -229,10 +227,6 @@ def =~(pattern)
       end
     end
 
-    def env_json
-      @env_json ||= (self.env || {}).to_json
-    end
-
     def self.scrub_params(params)
       if Array === params
         params.map! { |p| scrub_params(p) }
@@ -250,8 +244,77 @@ def self.scrub_params(params)
       end
     end
 
+    def drop_redundant_envs(limit)
+      if Array === env
+        env.slice!(limit..-1)
+      end
+    end
+
+    def apply_env_size_limit(size_limit)
+      if Array === env
+        env.each { |e| truncate_env(e, size_limit) }
+      elsif Hash === env
+        truncate_env(env, size_limit)
+      end
+    end
+
+    def apply_message_size_limit(limit, gems_dir: nil)
+      size = self.to_json(exclude_env: true).bytesize
+      if size > limit && @backtrace
+        backtrace_limit = limit - (size - @backtrace.bytesize)
+        @backtrace.gsub!(gems_dir, "") if gems_dir
+        @backtrace.strip!
+        orig = @backtrace.dup
+        stop = false
+        while @backtrace.bytesize > backtrace_limit && backtrace_limit > 0 && !stop
+          lines = @backtrace.lines
+          if lines.size > 1
+            lines.pop
+            @backtrace = lines.join
+          else
+            @backtrace.slice!(-1)
+          end
+          # protection to ensure we never get stuck
+          stop = orig == @backtrace
+        end
+      end
+    end
+
     protected
 
+    def truncate_env(env, limit)
+      if JSON.fast_generate(env).bytesize > limit
+        sizes = {}
+        braces = '{}'.bytesize
+        env.each do |k,v|
+          sizes[k] = JSON.fast_generate(k => v).bytesize - braces
+        end
+        sorted = env.keys.sort { |a,b| sizes[a] <=> sizes[b] }
+
+        kept_keys = []
+        if env.key?(:time)
+          kept_keys << :time
+        elsif env.key?("time")
+          kept_keys << "time"
+        end
+
+        sum = braces
+        if time_key = kept_keys.first
+          sum += sizes[time_key]
+          sorted.delete(time_key)
+        end
+        comma = ','.bytesize
+
+        sorted.each do |k|
+          extra = kept_keys.size == 0 ? 0 : comma
+          break if sum + sizes[k] + extra > limit
+          kept_keys << k
+          sum += sizes[k] + extra
+        end
+        env.select! { |k| kept_keys.include?(k) }
+      end
+    end
+
     def truncate_message(msg)
       return msg unless msg
       msg = msg.inspect unless String === msg