-
Notifications
You must be signed in to change notification settings - Fork 435
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
proposal: integrate with github.com/valyala/fasthttp #700
Comments
Yes I think that would be fantastic! Would you be interested in doing the work? If yes, what's the easiest way to integrate with this library? |
Is anyone working on this project at this moment :) |
@vjfreestar I don't believe anybody is working on this currently. Are you interested in working on it? |
any update on this? I am stuck with the usage of |
@ridhoperdana This is still on our radar but is not being actively worked on. If you have questions about our use of |
I think I'm running into the same issue as @ridhoperdana. fasthttp uses it's own context-like interface (
example usage:
I believe I need to use Now... I realize that that's not the only way to do it, but I believe the alternative is to include this boilerplate span setup in every handler individually. Downstream of the handlers, in my case, only require Let me know if you need any further information. |
@knusbaum From what I see the context is used to help if someone wants to create a child span inside whatever function that used the same context, am I right? @tills13 So you modified the Right now I am just ignoring the |
no, RequestCtx already implements context.Context - if you look below here, you'll see that it implements all the methods required by context.Context. There's a test which ensures it implments context.Context. The issue to me seems that you can use a RequestCtx as a context.Context but you cannot use a context.Context as a RequestCtx (for obvious reasons) and the hierarchy seems to be RequestCtx in the upper handler / wrapper -> context.Context from StartSpanFromContext -> RequestCtx in the actual handler. Further, AFAIK there should only ever be one RequestCtx - I've kinda got something to work... though I'm not sure how many Go rules I've violated here or if my assumptions about spans hold true. The way I see it, your RequestCtx should map to exactly one span - that's the "root" span for your Go service (though it could have a remote span as a parent). I've added two helper functions
and updated my wrapper to use
Finally, I have a function that sits between by request handlers and my downstream data providers (DAOs, ElasticSearch, etc)
In my handlers, I'll call that before passing the context down.
|
Wow thanks for the detailed response, and how silly I am not knowing that the |
That's what
I'm not sure if it's always the case but it seems to me like a good idea to only really use |
Hello, have we any plans to implement this? |
We have no plans to implement this in the immediate future, however we are interested in this integration. If you would like this integration developed, there are two paths we can provide:
Until one of these things happen, this issue will be closed as stale. |
Because of current work being done a similar integration PR, we want to re-open this issue to keep fasthttp in our radar for potential near-future development. The same offer still stands if anyone wants to create a rough draft PR or a starting point. |
Proposal:
as fasthttp based server is up to 10 times faster than net/http, if we should integrate fasthttp in the framework?
The text was updated successfully, but these errors were encountered: