CONTRIBUTING.md

Guidelines for contributing to trace-nodejs

RisingStack welcomes contributions for the trace-nodejs package by anyone who'd like to make it better!

To make everyone's life easier, please follow this guidelines on how you can do that.

Legal

By sending pull request you agree to grant RisingStack, Inc. perpetual, irrevocable, non-exclusive, worldwide, no-charge, royalty-free, unrestricted license in the code, algorithms, idea, object code, patch, tool, sample, graphic, specification, manual or documentation (“Your Contribution”) to reproduce, publicly display, publicly perform, sublicense, and distribute Your Contribution and such derivative works. You provide your contributions on an "as is" basis, without warranties of any kind, including warranties of correctness or fitness for a particular purpose.

Code compatibility

The library code is required to run on target platforms Node.js >=v0.10 (Node >= v0.12 for 3.x.x) without transpilation. This means the code should fully adhere to the ES5 standard: Use only ES5 compatible language features.

Most notable gotchas without completeness:

• No classes with class
• No arrow functions
• No object method shorthand
• No property assignment shorthand
• No rest parameters, no destructuring
• A lot of Array and Object methods you have in ES6 are missing from earlier versions.

When you are in doubt about the compatibility of a language specific feature check on Mozilla Developer Network!

Moreover, the code must be compatible with target Node.js APIs. As these remain mostly backward compatible, (functionality is added, seldom removed) this boils down to finding the common subset of features and in turn looking at the earliest supported version's feature set.

A few points to mention:

• Sync APIs were introduced in v0.12, v0.10 misses these.
• Streams API were largely extended in later versions.

Source code style

We use ESLint for code linting.

ESLint config

We use an ESLint configuration called Standard Style.

An incomplete list of rules:

• 2 spaces – for indentation
• Single quotes for strings – except to avoid escaping
• No unused variables – this one catches tons of bugs!
• No semicolons
• Never start a line with (, [, or ` This is the only gotcha with omitting semicolons – automatically checked for you!
• Space after keywords
• Space after function name
• Always use === instead of == – but obj == null is allowed to check null || undefined.
• Always handle the node.js err function parameter

Functional vs imperative

Code clarity is an important to us. Immutable data provides easier effect tracking therefore lesser cognitive overhead when understanding code of others. The fundamental program composition model of functional programming is based on function composition rather than state. However, keep in mind that Javascript is not a functional language. Using idiomatic language constructs even in presence of side effects enhances collaboration, and helps writing performant code. This requires you to keep a balance of imperative and functional code. The following rule of thumbs will help you:

• encapsulate and segregate state. Don't keep stateful objects all over the place, rather in reservoirs. Use private properties (starting with an _underscore) to encapsulate state that should not be accessed from outside.

• use resourceful state in a controlled manner, for example provide state management functions for native timers.

• Keep in mind that function calls in instrumentation wrapper code will be present in the stack trace if an error happens in the target library. So

• keep function call chains as shallow as possible.

Error tolerant instrumentation code

It is very important that the instrumentation hooks are tolerant to misuse. We should expect errors to be present in user code interfacing with the instrumented libraries. In this case it should be left to the instrumented library to handle the error, so that our intermediate layer remains as transparent as possible.

Code organization

.
├── CHANGELOG.md
├── CONTRIBUTING.md
├── LICENSE
├── README.md
├── circle.yml
├── coverage
├── example           There is an example app here.
├── lib               Source code goes here, along with unit tests.
├── package.json
├── scripts           Build, test, deployment scripts.
└── test              All tests except unit tests.

Keep the unit tests next to their corresponding source files. This makes it easier to navigate between them.

Guidelines about package dependencies

Keep in mind at all times that the code will run on clients' machines. Every dependency consumes some disk space, adds memory overhead (when loaded), network overhead on install. Native modules make build times longer with a non-negligible amount (if they are allowed to be built at all). Additionally every new dependency or update adds the possibility to introduce a bug, a regression or a security hole. To minimize exposure to such adverse effects, following rules must be adhered to.

Evaluation

Before adding a dependency first ask yourself:

How does this package help?

Summarize why this particular package helps solving your problem.

Is it really needed, can I achieve my goal without it? If I can what are the

trade-offs (very complex, missing knowledge in domain)?

Some packages do not simplify your problem too much. Those that add syntactic sugar for control structures, validation shortcuts, or those that try to solve a very complex general problem that you are facing may not be the best candidate for inclusion. However if it solves a specific hard mathematical or domain specific problem, eg. cryptographic or compression algorithms, protocol implementations, use it. Another particular kind is polyfills and lodash-like helpers, these are generally acceptable, just use them if you really need to.

Can I find a more mature one? Better maintained, used by more projects, etc.

This should go without further explanation.

Is there a package covering my needs with smaller footprint? Less transitive dependencies, optional native deps, etc.

Prefer modular packages, avoid kitchen sinks. For example, use lodash.find and lodash.defaults instead of depending on the whole lodash module, if you need only those two methods.

Prefer flat packages, that is, avoid ones with big dependency subtrees. Remember the leftpad scandal.

Is it a native module or does it include a native dependency? Is it optional?

To build native dependencies the native compiler build chain is required as well as some additional dependencies. Often on production systems these are not present because of security reasons. Prebuilding can be done but complicates distribution, therefore currently we do not do it. This means currently every native dependency must be optional, so the software needs to function (possibly with limited feature set and/or performance) without them.

Adding the dependency

Lock the dependency! Production dependencies must be locked in package.json. This means that semver ranges are not allowed. The reason behind this is our experience with change management in npm packages. There is a possibility that they introduce breaking changes even on patch level, and it is always harder to provide support for a range of possible versions.