dd-trace-js
dd-trace-js copied to clipboard
[http] Resource name for http should also contain path (not only method)
It is of no use to have resources grouped by http method because it does not provide any meaningfull measure of latency or errors when very different end services could be grouped together behind the same http method.
Proposal is for it to concatenate path in the same fashion as express.request does it.
https://github.com/DataDog/dd-trace-js/blob/17df4ca0f796bf9379d6203d70dd40cf0b7beed8/packages/datadog-plugin-http/src/client.js#L43
Before:

After (example):

The main issue is that for Express, there is a fixed set of routes that are matched and grouping is done using those. The same thing doesn't exist for HTTP clients because the routes are defined on the server of the remote endpoint and not in the client.
However, if you want to differentiate between endpoints and not necessarily by individual route, you can always split the remote endpoints by domain with tracer.use('http', { splitByDomain: true }) which will create a different service for each domain.
Having multiple services is terrible and clearly does not help the situation of having centralized meaningful metrics about your downstream dependencies and how those are behaving.
This seems to work out of the box for Java services, it would be a really useful feature to have
We ended up using a hook internally to overwrite the resource name with the the host of the outgoing request. For our workloads our services call out to 10s or 100s of services and including each and every path would have been unwieldy. Using the hook an app can set the resource_name to whatever is best for the use case you're trying to solve.
Here's how we customized the behavior for us:
tracer.use('http', {
hooks: {
request: (span, req, res) => {
if (req instanceof ClientRequest) {
span?.setTag('resource.name', `${req.method} ${req.host}`);
}
}
}
});
@gaston-haro can you confirm if this is still an issue and if @vanstinator's workaround worked for you or not?
I certainly have no idea if this is still an issue since I don't know if the code has changed since I reported it. A workaround is not a solution of course, can be a temporary one yes.