Skip to content

Commit

Permalink
Fix Pino transport
Browse files Browse the repository at this point in the history
The Pino transport could not be used as a transport (that is, as
initialised by `pino.transport()`, but only as a destination, and
only in the main thread.

This commit makes the Pino transport communicate with the extension
directly, which should work from any worker thread -- the assumption
being that the extension has been started by the main thread.

This works around the fact that the client cannot be serialised and
sent across threads.

This allows for the Pino transport to be used like other transports,
and _alongside_ other transports:

```js
const logger = pino({
  transport: {
    targets: [
      { target: "@appsignal/nodejs/pino", options: { group: "pino" } },
      { target: ... }
    ]
  }
})
```

The `pino/index.js` file has been added as a convenience to allow for
the target to be referred to as `@appsignal/nodejs/pino`, which is
nicer-looking than the transport's real location inside `dist/`.
  • Loading branch information
unflxw committed Nov 21, 2024
1 parent edadd7c commit ad72f5a
Show file tree
Hide file tree
Showing 4 changed files with 81 additions and 21 deletions.
50 changes: 50 additions & 0 deletions .changesets/allow-pino-transport-to-be-used-as-a-transport.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,50 @@
---
bump: patch
type: fix
---

Allow Pino transport to be used as a transport. Before, our Pino transport could only be used as a destination:

```js
import pino from "pino";
import { Appsignal, AppsignalPinoTransport } from "@appsignal/nodejs";

pino(AppsignalPinoTransport({
client: Appsignal.client,
group: "pino"
}));
```

This meant it was not possible to log both to our transport and to another destination.

Now, it is also possible to use it as a Pino transport, with the `transport` Pino config option or the `pino.transport()` function:

```js
import pino from "pino";

pino({
transport: {
target: "@appsignal/nodejs/pino",
options: {
group: "pino"
}
}
});
```

It is no longer necessary to pass the AppSignal client to the Pino transport. AppSignal must be active for the Pino transport to work.

By enabling its use as a transport, it is now possible to use it alongside other transports:

```js
pino({
transport: {
targets: [
// Send logs to AppSignal...
{ target: "@appsignal/nodejs/pino" },
// ... and to standard output!
{ target: "pino/file" }
]
}
});
```
1 change: 1 addition & 0 deletions .npmignore
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
*
!/dist/**/*
!/pino/**/*
!/scripts/**/*
scripts/**/*.test.js
!/ext/appsignal_extension.cpp
Expand Down
2 changes: 2 additions & 0 deletions pino/index.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
"use strict";

Check failure on line 1 in pino/index.js

View workflow job for this annotation

GitHub Actions / JavaScript linter (Prettier)

Delete `;`
module.exports = require("../dist/pino_transport");

Check failure on line 2 in pino/index.js

View workflow job for this annotation

GitHub Actions / JavaScript linter (Prettier)

Delete `;`
49 changes: 28 additions & 21 deletions src/pino_transport.ts
Original file line number Diff line number Diff line change
@@ -1,44 +1,51 @@
import { LOGGER_LEVEL_SEVERITY } from "./logger"
import { LOGGER_LEVEL_SEVERITY, LOGGER_FORMAT } from "./logger"
import build from "pino-abstract-transport"
import { Client } from "./client"
import { Extension } from "./extension"

type PinoTransportOptions = {
client: Client
group: string
group?: string
}

type LogData = {
severity: number
message: string
attributes: Record<string, any>
}

const appsignalPinoTransport = ({

Check failure on line 15 in src/pino_transport.ts

View workflow job for this annotation

GitHub Actions / JavaScript linter (Prettier)

Replace `⏎··group·=·"app"⏎` with `·group·=·"app"·`
client,
group = "app"
}: PinoTransportOptions) => {
return build(async (source: any) => {
// We expect this code to be running in a worker thread and for the
// extension to have already been loaded and started on the main thread.
// Since we expect the extension to already have been initialised, we
// pass `{ active: false }` to avoid initialising it again.
const extension = new Extension({ active: false })

for await (const obj of source) {
sendLogs(parseInfo(obj, group), client)
sendLogs(extension, group, parseInfo(obj))
}
})
}

async function sendLogs(data: Record<string, any>, client: Client) {
client.extension.log(
data.group || "app",
data.severity,
0,
data.msg,
data.attributes
)
}
async function sendLogs(extension: Extension, group: string, data: LogData) {
extension.log(
group,
data.severity,
LOGGER_FORMAT.plaintext,
data.message,
data.attributes
)
}

function parseInfo(

Check failure on line 41 in src/pino_transport.ts

View workflow job for this annotation

GitHub Actions / JavaScript linter (Prettier)

Replace `⏎··obj:·Record<string,·any>,⏎` with `obj:·Record<string,·any>`
obj: Record<string, any>,
group: string
): Record<string, any> {
const { hostname, level, msg, ...attributes } = obj
): LogData {
const { level, msg, ...attributes } = obj

return {
severity: getSeverity(level),
hostname,
group,
msg,
message: msg,
attributes: flattenAttributes(attributes)
}
}
Expand Down

0 comments on commit ad72f5a

Please sign in to comment.