Remove dependency on Bunyan

[jsumners]

This PR removes the explicit dependency on Bunyan. It replaces the internal usage of Bunyan with a no operation logger, abstract-logging. This removes the overhead introduced by Bunyan when an external logger is not supplied. Bunyan is not a fast logger, and thus slows down the ldapjs library unnecessarily (see https://github.com/pinojs/pino#benchmarks).

The documentation is also updated with some detail on what is required of a passed in logger and points out a couple compatible loggers.

[astorije]

Hey there!
(/Cc @melloc, sorry for the spam but I noticed you made a release of this project recently)

Is there any chance this PR could be considered for review/release?
We're using node-ldapjs as part of The Lounge and the dependency to dtrace-provider is causing issues to some of our users (who do not use the LDAP integration) at install time.

Unfortunately, this is forcing us to temporarily fork this project with this PR merged at thelounge/thelounge#2021 so we can patch our project. We would love to depend on the official project instead!

Thanks!

[melloc]

@astorije Hey! Taking a look at this, I think it's a reasonable change, although I think it'd probably be better to do it without the abstract-logging package, and instead require passing in a compatible logger.

Even with this change though, this library still uses dtrace-provider, and that's unlikely to change. Looking at thelounge/thelounge#2021, it looks like you're running into nodejs/node-gyp#454, which is caused by npm's permission dropping behaviour . Would you be able to recommend that people install using --unsafe-perm? That'll make the files and installation happen actually using the root user, instead of nobody.

[jsumners]

@melloc the point of abstract-logging is to facilitate passing in a compatible logger. It merely provides the API for the library to use for logging.

[xPaw]

@melloc thelounge/thelounge#2021 merges this PR and also has a separate commit to throw away dtrace-provider used within ldapjs as well, so the issue goes away.

--unsafe-perm is a rather silly recommendation, especially when it's only needed for some logging and not core functionality of the library. And on top of that you have to google to find that specific issue since there's little to no documentation on it.

I like how that issue was closed with this reason, which I think further proves my point.

I've taken the liberty of locking this because it's turning into one of those undead issues that attract comments long after its expiry date.

[astorije]

Hey @melloc, thanks a lot for your response! 🙏

Even with this change though, this library still uses dtrace-provider, and that's unlikely to change.

Indeed, and in our unfortunate fork, we have gotten rid of it: https://github.com/thelounge/node-ldapjs/commit/1aaf3e16118c14eda631f4eadf08ed61c2077de7

May I ask why it's unlikely to change?
Honestly, dtrace-provider has caused us and our users so many problems that it's hurting this library. It's a shame, because it's the best option out there to support LDAP in a Node.js app. In this repository alone, searching issues for dtrace yields 34 results. It's not an absolute metric of course, but it's a sign IMO.

Unfortunately, until the dependency to dtrace-provider is removed, we're stuck.

Would you be able to recommend that people install using --unsafe-perm?

We are not going to be able to do that because, on top of @xPaw's comment, that would transfer the solution from the project owners (both our project and this one) to the user.
We ship an end-user project, which does not require knowledge of Node.js. Were this a library or a developer-oriented tool instead of a user-centric tool, this would be a different story.
I hope you understand :)

Anyway, thanks again for your response and for picking up LDAP.js. It would be wonderful if we can work towards a solution! ❤️

[sunpeinju]

@melloc Is it possible to merge this PR to get rid of the dependency on Bunyan? It's linkage to dtrace-provider hurts ldapjs.

In my case, I want to build an App that indirectly depends on ldapjs. All dependencies of the App is platform independent, except ldapjs. That means when I want to deploy my App to a customer's server by running npm ci to install all dependencies, dtrace-provider will be installed and require the machine to provide build environment for this native module. It's unreasonable and I can't figure out how to get rid of it. By removing Bunyan, ldapjs could be platform independent and much more friendly.

[rmullenix]

Any update on the PR? New versions of Mojave do not play nicely with dTraceBindings and are the built-in logging utility is superfluous to our needs

[jsumners]

If anyone is still following this: I am merging it into the next branch. No promises on when a release will be issued from that branch but it will eventually come.

[jsumners]

⚠️ This issue has been locked due to age. If you have encountered a recent
problem that seems to be covered by this issue, please open a new issue.

Please include a minimal reproducible example
when opening a new issue.