Where communities thrive


  • Join over 1.5M+ people
  • Join over 100K+ communities
  • Free without limits
  • Create your own community
People
Repo info
Activity
Christopher Coco
@cacoco
You should not use them in the context of tracing normally.
I think you're overthinking them, they are a diff APi
Generally, theres not much you need to code for request tracing to work properly. If you're staring at the internals you're going to convince yourself that every method is something you should call and the normal case here that isn't true.
Christopher Coco
@cacoco
The let function is for setting something within the scope of a closure and it is a common pattern for things carried in locals (that is the LocalContext) to be able to scope s closure to a value, either for testing (very common usage) or something more esoteric where you're trying not to pick up the value carried in the LocalContext.
Ben Spencer
@dangerousben
@cacoco: (can't remember if you were involved in the original discussion, so apologies if I'm repeating myself) my use case is that I am launching a background task on a FuturePool - this background task might outlive the original request so I figured it should be a different span: does that make sense, and if so should I be using letId, traceLocalFuture, or something else?
Alessandro Vermeulen
@spockz
@dangerousben I’m not sure about zipkin. But there are trace systems where you can have “a sync” triggers. So they all share the same trace id. But a span is marked as an event rather than a RPC call. I would like to see what originally triggered the background task that is now going haywire for example
Christopher Coco
@cacoco
@dangerousben neither. I thought we covered that the trace will correctly cross the future pool. Again, the API is meant to be transparent to you.
And if it’s not clear ‘traceLocalFuture’ is a different tracing API so the answer if you wanted to scope work in a closure is letId. But you should have no need to do this.
I’m confused why there’s a feeling you need to do something special here. And why the fixation on the local tracing API
Ben Spencer
@dangerousben
@cacoco: I feel like for reporting purposes the background task is a different 'thing' so it would be useful to report it with a different span ID (although I can link it with the original request via the shared trace ID and the parent ID)
there's no fixation about how I achieve that - somebody pointed me in the direction of letId and in reading related code I discovered these traceLocal methods and was curious about the use case
but if there's another way I should be wrapping this code in a fresh span I have no particular attachment to them
Nicolas Rinaudo
@nrinaudo
I'm upgrading our backend to scala 2.13, and just realised that finagle-http-auth wasn't ported. Has this been maybe merged into another finagle project?
jyanJing
@jyanJing
Hi @nrinaudo all finagle targets should be on 2.13, I did not find the finagle-http-auth within finagle, would you mind pointing me to the github link of that target?
Nicolas Rinaudo
@nrinaudo
I already submitted a merge request to upgrade (and fix a bug)
jyanJing
@jyanJing
Thank you @nrinaudo , we will take a look
Nicolas Rinaudo
@nrinaudo
Thank you, much appreciated
Christopher Coco
@cacoco
@nrinaudo just for clarity, projects in github.com/finagle are not maintained by Twitter (we maintain the organization and the blog — not the individual projects here).
You’ll likely want to find the support channel for that project (this channel is for Twitter’s finagle and related Twitter projects)
Vladimir Kostyukov
@vkostyukov
@nrinaudo thanks for the PR! I merged and will cut a release today.
Nicolas Rinaudo
@nrinaudo
@cacoco ah, yes, that clarifies things. I assumed anything under /finagle was twitter maintained. Thank you
@vkostyukov my pleasure!
Now to hunt down the author of finagle-prometheus and I should be good to go...
Christopher Coco
@cacoco
They may be lurking here too...
Nicolas Rinaudo
@nrinaudo
Ah, so they are! Probably best to give them time before I hound them though, it’s only been a few hours
Alessandro Vermeulen
@spockz
Yeah
@samstarling I think there is nothing blocking for releasing for 2.13 right?
Nicolas Rinaudo
@nrinaudo
Oh that’s already released, but I submitted a PR to upgrade to the latest finagle version - there are some bincompat issues
Nicolas Rinaudo
@nrinaudo
@vkostyukov is there anything I can do to help with the 0.2.0 release?
Hamdi Allam
@hamdiallam
@dangerousben to close the loop on traceLocalFuture and traceFuture. As @cacoco mentioned, it's not well-documented api atm so I also wouldn't suggest using it for now. The use case for them are convenience methods that set things like the rpc name of a new trace for you & adds the "lc" binary annotation along with the local begin/end annotations around your code to indicate that a local computation is occurring as typically a new span is indicative of an RPC.
Ben Spencer
@dangerousben
@hamdiallam ok, thanks
Dermot Haughey
@hderms
anyone experience something with Finagle where running the server in SBT, killing it and then trying to run it again gives you a port already in use error?
would explicitly closing the server solve this problem?
Ryan Plessner
@rpless
@hderms I've seen this in sbt's interactive mode starting with sbt 1.3.x. if you fork the run operation with fork in run := true it seems to solve it
Alessandro Vermeulen
@spockz
@rpless I suppose that is because the process doesn’t quit so the socket handles aren’t cleaned up automatically so closing the server should work.
Ryan Plessner
@rpless
yea I see what you are seeing. I suspect that's why the 2 toy apps I have don't seem to work as expected without forking the process. I tried both calling close in a sys.addShutdownHook and putting the listening service into a Cats Effect Resource inside in IOApp. in both cases without forking the run process the port would not be released. I suspect there are other ways to do it though
Dermot Haughey
@hderms
@rpless yeah I did the exact same thing and couldn't get it to work as expected
rajeshkumarchandolu
@rajeshkumarchandolu
anyone know how to add the context path in the finagle httpservice
Ben Spencer
@dangerousben
if I consume a Reader as described in the docs (https://twitter.github.io/util/docs/com/twitter/io/Reader.html), am I right in thinking that the code in the flatMap will be run on whatever thread completed the promise, which in the case of a streaming http request is probably a netty IO thread, so if I want to do anything blocking or cpu-bound, I should shift it off onto a FuturePool each time?
8 replies
Ben Spencer
@dangerousben
does thrift support streaming data? google suggests not but I thought I'd double-check in case there's anything finagle-specific I can do to avoid having to add a separate http (or something) interface to an existing thrift service
2 replies
Dermot Haughey
@hderms
so if I have a finagle admission control filter and a request is not admitted, is it fair to assume it will show up as 503 somewhere? Currently that's what I experience when I run a load test against a server with admission control on. The documentation implies that there's some conditionality w.r.t. how it responds based on the protocol in use so I'm just trying to make sure I am understanding this correctly
1 reply
Alessandro Vermeulen
@spockz
Interesting. Whenever I configure our http client with a SSLContext directly tls resumption works. If I use SslClientConfiguration it does t work anymore. I don’t even see the “Client cached” debug log statements so no resumption is attempted at all
Has anyone else experienced this issue?
Alessandro Vermeulen
@spockz
@ryanoneill does this behaviour ring a bell? Gut feeling is that a ssl engine or context is not being reused. I’ll Port the reproducer from our code to a mr on finagle tomorrow so I can put it in an issue
Ryan O'Neill
@ryanoneill

@spockz Yes. By default it's not. What's available in OSS Finagle is also a little different than what we use internally. If you look at the apply method of the Netty4ClientEngineFactory you can see where it recreates an SSLContext each time. https://github.com/twitter/finagle/blob/develop/finagle-netty4/src/main/scala/com/twitter/finagle/netty4/ssl/client/Netty4ClientEngineFactory.scala

That's far less than ideal, but it's also the safest.

We use ones internally that don't recreate the contexts each time, but they are very tied to how our security infrastructure works.
We also aren't particularly concerned with resumption, as Finagle is mostly used for service to service communication and connections are generally long-lived. When you're reconnecting, it's almost always because a service has redeployed.
We rewrote all of this though when we moved to using it more internally