Activity

1. cacoco commented on Mar 3, 2021

   

   cacoco

   on Mar 3, 2021

   Contributor

   More actions

   Possibly related to twitter/finatra#271.
2. hamdiallam commented on Mar 3, 2021

   

   hamdiallam

   on Mar 3, 2021

   Contributor

   More actions

   The TraceInitializerFilter is responsible for initializing the trace id for the request to be handled. This is placed low in the stack before any other modules that might emit traces.

   Ideally this feature would be handled in an opinionated library like Finatra where there's robust per-route configuration. However, you can still do what you want by creating a module placed right above the trace initializer module in the http server stack. The module would apply a filter that unsets the sampling decision under some condition, like the request path. This isn't the best solution but would get you this behavior until thinking through how this would be integrated more naturally into finagle/finatra.

   👍React with 👍1Reacted by Niko Dziemba
3. dziemba commented on Mar 4, 2021

   

   dziemba

   on Mar 4, 2021

   Last edited by dziemba

   Author

   More actions

   Thanks for the pointers!

   I believe I got it working now like this:

   class DisableTracingForSpecificRequests[Req <: Request, Rep](shouldTraceRequest: Request => Boolean)
     extends Stack.Module0[ServiceFactory[Req, Rep]] {
     val role: Stack.Role = Stack.Role("DisableTracingForSpecificRequests")
     val description: String = "Disable tracing sampling for specific requests, based on the input function"
   
     def make(next: ServiceFactory[Req, Rep]): ServiceFactory[Req, Rep] = {
       val traceDisabler = Filter.mk[Req, Rep, Req, Rep] { (req, svc) =>
         if (shouldTraceRequest(req)) svc(req)
         else {
           val newId = Trace.id.copy(_sampled = Some(false))
           Trace.letId(newId) {
             svc(req)
           }
         }
       }
   
       traceDisabler.andThen(next)
     }
   }
   
   
   def shouldTrace(req: Request): Boolean = {
     req.path != "/-/health"
   }
   
   Http.server
     .withStack(_.insertAfter(TraceInitializerFilter.role, new DisableTracingForSpecificRequests[Request, Response](shouldTrace)))
     [...]
   

   That's good enough for me for now. I think it would still be nice if it was simpler than this but I see that it's not very easy to integrate that into the existing architecture.
4. hamdiallam

   added

   Design

   on Mar 4, 2021
5. hamdiallam commented on Mar 4, 2021

   

   hamdiallam

   on Mar 4, 2021

   Contributor

   More actions

   That implementation looks correct. Glad it's working. I'll keep this issue open to find some time to design better integration

   👍React with 👍1Reacted by Niko Dziemba