basho / riak_kv

Riak Key/Value Store
Apache License 2.0
653 stars 233 forks source link

queue stats logging refinements #1875

Open hmmr opened 1 year ago

hmmr commented 1 year ago

This PR has a few refinements to logging for 2.9.x users:

hmmr commented 1 year ago

An example configuration with filtering:

{lager,
 [
  {extra_sinks,
   [
    {object_lager_event,
     [{handlers,
       [{lager_file_backend,
         [{file, "./log/object.log"},
          {level, info},
          {formatter_config, [date, " ", time," [",severity,"] ",message, "\n"]}
         ]
        }]
      },
      {async_threshold, 500},
      {async_threshold_window, 50}]
    }
   ]
  },
  {traces, [
            {{lager_file_backend, "./log/tta_noise.log"}, [{queue_name, '=', q1_ttaaefs}], info}
           ]}
 ]
}
martinsumner commented 1 year ago

Some comments, but no strong opinion against merging this as-is.

I think there are issues if you upgrade riak, but have config items that are not included in the next riak release. So if we introduce this schema element in 2.9, anyone upgrading to 3.0 will need to remove it from riak.conf. I haven't verified this, working from memory.

It probably makes sense to make the name more general (i.e. queue_manager_log_frequency), and then add the same schema item with the same effect in develop-3.0/develop. Avoids this issue, and also the reaper/eraser processes aren't directly related to replrtq, so this would be a truer meaning.

I'm happy for empty queues to be debug by default. I do know that there is a customer that may prefer to still receive these logs. Should there also be a queue_manager_suppress_empty_log config flag which is off by default), so that this can be controlled - especially as you're taking something away that is currently used to feed production monitoring charts in that environment.

In general, looks fine to me.

hmmr commented 1 year ago

I have renamed the log frequency tunable as you suggested, and reverted separate log levels for zero/non-zero stats, in favor of it being controlled by another tunable, queue_manager_suppress_empty_log.

I'm going to create similar PRs for 3.0 and 3.2 branches.

Indeed, adding a new parameter to riak.conf will cause a slight hiccup in the upgrade procedure, but once we add these tunables to the upcoming releases of 3.0 and 3.2 lines, we should be good.