HTTP Client resource.name should show method + path

[bullywhippet]

Describe the bug
Traces for http-client metrics only show method name, not path so all endpoints are lumped together.

We are unable to create monitors on http-client resource_names using the automatic instrumentation because only the method is tagged on traces and we need to filter at a more granular level (per resource_name)
Example image:

Expected behavior is that the trace shows the method and the path using automatic configuration configuration, like the Java agent does:
Example image:

I believe that this may be the line that needs to be updated to include a path:
https://github.com/DataDog/dd-trace-js/blob/master/packages/datadog-plugin-http/src/client.js#L62
and https://github.com/DataDog/dd-trace-js/blob/master/packages/datadog-plugin-http2/src/client.js#L177

Environment

• Operation system:
• Node version:
  8.11.3 and 14.5
• Tracer version:
  dd-trace": "^0.20.0" and "dd-trace": "0.24.0"
• Agent version:
  7.20.0 and 7.23

[rochdev]

Had a discussion with our Java team and it looks like they do pattern matching to try to combine similar URLs into groups for HTTP clients. This is definitely something we could implement for Node as well. It would probably be optional however given the potential for high cardinality which could hit quotas and provide a subpar experience depending on the structure of the paths.

In the meantime you could group similar paths manually using span hooks. This would likely provide better results anyway since automatic grouping can be hit and miss as it doesn't have any knowledge about the remote routes while your code might.

tracer.use('http', {
  client: {
    hooks: {
      request: (span, req, res) => {
        const group = groupPathsSomehow(req) // possibly pattern matching using RegExp
        span.setTag('resource.name', `${req.method} ${group}`)
      }
    }
  }
})

Please let me know if that helps!

[hong823]

@rochdev we currently also having the same use case as @bullywhippet. Any plan of having this natively supported for nodejs like java in the near future?

[hong823]

@rochdev I was trying to use your suggestion with custom client hooks. But I did notice now the service name was defaulted back to .....-http-client, any idea?

// Suggestion from https://github.com/DataDog/dd-trace-js/issues/1173
tracer.use('http', { service: "my-service-dev" });
tracer.use('http', {
    client: {
      hooks: {
        request: (span, req, res) => {
          const group = groupPathsSomehow(req)

          span.setTag('resource.name', `${req.method} ${group}`)
        }
      }
    }
});

In the APM debug logs, I can see that it wasn't set properly:

...
  {
    "trace_id": "<removed>",
    "span_id": "<removed>",
    "parent_id": "<removed>",
    "name": "http.request",
    "resource": "GET /api/site/preload",
    "error": 0,
    "meta": {
      "service": "my-service-dev", // This is mapped correctly 
      "version": "<removed>",
      "span.kind": "client",
      "http.method": "GET",
      "http.url": "<removed>/api/site/preload",
      "http.status_code": "200"
    },
    "metrics": {
      "_sample_rate": 1,
      "_sampling_priority_v1": 1
    },
    "start": 1623951811246268400,
    "duration": 66464355,
    "service": "my-service-dev-http-client", // This is not using the "service" name "my-service-dev"
    "type": "http"
  },
...

I tried switching the order of tracer.use(...) and noticed it's always the last one that got recognized.

[rochdev]

@hong823 There should be only a single tracer.use() with both option set in it. Multiple uses of tracer.use() are not supported right now so otherwise the second block wins.

So basically:

tracer.use('http', {
  client: {
    service: "my-service-dev",
    hooks: {
      request: (span, req, res) => {
        const group = groupPathsSomehow(req)

        span.setTag('resource.name', `${req.method} ${group}`)
      }
    }
  }
});

[hong823]

@rochdev thanks, I can now see traces added with proper resource_name in the flame graph.

resource_name=GET /my-request/path

However, I did notice metric trace.http.request doesn't have resource_name=GET /my-request/path retained there somehow.

Do you have any idea?

[rochdev]

@hong823 What version of the tracer are you using? We only added support for measuring HTTP spans with a custom service name recently, so it's possible that by changing the service name stats are no longer calculated properly. If you are not on the latest version, definitely try to update first. I'm also not sure if all stats are still calculated or only a subset when the service name is changed, so if you are using latest I would recommend contacting support who would be able to look into this further.

[hong823]

@rochdev upgrading from 0.33.2 to 0.35.0 seems to do the tricks, thanks for the tips!

[hong823]

@rochdev I just noticed express modules is also missing path from resource_name, can you confirm that it is not supported and we should do the same workaround using hooks?

[rochdev]

@hong823 This is supported out of the box and should be using the routes from the calls to app.get('some-route', ...). Do you use a custom router or Express built-in one?

[hong823]

@rochdev we're using the router as a wildcard like below:

const server = express();
server.post('*', express.json(), (req, res) => {
   ...
});

I was able to use the hooks approach similar to http.request and it worked the same now.

[YogeshTelawane]

@rochdev Sorry for tagging you directly.
I've used the hook to set the resource.name.
It is working fine in local environment and I'm able to set the resource name.
But after deploying it to development environment the req.host is captured as "undefined".

tracer.use('http', {
  client: {
    hooks: {
      request: (span, req, res) => {
       console.log(req.host)  // undefined
        span.setTag('resource.name', `${req.host} ${req.path}`)
      }
    }
  }
});

Specs -
Node-version : 14
dd-trace : 1.7.0

Any suggestions what is wrong?

Thank you.

[rochdev]

@YogeshTelawane Are you using the same version of everything in both environments? It's possible that some versions of Express (assuming you use Express) don't set req.host. In that case, you could try using instead req.headers['host'].

[vigohe]

Any plans for this feature?? Or the only way, it's by implementing the workaround ?

[Qard]

Not currently planned, as far as I'm aware.

@rochdev should we make a proper feature request for this?

[mtorre4580]

Hi, In my current team i built this helper for normalice the request

function normalizeResource(req) {
  const { path: pathRequest, method } = req;
  const path = pathRequest ? pathRequest.split(/[?#]/)[0] : '/';
  return `${method} ${path.replace(/\/\d+(?=\/|$)/g, '/:id')}`;
}

This allows to handle correct path variables like offers/123 or offers/123/reject to offers/:id, offers/:id/reject

[anden-akkio]

I had an adjacent issue of sorts with Express. My issue here was that the express plugin wasn't correctly loading in the first place due to some weird blend of ESM and TypeScript transpilation, so the resource.name was defaulting to just the request method. The fix was to adjust the loader used to use this library's:

tsx --loader dd-trace/loader-hook.mjs src/app.ts

The order of everything is very particular here - make sure your --loader command is before your actual file.