[express] Add option to record HTTP headers. #266
Conversation
alloy
force-pushed
the
express-capture-headers
branch
from
69fda45
to
e4f2afd
Compare
docs/API.md
Outdated
|
|
||
| <h5 id="express-config">Configuration Options</h5> | ||
|
|
||
| | Option | Default | Description | | ||
| |------------------|---------------------------|----------------------------------------| | ||
| | service | *Service name of the app* | The service name for this integration. | | ||
| | validateStatus | `code => code < 500` | Callback function to determine if there was an error. It should take a status code as its only parameter and return `true` for success or `false` for errors. | | ||
| | recordHeaders | `undefined` | An array of headers to include in the span metadata. | |
alloy
force-pushed
the
express-capture-headers
branch
from
e4f2afd
to
e5617ce
Compare
src/plugins/express.js
Outdated
| @@ -9,6 +9,8 @@ const pathToRegExp = require('path-to-regexp') | |||
| const OPERATION_NAME = 'express.request' | |||
|
|
|||
| function createWrapMethod (tracer, config) { | |||
| const recordHeaders = config.recordHeaders ? config.recordHeaders.map(key => key.toLowerCase()) : [] | |||
There was a problem hiding this comment.
Make sure it's an array first to be safe.
There was a problem hiding this comment.
I figured that an error here, early in the process, could help setup of the plugin. What would you want to happen if this is not an array?
There was a problem hiding this comment.
The idea is to try and never crash, even if any user input is wrong. In any case where it would be interesting to know why something is not working, a log entry of the appropriate level should be created instead, which the user will be able to view in debug mode.
docs/API.md
Outdated
|
|
||
| <h5 id="express-config">Configuration Options</h5> | ||
|
|
||
| | Option | Default | Description | | ||
| |------------------|---------------------------|----------------------------------------| | ||
| | service | *Service name of the app* | The service name for this integration. | | ||
| | validateStatus | `code => code < 500` | Callback function to determine if there was an error. It should take a status code as its only parameter and return `true` for success or `false` for errors. | | ||
| | recordHeaders | `[]` | An array of headers to include in the span metadata. | |
There was a problem hiding this comment.
Me again, your favorite nitpicker :) I think simply headers would make more sense to avoid a verb since this is not a function nor a boolean value.
There was a problem hiding this comment.
Hah. I appreciate it, naming is hard 👍
|
Also don't forget to rebase on v0.6.0 branch |
|
Ohhh, I had previously thought you meant that master was in prep for v0.6.0. I see, I’ll rebase it on there instead 👍 |
alloy
force-pushed
the
express-capture-headers
branch
from
e5617ce
to
4f2558b
Compare
alloy
force-pushed
the
express-capture-headers
branch
from
4f2558b
to
a5ec769
Compare
|
Done 👍 |
alloy
force-pushed
the
express-capture-headers
branch
from
a5ec769
to
e7bf780
Compare
In our case we have some clients of our GraphQL service that do not (yet) have any DD tracing, so in order to namespace GraphQL operations to specific clients I want to record their user-agent.