feat(opentelemetry): create otel instrumentation for typed-express-router - #1044
Conversation
9c177cd to
0bff27e
Compare
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
43daf20 to
3781856
Compare
6cd264b to
11fc7f0
Compare
f804fb4 to
0158ca3
Compare
a224511 to
ab8a123
Compare
3cfad4e to
4248ffd
Compare
ericcrosson-bitgo
left a comment
ericcrosson-bitgo
left a comment
There was a problem hiding this comment.
Apparently this text box can't be empty.
| onEncodeError: (_err, _req, res) => { | ||
| res.status(500).json('Custom encode error').end(); | ||
| encodeErrorFormatter: (_err, _req) => { | ||
| return 'Custom encode error'; |
There was a problem hiding this comment.
praise: Look at all that data instead of state! 😍
| body?: any, | ||
| ): Promise<{ statusCode: number; data: string }> => { | ||
| return new Promise((resolve, reject) => { | ||
| const url = `http://localhost:${(server.address() as any).port}${path}`; |
There was a problem hiding this comment.
question: LSP says server.address() is of type string | AddressInfo | null -- how do you always know it is of type AddressInfo here? I wonder if there's a way to clean up the type cast.
There was a problem hiding this comment.
According to the documentation: https://github.com/nodejs/node/blob/main/doc/api/net.md#serveraddress
For a server listening on a pipe or Unix domain socket, the name is returned
and
server.address() returns null before the 'listening' event has been emitted or after calling server.close().
Since we're listening on a port instead of a pipe or a Unix domain socket, we can at least be sure that it'll at least be of type AddressInfo | null
IDEALLY, makeRequest should only be called when the server is listening. The server starts listening before every test (as defined in beforeEach).
So I think the best option here would just be to reject the promise if it's a string or null. (This would be an impossible case for the test)
Something like:
const address = server.address();
if (typeof address === 'string' || address === null) {
reject('Unexpected address value');
return;
}
const url = `http://localhost:${address.port}${path}`;
090eaed to
08e1817
Compare
…uter
This commit implements `opentelemetry` instrumentation for the `decode` and `sendEncoded` calls in `wrapRouter`.
Changes:
- `typed-express-wrapper`
- `@opentelemetry/api@^1.0.0` added as an optional peer dependency
- `@opentelemetry/sdk-trace-base@1.30.1` , `@opentelemetry/sdk-trace-node@1.30.1`, and `@opentelemetry/api@1.9.0` added as dev dependencies
- `wrapRouter` modified to create decode and encode spans if `@opentelemetry/api` is installed
- if `@opentelemetry/api` is not installed, spans are not created
- `onDecodeError` option removed. Please use `decodeErrorFormatter` and `getDecodeErrorStatusCode`
- `decodeErrorFormatter` takes in an array of `ValidationError`s and a `WrappedRequest`, returning a `Json` object.
- `getDecodeErrorStatusCode` takes in an array of `ValidationError`s and a `WrappedRequest`, returning a number.
- `onEncodeError` option removed. Please use `encodeErrorFormatter` and `getEncodeErrorStatusCode`
- `encodeErrorFormatter` takes in an error and a `WrappedRequest`, returning a `Json` object.
- `getEncodeErrorStatusCode` takes in an error and a `WrappedRequest`, returning a number.
- `typed-express-router` now handles the sending of the http response when there is a decode or encode error
- `express-wrapper`
- `routerForApiSpec` modified to pass in new parameters `decodeErrorFormatter`, `getDecodeErrorStatusCode`, `encodeErrorFormatter`, and `getEncodeErrorStatusCode`
- These new parameters can also be customized by modifying the props passed into the function (see `CreateRouterProps`)
BREAKING CHANGE: `onDecodeError` and `onEncodeError` have been removed. Please use `decodeErrorFormatter`, `getDecodeErrorStatusCode`, `encodeErrorFormatter`, and `getEncodeErrorStatusCode`
08e1817 to
296a92e
Compare
ericcrosson-bitgo
left a comment
ericcrosson-bitgo
left a comment
There was a problem hiding this comment.
Adds opentelemetry support for typed-express-router
Exemplary work, @starfy84!! 🚀
|
Double-checked that beta and master still point to the same commit: |
|
🎉 This PR is included in version @api-ts/typed-express-router@2.0.0-beta.1 🎉 The release is available on npm package (@beta dist-tag) Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version @api-ts/express-wrapper@2.0.0-beta.1 🎉 The release is available on npm package (@beta dist-tag) Your semantic-release bot 📦🚀 |
|
@starfy84 Can we update README.md on deprecation of |
Will do! Thanks for letting me know. |
|
🎉 This PR is included in version @api-ts/typed-express-router@2.0.0 🎉 The release is available on npm package (@latest dist-tag) Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version @api-ts/express-wrapper@2.0.0 🎉 The release is available on npm package (@latest dist-tag) Your semantic-release bot 📦🚀 |
|
🎉 This PR is included in version @api-ts/openapi-generator@5.10.2 🎉 The release is available on npm package (@latest dist-tag) Your semantic-release bot 📦🚀 |
4 participants
Ticket: DX-1473
This PR implements
opentelemetryinstrumentation for thedecodeandsendEncodedcalls inwrapRouter.Changes:
typed-express-wrapper@opentelemetry/api@^1.0.0added as an optional peer dependency@opentelemetry/sdk-trace-base@1.30.1,@opentelemetry/sdk-trace-node@1.30.1, and@opentelemetry/api@1.9.0added as dev dependencieswrapRoutermodified to create decode and encode spans if@opentelemetry/apiis installed@opentelemetry/apiis not installed, spans are not createdonDecodeErroroption removed. Please usedecodeErrorFormatterandgetDecodeErrorStatusCodedecodeErrorFormattertakes in an array ofValidationErrors and aWrappedRequest, returning aJsonobject.getDecodeErrorStatusCodetakes in an array ofValidationErrors and aWrappedRequest, returning a number.onEncodeErroroption removed. Please useencodeErrorFormatterandgetEncodeErrorStatusCodeencodeErrorFormattertakes in an error and aWrappedRequest, returning aJsonobject.getEncodeErrorStatusCodetakes in an error and aWrappedRequest, returning a number.typed-express-routernow handles the sending of the http response when there is a decode or encode errorexpress-wrapperrouterForApiSpecmodified to pass in new parametersdecodeErrorFormatter,getDecodeErrorStatusCode,encodeErrorFormatter, andgetEncodeErrorStatusCodeCreateRouterProps)BREAKING CHANGE:
onDecodeErrorandonEncodeErrorhave been removed. Please usedecodeErrorFormatter,getDecodeErrorStatusCode,encodeErrorFormatter, andgetEncodeErrorStatusCode