Tap Pipeline & Dump Context for Debugging #61
Conversation
Codecov Report
@@ Coverage Diff @@
## master #61 +/- ##
==========================================
+ Coverage 98.08% 98.23% +0.15%
==========================================
Files 23 25 +2
Lines 469 509 +40
Branches 73 73
==========================================
+ Hits 460 500 +40
Misses 9 9
Continue to review full report at Codecov.
|
README.md
Outdated
|
|
||
| When run in non-production, i.e. outside an OpenWhisk action, for example in `hlx up`, Pipeline Dumping is enabled. Pipeline Dumping allows developers to easily inspect the `Context` object of each step of the pipeline and can be used to debug pipeline functions and to generate realistic test cases. | ||
|
|
||
| Each stage of the pipeline processing will create a file like `$PWD/debug/context_dump_34161BE5KuR0nuFDp/context-2018-9-2-14-18-5.635-step-2.json` inside the `debug` directory. These dumps will be removed when the `node` process ends, so that after stopping `hlx up` the `debug` directory will be clean again. The `-step-n` in the filename indicates the step in the pipeline that has been logged. |
There was a problem hiding this comment.
pad days,months, and hours. hint: String.padStart(num, char)
src/defaults/html.pipe.js
Outdated
| @@ -31,8 +33,10 @@ const htmlpipe = (cont, payload, action) => { | |||
| action.logger.log('debug', 'Constructing HTML Pipeline'); | |||
| const pipe = new Pipeline(action); | |||
| pipe | |||
| .tap(dump).when(() => !production()) | |||
There was a problem hiding this comment.
we have pre, once, post, why not call it each or every ?
There was a problem hiding this comment.
tap was the name suggested in https://www.amazon.com/dp/B0777JHD5M
src/utils/dump-context.js
Outdated
| const fs = require('fs-extra'); | ||
|
|
||
| fs.mkdirpSync(path.resolve(process.cwd(), 'debug')); | ||
| const dumpdir = tmp.dir({ prefix: 'context_dump_', dir: path.resolve(process.cwd(), 'debug'), unsafeCleanup: true }).then(o => o.path); |
There was a problem hiding this comment.
why not put it in logs/debug ?
src/utils/dump-context.js
Outdated
| function dump(context, _, index) { | ||
| return Promise.resolve(dumpdir).then((dir) => { | ||
| const now = new Date(); | ||
| const dumppath = path.resolve(dir, `context-${now.getUTCFullYear()}-${now.getUTCMonth()}-${now.getUTCDay()}-${now.getUTCHours()}-${now.getUTCMinutes()}-${now.getUTCSeconds()}.${now.getUTCMilliseconds()}-step-${index}.json`); |
There was a problem hiding this comment.
- pad with 0s.
- i would make it more compact. remove
-inside date/time, eg:20180923-1434-123
src/utils/dump-context.js
Outdated
| return Promise.resolve(dumpdir).then((dir) => { | ||
| const now = new Date(); | ||
| const dumppath = path.resolve(dir, `context-${now.getUTCFullYear()}-${now.getUTCMonth()}-${now.getUTCDay()}-${now.getUTCHours()}-${now.getUTCMinutes()}-${now.getUTCSeconds()}.${now.getUTCMilliseconds()}-step-${index}.json`); | ||
| fs.writeJsonSync(dumppath, context, { spaces: 2 }); |
| */ | ||
| function production() { | ||
| // eslint-disable-next-line no-underscore-dangle | ||
| return !!process.env.__OW_ACTIVATION_ID; |
There was a problem hiding this comment.
also, what is the purpose of those dumps, if you cannot debug your deployed actions?
There was a problem hiding this comment.
Are we sure NODE_ENV is different in OpenWhisk and Petridish?
There was a problem hiding this comment.
also, what is the purpose of those dumps, if you cannot debug your deployed actions?
See #61 (diff) the tap is only used when !production, i.e. in Petridish
There was a problem hiding this comment.
According to this post (http://heidloff.net/article/how-to-create-docker-actions-openwhisk-bluemix) NODE_ENV isn't forced to production in OpenWhisk, and not reliable.
test/testHTML.js
Outdated
| assert.equal(res.headers['Surrogate-Key'], 'foobar'); | ||
|
|
||
| dump({}, {}, -1).then((dir) => { | ||
| const outdir = path.resolve(dir, '..'); |
| @@ -52,6 +52,7 @@ | |||
| "retext": "^5.0.0", | |||
| "retext-smartypants": "^3.0.1", | |||
| "snyk": "^1.88.1", | |||
| "tmp-promise": "^1.0.5", | |||
There was a problem hiding this comment.
tmp-promise is not available on the docker image. suggest to use tmp
There was a problem hiding this comment.
The docker image will pull in pipeline and its transitive dependencies, so that won't be an issue.
trieloff
dismissed
tripodsan’s stale review
All issues addressed, questions answered.
This fixes #14 and provides an alternative to #19:
when running in development-mode (i.e. no OpenWhisk in sight), every step of pipeline processing will be dumped in the
debugdirectory. This allows for very fine-grained inspection of the pipeline processing.The new
Pipeline.taphelper function allows the registration of effect-less observers or validators.