Skip to content
This repository has been archived by the owner on Dec 10, 2018. It is now read-only.

No metrics sent when just using middlewares #10

Open
klausbayrhammer opened this issue Jun 16, 2016 · 17 comments
Open

No metrics sent when just using middlewares #10

klausbayrhammer opened this issue Jun 16, 2016 · 17 comments

Comments

@klausbayrhammer
Copy link

We observed that connect-datadog does not send any metrics when it is added to an express middleware-stack where there is no router to match routes.

For this sample webapp I would expect connect-datadog to send metrics everytime a static resource is requested

const express = require('express');
var app = express();

app.use(require("connect-datadog")({}));
app.use(express.static('.'));

app.listen(3000);

But since there is a check whether the req.route.path is set (lib/index.js) nothing is sent.

@briangonzalez
Copy link

Any word on this one?

@briangonzalez
Copy link

FYI – we're trying to use this middleware to monitor the response times of an ember-fastboot server.

@klausbayrhammer
Copy link
Author

We've worked around this issue by adding a middleware which just calls next

    router.all('*', (req, res, next) => next());

@lalarsson
Copy link

lalarsson commented Sep 14, 2016

The workaround above didn't work for me.
I'm using it as follows

import connect_datadog from 'connect-datadog';
app.use(connect_datadog({
            method: true,
            response_code: true,
            path: true
        }));

app.all('*', (req, res, next) => next());

I'm running the DataDog agent directly on the host, changed bind_host to 0.0.0.0 and changed non_local_traffic to "yes".

I'm a bit clueless to what I'm doing wrong.

@lalarsson
Copy link

I found out that the node-dogstatsd sets the host to "localhost" which made it not work for me. I'll open a PR with a solution for this.

@raoulus
Copy link

raoulus commented Nov 22, 2017

Hi guys,
I am also facing this issue in combination with swagger-express-middleware. By using this middleware there's also no req.route available and therefore no metrics are sent because of this condition

if (!req.route || !req.route.path) {
  return;
}

A simple fix would to fallback to req.path, like

const path = (req.route && req.route.path) || req.path
if (!path) {
  return;
}

what are your thoughts?

This project doesn't seem to actively maintained anymore.

@respectTheCode is there a chance to get this done when I submit a PR?

Best

@ljbade
Copy link

ljbade commented Nov 30, 2017

@raoulus I would love to see this fixed, I ended up having to use the workaround from #10 (comment)

A bit disappointing since this is the package Datadog recommend.

I think the fallback to path is sensible, currently all my route tags are empty.

@ljbade
Copy link

ljbade commented Nov 30, 2017

It is also worth noting that https://github.com/brightcove/hot-shots seems to be a more updated version of node-dogstatsd

@raoulus
Copy link

raoulus commented Nov 30, 2017

@ljbade there are a couple of forks, which have this issue fixed. For instance, https://github.com/wiivv/node-connect-datadog

The suggested workaround is a hack. The path is set *, which is not the desired output I need...

@respectTheCode
Copy link
Member

We stopped using datadog years ago when they changed their pricing model. If someone wants to maintain the module I would be happy to transfer it.

@jeremy-lq
Copy link

@respectTheCode, thanks for not only maintaining this previously, but offering to transfer it so those using it can continue getting support. If the offer still stands, we're @DataDog on GitHub and npm, and will maintain moving forward.

@respectTheCode
Copy link
Member

npm isn't letting me transfer it for some reason. Someone will need to contact npm support to initiate the transfer.

@jeremy-lq
Copy link

I've opened ticket 27882 with npm support.

@jeremy-lq
Copy link

@respectTheCode, npm support has indicated that the request needs to come from you. You can reference the above ticket number for context. Thanks again.

@jeremy-lq
Copy link

@respectTheCode, just wanted to touch base on this. npm support has indicated that the request needs to come from you. You can reference the above ticket number for context. Thanks again.

@Ianfeather
Copy link

@respectTheCode @jeremy-lq Do you think there's any chance you could conclude the handover? It would be great to use a maintained version of this package if possible. Appreciate your work on this though.

@jeremy-lq
Copy link

@Ianfeather working on an update to this now. Will post a further update ASAP.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

8 participants