I have looked at the implementation, and I would suggest some bigger changes.

The main concern for me is SetLimiter.

• we need to call it before every log line. Otherwise, logs could be ignored
• the first call of SetLimiter defines the limit for all logs afterward

I would suggest defining rate limiting when an instance of NewRateLimitLogger. That allows us to create logs with different rates and use them. i.e.

logRateLimit5min = logging.NewRateLimitLogger(logging.LoggerForModule(), 100, 1, 5*time.Minute, 1)
logRateLimit10sec = logging.NewRateLimitLogger(logging.LoggerForModule(), 100, 1, 10*time.Second, 1)

And then we can call log.

logRateLimit5min.WarnL(ri.Hostname, "Token validation failed for hostname %v: %v", ri.Hostname, err)

This would require a change in allowLog. We should create a limiter if it does not exist. i.e.

func (rl *RateLimitedLogger) allowLog(limiter string) bool {
   if lim, ok := rl.rateLimiters.Get(limiter); ok {
      return lim.(*rate.Limiter).Allow()
   }

   rl.rateLimiters.Add(limiter, rate.NewLimiter(rl.baseLimit, rl.burst))

   return true
}

I assume that rl.baseLimit and rl.burst are created with a constructor.

@clickboo What do you think about these suggestions?

I have looked at the solution. It looks good. I like what you did here.

I would not completely rely on ingress. Ingres is something that we will use in ACSCS. And with that in place, we don't even need this functionality. This PR is really relevant for on-prem deployments too. For on-prem deployments, we don't know if the customer's infrastructure has such kind of protection or not. We know that without rate-limiting logs, there is a problem that central crashes because of accumulated logs. As I remember from our meeting, that's one reason we are adding this functionality in central.

There is one thing that I'm interested in, and I don't know if that is relevant for this implementation because it's not described in the ticket, so it's hard to know.

Hypothetical case. Let's say we have code like this:

log.WarnL(ri.Hostname, "Record 1")
// several lines of code
log.WarnL(ri.Hostname, "Record 2")

Our current implementation will only output the following:

Record 1

2nd log line will never be output, regardless of execution reaching it. Could that be a problem that we have partial context and we are losing important information?

Besides that, we can get this in. If we notice that additional functionality is required or that we can improve something, we can always add it.