Node-ldapjs: Consider dropping dtrace-provider

[sorccu]

Hi,

I know there's already been a ticket for this, but I propose that we remove dtrace-provider from optionalDependencies, but keep the functionality. From my understanding (although please feel free to correct any misunderstandings) it's mainly useful for someone who's developing the ldapjs module itself.

Including dtrace-provider has the following disadvantages:

• It will always at least attempt to build the native extension. While the dependency is optional, any errors here look extremely bad and confusing to normal users.
• It slows down installation unnecessarily even when everything works fine.
• It breaks easily between Node.js versions.
• Because the dependency is tied to a specific version, ldapjs needs to be released whenever there's an important dtrace-provider update (like 0.12 fixes). A more lax version requirement might help here, though.
• You wouldn't normally expect an ldap module to include a native extension.

I propose that:

• We move dtrace-provider from optionalDependencies to devDependencies so that it can still be used during development.
• We keep the dtrace code.
• Whoever needs access to the dtrace probes, will have to do either npm install -g dtrace-provider or include dtrace-provider in their own package.json. They won't have to do any other changes.

Doing this would achieve:

• No (or at the very least considerably less) GitHub issues about compilation errors.
• No changes required for new Node.js versions.
• More freedom over release schedule, since dtrace-provider updates would no longer impact end users.
• Happy users (like me) :)

What do you think?

[msimerson]

:+1:

Adding a section like this in the README would be better:

DTrace

DTrace support is included in ldapjs. To enable it, you need to npm install -g dtrace-provider

[pfmooney]

The point of dtrace is debugging in production. On non-supported platforms, the errors are harmless. I have no intention of removing it from the dependencies at this time.

[sorccu]

Hopefully I don't sound like a complete ahole, and I really do get that
open source maintainers (including myself) have very limited time, but the
cold hard reality is that you're not updating the module often enough for
dtrace-provider to be actually useful. For the past year or so it has
mostly been good at creating compile errors, without any benefit whatsoever
in production or development.

If you really want to keep it, at the very least make the version range
more liberal so that users can update dtrace-provider without forking
ldapjs.

On Friday, 16 October 2015, Patrick Mooney [email protected] wrote:

The point of dtrace is debugging in production. On non-supported
platforms, the errors are harmless. I have no intention of removing it from
the dependencies at this time.

—
Reply to this email directly or view it on GitHub
https://github.com/mcavage/node-ldapjs/issues/286#issuecomment-148596188
.

[astorije]

@melloc, this seems to address the second of the 2 issues with dtrace-provider (first one being addressed by #399) and as far as I can tell, doing so would not bring any disadvantage.
@sorccu explained nicely and rather cleverly how to keep using DTrace when necessary, so this drastic removal would not even be necessary.

On non-supported platforms, the errors are harmless.

Unfortunately, as we've been able to notice in the last 1.5 years, this appears to not be true. We've had install-time error reports from our users on all major platforms :man_shrugging:

Thanks a lot for your help!

[jsumners]

Resolved in #466.