Introduce Trace (what endpoint was matched) [RFC]

[vkostyukov]

We've been talking quite a bit about the ways to provide users with a context of what endpoint was actually matched (see #953 for a top-level issue). This wasn't possible today.

Both of our attempts to built-in metrics/telemetry failed because of that (see #845 and #855) as we didn't have the right tools to implement it.

I've been playing with the idea of including the matched path into an EndpointResult so it could be propagated along the request/input path. This way, at the very bottom (in ToService) we can extract this matched path and do something useful with that, for example, report metrics.

This PR [RFC] introduces a new concept io.finch.Trace that represent a matched path of an endpoint and is optimized for low footprint and fast concat. Right now, we aren't doing anything useful with it in ToService as I wanted to make sure we settle on a general idea before I proceed with actually employing the new data.

Let me know what do you think! Here is a quick usage example.

scala> import io.finch._, io.finch.syntax._
import io.finch._
import io.finch.syntax._

scala> val foo = get("foo" :: "bar" :: path[String]) { s: String => Ok(s) }
foo: io.finch.Endpoint[String] = GET /foo :: bar :: :string

scala> val bar = get("bar" :: "foo" :: path[Int]) { i: Int => Ok(i) }
bar: io.finch.Endpoint[Int] = GET /bar :: foo :: :int

scala> val fooBar = foo :+: bar
fooBar: io.finch.Endpoint[String :+: Int :+: shapeless.CNil] = (GET /foo :: bar :: :string :+: GET /bar :: foo :: :int)

scala> fooBar(Input.get("/foo/bar/baz")).trace
res0: Option[io.finch.Trace] = Some(/foo/bar/:string)

scala> fooBar(Input.get("/bar/foo/10")).trace
res1: Option[io.finch.Trace] = Some(/bar/foo/:int)

[codecov-io]

Codecov Report

Merging #957 into master will increase coverage by <.01%.
The diff coverage is 87.87%.

@@            Coverage Diff             @@
##           master     #957      +/-   ##
==========================================
+ Coverage   83.24%   83.25%   +<.01%     
==========================================
  Files          49       50       +1     
  Lines         794      818      +24     
  Branches       48       48              
==========================================
+ Hits          661      681      +20     
- Misses        133      137       +4

Impacted Files	Coverage Δ
core/src/main/scala/io/finch/endpoint/cookie.scala	46.66% <100%> (ø)	⬆️
...tee/src/main/scala/io/finch/iteratee/package.scala	95.45% <100%> (+0.21%)	⬆️
core/src/main/scala/io/finch/endpoint/header.scala	75% <100%> (ø)	⬆️
...rc/main/scala/io/finch/syntax/EndpointMapper.scala	100% <100%> (ø)	⬆️
core/src/main/scala/io/finch/Endpoint.scala	76.53% <100%> (+0.74%)	⬆️
core/src/main/scala/io/finch/ToService.scala	100% <100%> (ø)	⬆️
core/src/main/scala/io/finch/endpoint/path.scala	96% <100%> (+0.76%)	⬆️
...e/src/main/scala/io/finch/endpoint/multipart.scala	72.05% <100%> (ø)	⬆️
core/src/main/scala/io/finch/endpoint/param.scala	77.14% <100%> (ø)	⬆️
core/src/main/scala/io/finch/endpoint/body.scala	65% <25%> (-1.67%)	⬇️
... and 4 more

---

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 849b6bb...371a0da. Read the comment docs.

[sergeykolbasov]

Looks awesome, I was about to land the very similar PR but with trace replaced by Endpoint.Meta where meta is just a type alias to string.

My point is that Trace looks like the very lightweight & cut version of metadata ADT and I wonder if we are able to extend its use to the API users anyhow.

[sergeykolbasov]

Maybe it makes sense to introduce lazy val metadata (or trace) in Endpoint and during the compilation phase, all ::ed and |+|ed endpoints will compose their metadata together. It means, there will be fewer allocations spent on each call for concat operation as it is now.

@vkostyukov what are your thoughts?

[sergeykolbasov]

Regarding the telemetry, I think it's enough to put resulting instance into Response.ctx and leave it up to the users (filters), and make it optional through the Bootstrap configuration.

[vkostyukov]

@sergeykolbasov Yeah, I think exporting the Trace object under a response's record schema is a reasonable thing to do at this point.

Re: Meta. I'm perfectly fine if we keep metadata's purpose as a structure defining composed endpoints (built at endpoint creation), while trace could carry the information about what endpoints were matched during this evaluation (built when an endpoint is run). I hear you, we can populate Trace with metadata (similar to how we do it with Strings) in case if users would want that information at the end. As of now, there is an immediate use-case a Trace structure is trying to address - figuring out what endpoint was matched so we can report telemetry. In my mind's eye, we should stick with it and see if people need anything more than that.

[sergeykolbasov]

what if we move this concat to private val trc = a.trc.concat(b.trc) of the endpoint to generate it only once?

[vkostyukov]

Sorry, I'm not sure this is possible. Neither a.trc nor b.trc are available until we run an endpoint (Endpoint.apply).

[sergeykolbasov]

Ah, yeah

nvm then.

[sergeykolbasov]

LGTM 👍

[rpless]

👍 from me. Regarding Response.ctx though are there issues with that if an exception gets raise and the future your service returns is failed. You don't have a response to call ctx there but you may have matched a Finch Endpoint in the underlying service. Feels weird but we could set it on the Requests context instead, that way we'd know if we matched but raised an exception in a finch endpoint.

Also @vkostyukov curious how this affects the benchmarks. Your comment about it being low footprint seems to indicate that it didn't have too much of an impact.

[vkostyukov]

@rpless - that's a really good point about the failed request. We, however, have a top-level failure handler that always catches everything and converts it into a very basic 400 response. I think we should be fine with just setting the response.ctx field.

The new structure definitely comes at the cost yet I think we're in the position to pay it. Some recent performance optimizations should make it more-or-less even. I want to run some benchmarks to see the overhead.

UPDATE: I take that back, we only handle Finch's own errors and bypass everything else.

[vkostyukov]

There is a slight (and expected) increase for path endpoints but nothing concerning:

BEFORE:

[info] MatchPathBenchmark.stringSome                                avgt   10  207.146 ±   10.804   ns/op
[info] MatchPathBenchmark.stringSome:·gc.alloc.rate.norm            avgt   10  280.000 ±    0.001    B/op

AFTER:

[info] MatchPathBenchmark.stringSome                                avgt   10  216.268 ±  57.560   ns/op
[info] MatchPathBenchmark.stringSome:·gc.alloc.rate.norm            avgt   10  304.000 ±   0.001    B/op

I'm going to merge this in and employ the new data later, in a separate PR. As usual, we can iterate on performance later (if/as needed).