Skip to content

Commit

Permalink
fix(koa): ignore generator-based Koa middleware
Browse files Browse the repository at this point in the history
In Koa 2.x, which is the Koa version currently targeted by the
instrumentation, generator-based middleware from Koa 0.x and 1.x
are deprecated, and a deprecation warning is shown when these
middleware are used.

However, most of these middleware still work as intended on Koa
version 2.x, due to the use of the `koa-convert` compatibility
layer, which is applied automatically when a generator-based
middleware is used. Because of this, low-quality tutorial websites
continue to recommend the usage of generator-based middleware for
error handling.

Before this change, using a generator-based middleware alongside
the OpenTelemetry instrumentation would break the request, as the
instrumentation does not expect a generator-based middleware.
Furthermore, the replacement of the middleware with its
async-based patch would silence the deprecation warning about
the generator-based middleware.

After this change, generator-based middleware are not
instrumented, and the deprecation warning is shown. Users who wish
for their generator-based middleware to be instrumented can
manually use `koa-convert`.

In Koa 3.x, generator-based middleware will not work, with or
without instrumentation.
  • Loading branch information
unflxw committed Aug 18, 2022
1 parent bcc048b commit 1a6cfec
Show file tree
Hide file tree
Showing 3 changed files with 46 additions and 1 deletion.
4 changes: 3 additions & 1 deletion plugins/node/opentelemetry-instrumentation-koa/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,9 @@ registerInstrumentations({
});
```

See [`examples/koa`](https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/examples/koa) for a short example using both Koa and @koa/router
See [`examples/koa`](https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/examples/koa) for a short example using both Koa and @koa/router.

Note that generator-based middleware are deprecated and won't be instrumented.

### Koa Instrumentation Options

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,17 @@ export class KoaInstrumentation extends InstrumentationBase<typeof koa> {
isLayerIgnored(layerType, this._config)
)
return middlewareLayer;

if (
middlewareLayer.constructor.name === 'GeneratorFunction' ||
middlewareLayer.constructor.name === 'AsyncGeneratorFunction'
) {
api.diag.debug('ignoring generator-based Koa middleware layer');
return middlewareLayer;
}

middlewareLayer[kLayerPatched] = true;

api.diag.debug('patching Koa middleware layer');
return async (context: KoaContext, next: koa.Next) => {
const parent = api.trace.getSpan(api.context.active());
Expand Down
33 changes: 33 additions & 0 deletions plugins/node/opentelemetry-instrumentation-koa/test/koa.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -120,6 +120,10 @@ describe('Koa Instrumentation', () => {
throw new Error('I failed!');
};

const generatorMiddleware: koa.Middleware = function* (next) {
yield next
}

describe('Instrumenting @koa/router calls', () => {
it('should create a child span for middlewares', async () => {
const rootSpan = tracer.startSpan('rootSpan');
Expand Down Expand Up @@ -422,6 +426,35 @@ describe('Koa Instrumentation', () => {
);
});

it('should not instrument generator middleware functions', async () => {
const rootSpan = tracer.startSpan('rootSpan');
app.use((_ctx, next) =>
context.with(trace.setSpan(context.active(), rootSpan), next)
);

app.use(generatorMiddleware);
app.use(simpleResponse);

await context.with(
trace.setSpan(context.active(), rootSpan),
async () => {
await httpRequest.get(`http://localhost:${port}`);
rootSpan.end();
assert.deepStrictEqual(memoryExporter.getFinishedSpans().length, 2);

const simpleResponseSpan = memoryExporter
.getFinishedSpans()
.find(span => span.name.includes('simpleResponse'));
assert.notStrictEqual(simpleResponseSpan, undefined);

const exportedRootSpan = memoryExporter
.getFinishedSpans()
.find(span => span.name === 'rootSpan');
assert.notStrictEqual(exportedRootSpan, undefined);
}
);
})

it('should propagate exceptions in the middleware while marking the span with an exception', async () => {
const rootSpan = tracer.startSpan('rootSpan');
app.use((_ctx, next) =>
Expand Down

0 comments on commit 1a6cfec

Please sign in to comment.