-
Notifications
You must be signed in to change notification settings - Fork 19
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
Optionally obfuscate file paths #28
Comments
I really love this idea. I don't think an extra |
I'm assuming that the "mechanism for modifying output" that you're referencing is the support for formatters? I'm not so intricately familiar with the code base, so I don't want to get the wrong idea. I think it would be certainly possible to modify the output when it's actually formatted. What I'd personally shy away from is having a formatter like Putting that discussion aside temporarily, let's assume we'd be modifying the log origin globally in the code block: Lines 237 to 253 in 8f08f08
Modifying that section to look like: _getOrigin() {
Error.prepareStackTrace = arrayPrepareStackTrace
const stack = (new Error()).stack
let origin = null
let cwd = this.relativeStackTracePaths ? process.cwd() : null;
for (let i = 1; i < stack.length; i++) {
const file = stack[i].getFileName()
if (file !== __filename) {
origin = {
file: this.relativeStackTracePaths ? ('.' + path.sep + path.relative(cwd, file)) : file,
line: stack[i].getLineNumber().toString()
}
break
}
}
Error.prepareStackTrace = originalPrepareStackTrace
return origin
} I think would work. But we might be more inclined to apply a log.addPathTransform(function(location) {
return '.' + path.sep + path.relative(process.cwd(), location);
}); This discussion also begs the question of whether or not this whole idea is a hack fix to circumvent the apparent lack of a similar V8 option. We're also left with the problem of how (and indeed if) we should handle the full error stack trace. For instance:
Is the result of just logging the class Bristol {
constructor(options) {
...
this.relativeStackTracePaths = options.relativeStackTracePaths === true
if (this.relativeStackTracePaths) {
this.addTransform(function(elem) {
if (elem instanceof Error) {
let cwd = process.cwd()
let re = new RegExp('(' + cwd + '.+)(?=:[0-9]+:[0-9]+)', 'g')
return elem.stack.replace(re, function(match) {
return '.' + path.sep + path.relative(cwd, match)
})
}
return null
})
}
...
}
} (Which works in my admittedly simple manual tests for the sake of discussion.) |
I'd also like the ability to use relative paths, or omit paths altogether from the output. For example, it's not that relevant where a startup message like It's easy enough to add a transform to remove the path: logger.addTransform(e => {
if (e.file && e.line)
e.file = e.file.match(/([^/\\]*?$)/)[1];
return e;
});
logger.info('foo'); // [2018-07-12 16:00:31] INFO: up and running (test.mjs:28) I can't figure out how to omit paths completely though (#53). I've tried passing |
I'm sure you're interested in this change being made to the default Bristol formatter, but you could also roll your own. For instance, my formatter allows trimming paths. You could write your own to remove paths completely. Just food for thought. Good luck. |
@TomFrost Love Bristol, been using it for a long time.
I recently came across a need to obfuscate my file paths. I found that this was relevant for live demos both for the sake of security and clarity and that it would be helpful in certain production deployments to reduce the logging payload when I already know the executable path.
I'd like to see an option on the Bristol constructor to support relative pathing from
process.cwd()
. Using this setting the logged stack trace path would be limited to the folders in a given project directory assuming that it was the current working directory when the process was started.Would end up using:
I wouldn't worry about handling the cases of starting a node process from a different directory.
I've modified a version of Bristol and can PR in the fix if you think it would be appropriate/useful. With respect to performance, this would add a single
if
statement in every log event unless you wanted to maintain two path retrieval functions and dynamically set them on theBristol
instance.I have not tested how this affects globally linked modules, but based on my experience with the
path
module, worst case scenario you'd end up with a bunch of../../..
relative path directives.The text was updated successfully, but these errors were encountered: