Skip to content

Make README example less pseudo-y #69

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

Draft
wants to merge 2 commits into
base: main
Choose a base branch
from
Draft
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
129 changes: 94 additions & 35 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -29,35 +29,110 @@ It must be configured before defining routes and other plugins in order to cover
- `onResponse`
- `onError`
- Instruments automatically custom 404 Not Found handler
- TODO: Clarify server shutdown/cleanup around otel metrics/traces

Example:
## Example
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

all this code could be moved to an example.js file instead of the readme?

it seems too much here to me (and we can't test it too)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree. Will add in a runnable example in.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I reduced the scope of the example in the README. I will still set up an external example folder.


```js
// ... in your OTEL setup
const FastifyOtelInstrumentation = require('@fastify/otel');
// otel.js
// Sets up metrics, tracing, HTTP & Node runtime instrumentation, and host metrics.
// Uses ConsoleSpanExporter for local debugging of spans.
const { NodeSDK } = require('@opentelemetry/sdk-node')
const { ConsoleSpanExporter } = require('@opentelemetry/sdk-trace-node')
const { SimpleSpanProcessor } = require('@opentelemetry/sdk-trace-base')
const { PrometheusExporter } = require('@opentelemetry/exporter-prometheus')
const { HttpInstrumentation } = require('@opentelemetry/instrumentation-http')
const { RuntimeNodeInstrumentation } = require('@opentelemetry/instrumentation-runtime-node')
const { HostMetrics } = require('@opentelemetry/host-metrics')
const { FastifyOtelInstrumentation } = require('@fastify/otel')
const { metrics, trace } = require('@opentelemetry/api')
const { resourceFromAttributes } = require('@opentelemetry/resources')
const { SemanticResourceAttributes } = require('@opentelemetry/semantic-conventions')

// Console exporter for development-time span inspection
// We could use '@opentelemetry/exporter-trace-otlp-http' instead
const traceExporter = new ConsoleSpanExporter()
const spanProcessor = new SimpleSpanProcessor(traceExporter)

const prometheusExporter = new PrometheusExporter({ port: 9091 })

const sdk = new NodeSDK({
resource: resourceFromAttributes({
[SemanticResourceAttributes.SERVICE_NAME]: 'my-service-name',
}),
spanProcessor,
metricReader: prometheusExporter,
instrumentations: [
new HttpInstrumentation(),
new RuntimeNodeInstrumentation(),
],
})

await sdk.start()

// If serverName is not provided, it will fallback to OTEL_SERVICE_NAME
// as per https://opentelemetry.io/docs/languages/sdk-configuration/general/.
const fastifyOtelInstrumentation = new FastifyOtelInstrumentation({ servername: '<yourCustomApplicationName>' });
fastifyOtelInstrumentation.setTracerProvider(provider)
const fastifyOtelInstrumentation = new FastifyOtelInstrumentation({
registerOnInitialization: true,
})
fastifyOtelInstrumentation.setTracerProvider(trace.getTracerProvider())

module.exports = { fastifyOtelInstrumentation }
new HostMetrics({ meterProvider: metrics.getMeterProvider() }).start()

// ... in your Fastify definition
const { fastifyOtelInstrumentation } = require('./otel.js');
const Fastify = require('fastify');
module.exports = { sdk, fastifyOtelInstrumentation }
```

const app = fastify();
If `registerOnInitialization=true`, use a loader to load your otel.js before everything else.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd set this up as an alternative; often instrumentation is just imported manually first thing when booting up a service

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The reason why the the loader flags are appropriate as the default suggestion is because fastify-cli. Rather than directly launching some entry point with node where you control load order, many fastify apps will just have an app.js that fastify-cli loads. In order to guarantee the otel instruments are the first thing that loads, you must load them in with the loader/reqiuire flag otherwise the fastify-cli might load in a module before the otel instruments can intercept it.

Additionally the otel tutorials and docs recommend the loader flag approach. I found that working through those docs, and then hopping over to this module and seeing two different approaches super confusing.

https://opentelemetry.io/docs/languages/js/getting-started/nodejs/
https://github.com/open-telemetry/opentelemetry-js/blob/main/doc/esm-support.md

(Speaking of loader flags, I need to add the experimental loader note for esm)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fair enough; I'd just add a note that import is also an alternative


[Fastify-cli](https://github.com/fastify/fastify-cli):

- `fastify start --import otel.js` for esm
- `fastify start --require otel.js` for cjs

Node.js:

- `node --import ./otel.js ./app.js` for esm
- `node --require ./otel.js ./app.js` for cjs

```js
// app.js
import Fastify from 'fastify';

// Because we used a loader flag and registerOnInitialization=true
// All routes will automatically be insturmented
const app = await Fastify();

app.get('/', async () => 'hello world');

app.get(
'/healthcheck',
{ config: { otel: false } }, // skip tracing/metrics for this route
async () => 'Up!'
);

// example of encapsulated context with its own instrumentation
app.register(async (instance) => {
// will inherit global FastifyOtelInstrumentation because we registered with
// registerOnInitialization
instance.get('/', async () => 'nested hello world');
}, { prefix: '/nested' });

await app.listen({ port: 3000 });
console.log('⚡ Fastify listening on http://localhost:3000');
````

### Manual plugin registration

If `registerOnInitialization=false`, you must register the fastify plugin before defining any routes.

```js
import Fastify from 'fastify';
import { fastifyOtelInstrumentation } from './otel.js';

const app = await Fastify();
// It is necessary to await for its register as it requires to be able
// to intercept all route definitions
await app.register(fastifyOtelInstrumentation.plugin());

// automatically all your routes will be instrumented
app.get('/', () => 'hello world')
// as well as your instance level hooks.
app.addHook('onError', () => /* do something */)
// Manually skip telemetry for a specific route
app.get('/healthcheck', { config: { otel: false } }, () => 'Up!')
app.get('/', async () => 'hello world');

// you can also scope your instrumentation to only be enabled on a sub context
// of your application
Expand All @@ -69,23 +144,7 @@ app.register((instance, opts, done) => {

done()
}, { prefix: '/nested' })
```

### Automatic plugin registration

The plugin can be automatically registered with `registerOnInitialization` option set to `true`.
In this case, it is necessary to await fastify instance.

```js
// ... in your OTEL setup
const fastifyOtelInstrumentation = new FastifyOtelInstrumentation({
registerOnInitialization: true,
});

// ... in your Fastify definition
const Fastify = require('fastify');
const app = await fastify();
```
````

> **Notes**:
>
Expand Down