4
\$\begingroup\$

I want to limit the request rate in a spray/akka-http route. I'm not an expert with both scala and spray/akka but learning. So I came up with the following custom directive:

  trait RequestRateLimiter[T] {
    def check(ctx: RequestContext, realIp: Option[RemoteAddress], proxiedIp: Option[RemoteAddress]) : RequestRateResult[T]
  }

  sealed trait RequestRateResult[T]
  case class Pass[T](result: T) extends RequestRateResult[T]
  case class Block[T](rejection: Rejection) extends RequestRateResult[T]

  def limit[T](requestRateLimiter: RequestRateLimiter[T]): Directive1[T] = {
    def remoteAddress: Directive1[Option[RemoteAddress]] = optionalHeaderValuePF { case `Remote-Address`(address) ⇒ address }
    def xForwardedFor: Directive1[Option[RemoteAddress]] = optionalHeaderValuePF { case `X-Forwarded-For`(Seq(address, _*)) ⇒ address }
    extract(identity) flatMap { ctx =>
      remoteAddress.flatMap { realIp =>
        xForwardedFor.flatMap { proxiedIp =>
          requestRateLimiter.check(ctx, realIp, proxiedIp) match {
            case Pass(result)     => provide(result)
            case Block(rejection) => reject(rejection)
          }
        }
      }
    }
  }

The custom directive takes an instance of an RequestRateLimiter which keeps track of the request rate based on the RequestContext and RemoteAddress. The RequestRateLimiter then decides if the request should be passed or blocked. I'm not quiet happy with the RequestRateLimiter because it's too generic. It can be anything that either provides something or rejects.

I also posted a Gist (feel free to share, use and improve).

What do you think of the directive? Anything that could be improved?

\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! Good job on your first question \$\endgroup\$ – SirPython Feb 21 '16 at 19:25
  • \$\begingroup\$ It looks to me like you could replace your suite of flatMap by a for comprehension. It would make it more readable, I think. \$\endgroup\$ – Olivier Feb 26 '16 at 3:42
5
\$\begingroup\$

I think you could rewrite your limit function like this :

def limit[T](requestRateLimiter: RequestRateLimiter[T]) = {
  def remoteAddress = optionalHeaderValuePF { case `Remote-Address`(address) ⇒ address }
  def xForwardedFor = optionalHeaderValuePF { case `X-Forwarded-For`(Seq(address, _*)) ⇒ address }

  for {
    ctx <- extract(identity)
    realIp <- remoteAddress
    proxiedIp <- xForwardedFor
  } yield requestRateLimiter.check(ctx, realIp, proxiedIp) match {
    case Pass(result) => provide(result)
    case Block(rejection) => reject(rejection)
  }
}

Here are the two main changes that I've done :

  1. Changed your flatMaps for a "for comprehension" (which imho is more readable)
  2. Removed the types on the declaration of the two variables.

Overall, I find it more readable.

\$\endgroup\$
  • \$\begingroup\$ Welcome to Code Review! Good job on your first answer. \$\endgroup\$ – SirPython Feb 27 '16 at 0:45
1
\$\begingroup\$

I've been using in some cases the Akka Throttling module and seems to be a very good solution when you want to limit some resources. I think you don't have to implement a custom directive for that, think on your code and try to adapt a throttling to control that.

http://doc.akka.io/docs/akka/2.4.1/contrib/throttle.html

EDIT

Well, I would use the throttle in an actor instead.

eg:

import spray.routing.SimpleRoutingApp

object Main extends App with SimpleRoutingApp {

  implicit val system = ActorSystem("my-system")
  val myActor = system.actorOf(Props[MyActor], name = "my-actor")
  val throttler = system.actorOf(Props(classOf[TimerBasedThrottler], 3 msgsPer 1.second))
  throttler ! SetTarget(Some(myActor))

  startServer(interface = "localhost", port = 8080) {
    path("hello") {
      get {
        complete(throttler ! "Hello World!!!)
      }
    }
  }
}
\$\endgroup\$
  • 1
    \$\begingroup\$ A clear example with the code of OP would go a long way here. At Code Review, we want to help OP get better and write better code. While suggestion are valid answer, a clear example would help op better. \$\endgroup\$ – Marc-Andre Mar 1 '16 at 13:36
  • 1
    \$\begingroup\$ ah cool @Marc-Andre I will edit that ASAP providing some example using the throttling. Thanks \$\endgroup\$ – Thiago Pereira Mar 1 '16 at 13:38
  • \$\begingroup\$ I like this idea, but I don't know if it is powerful enough for what I'm trying to do. I wrote the directive because I want to have different limitations for different resources (or even no limitations at all). Furthermore It has to be limited per IP-Address (or other extractions from the context). I know that spray/akka-http is based on akka actors but don't know how to instrument these actors. \$\endgroup\$ – TobiP Mar 1 '16 at 19:49

Your Answer

By clicking “Post Your Answer”, you agree to our terms of service, privacy policy and cookie policy

Not the answer you're looking for? Browse other questions tagged or ask your own question.