-
Notifications
You must be signed in to change notification settings - Fork 375
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
We had a use case for Datadog::Tracing::Contrib::HTTP::Instrumentation.after_request #3736
Comments
Apparently this use case of ours isn't too surprising. While searching for
So yeah. All I can do is say that the use case for which the feature apparently was built initially still exists 🤷 Maybe you can support it in a simpler/more generic way (see my suggestion above). |
Hi @NobodysNightmare! Thanks for highlighting. This is a challenging feature, less so from a mechanical perspective, but from a change management one, as we try to balance user customization with other functionality out-of-the-box. From what I remember, I deliberately introduced this feature as undocumented, because while I recognized its pragmatism for modifying resource, I wasn't sure of the side effects of arbitrary execution (e.g. slower HTTP responses when doing heavy work within Also, as you alluded to, this is chiefly used for modifying I don't have any concrete solutions to suggest yet, but I do think our team should consider what a solution should look like, and how it would practically improve the experience with Off the cuff, maybe this could be:
Hopefully we can get some kind of workaround in place short term. Maybe the @DataDog/ruby-guild would like to weigh in on ideas for this, and the long term solution. |
Hi, thanks for the open sharing of your thoughts. I share the doubts you have on two of those ideas, especially the ability to automagically guess a better resource name. Option 3 sounds promising, but also like it might be a "longshot" option because of its complexity. Pragmatically, I will probably tell people at our company to wait a few months before going to the 2.x line of ddtrace. Eventually we will be forced to migrate over anyways because nobody wants to run with too old dependencies, but maybe this buys us enough time for an alternative solution. |
@delner Wondering if there's been any more movement on this one. There are two big use cases we have:
In both cases, having a hook into the instrumentation for faraday/http here to override the service and/or resource name would give us the flexibility to do this on our side and vastly improve our observability out of the box. Any other thoughts here? Would you accept a PR that gives the client the ability to configure a custom service and resource (by implementing a block that yields maybe something related to the faraday client / the request / the span)? |
Hey folks! FYI a few folks are out for vacation and whatnot, so we may take a bit to reply on this one, hope that's ok! |
Hey @ivoanjo wanted to bump this 🙏🏼 Would be awesome to be able to override the
|
We're still looking into this, hopefully will have an answer soon! 😅 Update: Still no decision on this one! Thanks for the patience folks :/ |
This is in response to #3367:
Describe the goal of the feature
We were looking for a good way to consistently (i.e. without manual repetition) set the resource name of HTTP requests. This is what we did through the
after_request
hook. Snippet:It's worth noting that
HTTPFestival::TraceCalls.current_caller
goes back to a thread-local variable, so we ensure that for each HTTP request that might concurrently occur, we set a proper request name. We had a mechanism in place to automatically set a reasonable default for resource names just before we issued the actual HTTP request.Describe alternatives you've considered
I am not aware of any alternative that could be used to overwrite the resource name of HTTP calls. This leaves us with unhelpful defaults such as
GET 200
orPOST 404
, when our implementation allowed us to construct names that were specific to the action being executed, without being derailed by details like identifiers in the request path.Judging from the implementation that we had in place before, something that would alternatively work for us is an API like this:
The text was updated successfully, but these errors were encountered: