Skip to content
This repository has been archived by the owner. It is now read-only.

We shouldn't log metrics events for unknown routes #2423

Closed
rfk opened this issue May 7, 2018 · 3 comments
Closed

We shouldn't log metrics events for unknown routes #2423

rfk opened this issue May 7, 2018 · 3 comments
Assignees

Comments

@rfk
Copy link
Member

@rfk rfk commented May 7, 2018

From a recent discussion on dev-fxacct, a server misconfiguration lead to some requests for unknown URLs to the auth-server, and the following log output:

fxa-auth-server.DEBUG: server.onRequest
fxa-auth-server.INFO: request.summary {"op":"request.summary","code":404,"errno":999,"rid":"1525718907949:linux-box:7129:jgtxgn8h:10055","path":"/bundle-ba35c2d66b86a6485c3243f4ca7d121be2c892c9/app.bundle.de.js","agent":"Mozilla/5.0 (X11; Linux x86_64; rv:59.0) Gecko/20100101 Firefox/59.0","t":1,"uid":"00"}
fxa-auth-server.ERROR: metricsEvents.emitFlowEvent {"op":"metricsEvents.emitFlowEvent","event":"route./bundle-ba35c2d66b86a6485c3243f4ca7d121be2c892c9/app.bundle.de.js.404.999","missingFlowId":true}

It seems weird that we're trying to log a flow event on a completely non-existent route. I wonder if the filtering list here:

https://github.com/mozilla/fxa-auth-server/blob/master/lib/metrics/events.js#L52

Should be an include-list, rather than an ignore-list.

@philbooth
Copy link
Contributor

@philbooth philbooth commented May 7, 2018

Should be an include-list, rather than an ignore-list.

Ha, actually it was opt-in until very recently, I changed it to opt-out in #2373. No probs, I can change it back! 😄

@philbooth
Copy link
Contributor

@philbooth philbooth commented May 8, 2018

The other thing we could do is just not emit the event whenever the response status is 404.

@vbudhram
Copy link
Contributor

@vbudhram vbudhram commented May 8, 2018

The other thing we could do is just not emit the event whenever the response status is 404.

I think that is fair. The original bug was found because I forgot to add a route explicitly to log metrics.

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

Successfully merging a pull request may close this issue.

None yet
4 participants