-
Notifications
You must be signed in to change notification settings - Fork 36
Optionally capture Lambda request and response payloads #222
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
Codecov Report
@@ Coverage Diff @@
## main #222 +/- ##
==========================================
- Coverage 88.06% 87.65% -0.42%
==========================================
Files 31 32 +1
Lines 1198 1231 +33
Branches 240 249 +9
==========================================
+ Hits 1055 1079 +24
- Misses 86 91 +5
- Partials 57 61 +4
Continue to review full report at Codecov.
|
src/utils/tagObject.ts
Outdated
@@ -0,0 +1,34 @@ | |||
const redactableKeys = ["authorization", "x-authorization", "password", "token"]; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe this could be customizable by the customer?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh good question - we actually already allow the customer to define obfuscation rules via datadog.yaml
https://docs.datadoghq.com/tracing/setup_overview/configure_data_security/?tab=datadogyaml#replace-rules-for-tag-filtering
This is just a hardcoded list to prevent non-configured payload capture from indexing basic/common secrets. It's not something we want to automatically index. If a user wants to capture these, they'll have to manually set a tag.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh I see, so maybe we should make a link/note about this config option when we'll release this new env var?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yup! And it'll be in the blog post :)
…datadog-lambda-js into aj/capture-request-and-response
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, left some comments.
src/utils/index.ts
Outdated
@@ -3,3 +3,4 @@ export { wrap, promisifiedHandler } from "./handler"; | |||
export { Timer } from "./timer"; | |||
export { logError, logDebug, Logger, setLogLevel, setLogger, LogLevel } from "./log"; | |||
export { get, post } from "./request"; | |||
export { tagObject } from "./tagObject"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
nit: we typically name files like this tag-object.ts
src/utils/tagObject.ts
Outdated
if (typeof obj === "string") { | ||
let parsed: string; | ||
try { | ||
parsed = JSON.parse(obj); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If it's a string, does it need to be parsed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Good question. This is needed to deeply tag nested JSON into separate items, otherwise it just tags an object (ala: setTag('request.body', { foo: { blah: 'ahhh' }})
, which doesn't display nicely in the app.
What does this PR do?
Uses span tags to recursively tag attrs of the request and response object.
datadog.yaml
obfuscationsMotivation
Testing Guidelines
Additional Notes
Types of Changes
Check all that apply
TODO