Skip to content

feat: implement unified JSON/text logging format #4392

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

feat: implement unified JSON/text logging format #4392

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
1 change: 1 addition & 0 deletions api/lib/Gateway.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -191,6 +191,7 @@ export type GatewayConfig = {
logEnabled?: boolean
logLevel?: LogLevel
logToFile?: boolean
logFormat?: 'text' | 'json'
values?: GatewayValue[]
jobs?: ScheduledJob[]
plugins?: string[]
Expand Down
103 changes: 95 additions & 8 deletions api/lib/ZwaveClient.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,8 @@ import {
} from '@zwave-js/core'
import { createDefaultTransportFormat } from '@zwave-js/core/bindings/log/node'
import { JSONTransport } from '@zwave-js/log-transport-json'
import winston from 'winston'
import DailyRotateFile from 'winston-daily-rotate-file'
import { isDocker } from './utils'
import {
AssociationAddress,
Expand Down Expand Up @@ -133,6 +135,7 @@ import { socketEvents } from './SocketEvents'
import { isUint8Array } from 'util/types'
import { PkgFsBindings } from './PkgFsBindings'
import { join } from 'path'
import * as path from 'path'
import { regionSupportsAutoPowerlevel } from './shared'

export const deviceConfigPriorityDir = join(storeDir, 'config')
Expand Down Expand Up @@ -2318,14 +2321,8 @@ class ZwaveClient extends TypedEventEmitter<ZwaveClientEventCallbacks> {

utils.parseSecurityKeys(this.cfg, zwaveOptions)

const logTransport = new JSONTransport()
logTransport.format = createDefaultTransportFormat(true, false)

zwaveOptions.logConfig.transports = [logTransport]

logTransport.stream.on('data', (data) => {
this.socket.emit(socketEvents.debug, data.message.toString())
})
// Setup driver logging based on format setting
this.setupDriverLogging(zwaveOptions)

try {
if (shouldUpdateSettings) {
Expand Down Expand Up @@ -6962,6 +6959,96 @@ class ZwaveClient extends TypedEventEmitter<ZwaveClientEventCallbacks> {
}
}, 1000)
}


private setupDriverLogging(zwaveOptions: PartialZWaveOptions) {
Copy link
Member

@robertsLando robertsLando Oct 1, 2025

Choose a reason for hiding this comment

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

I think the code in this function is overcomplicated and introduces lot of repeated code

Copy link
Author

@lngarrett lngarrett Oct 2, 2025

Choose a reason for hiding this comment

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

I broke this up into more manageable functions and made it more DRY.

const logFormat = this.getLogFormat()

if (logFormat === 'json') {
this.setupJsonDriverLogging(zwaveOptions)
} else {
this.setupTextDriverLogging(zwaveOptions)
}
}

private getLogFormat(): 'text' | 'json' {
const settings = jsonStore.get(store.settings)
return settings?.gateway?.logFormat || 'text'
}

private setupJsonDriverLogging(zwaveOptions: PartialZWaveOptions) {
const transports = []

const parseFormat = this.createParseDriverJsonFormat()
const jsonFormat = winston.format.combine(
parseFormat(),
winston.format.json(),
)

// Console transport
transports.push(new winston.transports.Console({
format: jsonFormat,
}))

// File transport (if enabled)
if (this.cfg.logToFile) {
transports.push(new DailyRotateFile({
filename: ZWAVEJS_LOG_FILE,
auditFile: utils.joinPath(logsDir, 'zwavejs-logs.audit.json'),
datePattern: 'YYYY-MM-DD',
createSymlink: true,
symlinkName: path.basename(ZWAVEJS_LOG_FILE).replace('_%DATE%', '_current'),
zippedArchive: true,
maxFiles: process.env.ZUI_LOG_MAXFILES || '7d',
maxSize: process.env.ZUI_LOG_MAXSIZE || '50m',
format: jsonFormat,
}))
}

// WebSocket transport with JSON format
const jsonTransport = new JSONTransport()
jsonTransport.format = jsonFormat
transports.push(jsonTransport)

// Configure driver
zwaveOptions.logConfig = {
...zwaveOptions.logConfig,
enabled: false,
raw: true,
showLogo: false,
transports: transports,
}

// Stream JSON logs to WebSocket for debug view
jsonTransport.stream.on('data', (data) => {
this.socket.emit(socketEvents.debug, data.message.toString())
})
}

private setupTextDriverLogging(zwaveOptions: PartialZWaveOptions) {
const logTransport = new JSONTransport()
logTransport.format = createDefaultTransportFormat(true, false)

zwaveOptions.logConfig.transports = [logTransport]

logTransport.stream.on('data', (data) => {
this.socket.emit(socketEvents.debug, data.message.toString())
})
}

private createParseDriverJsonFormat() {
return winston.format((info) => {
if (typeof info.message === 'string' && info.message.startsWith('{')) {
try {
const parsed = JSON.parse(info.message)
info.message = parsed
} catch (e) {
// Keep as string if parsing fails
}
}
return info
})
}
}

export default ZwaveClient
32 changes: 25 additions & 7 deletions api/lib/logger.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -70,8 +70,22 @@ export function sanitizedConfig(
/**
* Return a custom logger format
*/
export function customFormat(noColor = false): winston.Logform.Format {
export function customFormat(
noColor = false,
logFormat: 'text' | 'json' = 'text',
): winston.Logform.Format {
noColor = noColor || disableColors

if (logFormat === 'json') {
// JSON format for all outputs
return combine(
timestamp(),
format.errors({ stack: true }),
format.json(),
)
}

// Existing text format
const formats: winston.Logform.Format[] = [
splat(), // used for formats like: logger.log('info', Message %s', strinVal)
timestamp({ format: 'YYYY-MM-DD HH:mm:ss.SSS' }),
Expand Down Expand Up @@ -106,7 +120,10 @@ export const logStream = new PassThrough()
/**
* Create the base transports based on settings provided
*/
export function customTransports(config: LoggerConfig): winston.transport[] {
export function customTransports(
config: LoggerConfig,
logFormat: 'text' | 'json' = 'text',
): winston.transport[] {
// setup transports only once (see issue #2937)
if (transportsList) {
return transportsList
Expand All @@ -117,15 +134,15 @@ export function customTransports(config: LoggerConfig): winston.transport[] {
if (process.env.ZUI_NO_CONSOLE !== 'true') {
transportsList.push(
new transports.Console({
format: customFormat(),
format: customFormat(false, logFormat),
level: config.level,
stderrLevels: ['error'],
}),
)
}

const streamTransport = new transports.Stream({
format: customFormat(),
format: customFormat(false, logFormat),
level: config.level,
stream: logStream,
})
Expand All @@ -137,7 +154,7 @@ export function customTransports(config: LoggerConfig): winston.transport[] {

if (process.env.DISABLE_LOG_ROTATION === 'true') {
fileTransport = new transports.File({
format: customFormat(true),
format: customFormat(true, logFormat),
filename: config.filePath,
level: config.level,
})
Expand All @@ -154,7 +171,7 @@ export function customTransports(config: LoggerConfig): winston.transport[] {
maxFiles: process.env.ZUI_LOG_MAXFILES || '7d',
maxSize: process.env.ZUI_LOG_MAXSIZE || '50m',
level: config.level,
format: customFormat(true),
format: customFormat(true, logFormat),
}
fileTransport = new DailyRotateFile(options)

Expand Down Expand Up @@ -182,6 +199,7 @@ export function setupLogger(
config?: DeepPartial<GatewayConfig>,
): ModuleLogger {
const sanitized = sanitizedConfig(module, config)
const logFormat = config?.logFormat || 'text'
// Winston automatically reuses an existing module logger
const logger = container.add(module) as ModuleLogger
const moduleName = module.toUpperCase() || '-'
Expand All @@ -196,7 +214,7 @@ export function setupLogger(
), // to correctly parse errors
silent: !sanitized.enabled,
level: sanitized.level,
transports: customTransports(sanitized),
transports: customTransports(sanitized, logFormat),
})
logger.module = module
logger.setup = (cfg) => setupLogger(container, module, cfg)
Expand Down
18 changes: 18 additions & 0 deletions src/views/Settings.vue
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,20 @@
label="Log Level"
></v-select>
</v-col>
<v-col
cols="12"
sm="6"
md="4"
v-if="newGateway.logEnabled"
>
<v-select
:items="logFormats"
v-model="newGateway.logFormat"
label="Log Format"
hint="Choose between human-readable text or structured JSON logging"
Copy link
Member

@AlCalzone AlCalzone Oct 9, 2025

Choose a reason for hiding this comment

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

I'm a bit afraid that making users choose hinders our troubleshooting capabilities. Everything from the automated bot comments when users post the wrong log, over the automatic log analyzer, to my feeble brain expects the text-based logging format.

Can we instead just add the JSON based logs as an additional transport that can be toggled?

Copy link
Author

@lngarrett lngarrett Oct 9, 2025

Choose a reason for hiding this comment

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

I'm trying to think about how best to do this. My use case for JSON logs is to ship them to centralized logging tools - ideally this reads the STDOUT of the container/process. Alternatively, it would read known files, but that's less ideal as the files then need to be specified whereas STDOUT is assumed.

I see no problem in always making the web console text format. I actually almost did that, but decided consistency was better. But, I think it would make sense to do so.

Where I'm stuck is - what is your expectation of the format of STDOUT and file logs in the case where the additional JSON transport is toggled on?

Copy link
Member

@AlCalzone AlCalzone Oct 16, 2025

Choose a reason for hiding this comment

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

I think configuring the STDOUT should be fine. Our most common troubleshooting use case is having users write debug logs to file and uploading those files.

Copy link
Author

@lngarrett lngarrett Oct 16, 2025

Choose a reason for hiding this comment

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

It just occurred to me that this presents a conflict I believe:

Driver's raw option is global - it affects ALL transports (console, file, web UI)

  • When raw: true -> JSON for everything (loses human-readable formatting)
  • When raw: false -> Text for everything (can't get JSON console)

We could move human parsing logic to the UI, but that's duplicative to what the driver already does in text mode so it feels a bit hacky. Is there another way to solve this that I'm missing?

Copy link
Member

@AlCalzone AlCalzone Oct 21, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

I guess I'll have to tie the message formatting into the transport format, so it can be toggled on a per-transport basis.
I've raised zwave-js/zwave-js#8375 for this. The solution probably involves getting rid of the raw option again though, so this PR should be considered blocked by that.

persistent-hint
></v-select>
</v-col>
<v-col
cols="12"
sm="6"
Expand Down Expand Up @@ -2269,6 +2283,10 @@ export default {
{ title: 'Debug', value: 'debug' },
{ title: 'Silly', value: 'silly' },
],
logFormats: [
{ title: 'Text (Human-readable)', value: 'text' },
{ title: 'JSON (Structured)', value: 'json' },
],
headers: [
{ title: 'Device', key: 'device' },
{ title: 'Value', key: 'value', sortable: false },
Expand Down
21 changes: 21 additions & 0 deletions test/lib/logger.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import * as utils from '../../api/lib/utils'
import { logsDir } from '../../api/config/app'
import {
customTransports,
customFormat,
defaultLogFile,
ModuleLogger,
sanitizedConfig,
Expand Down Expand Up @@ -177,4 +178,24 @@ describe('logger.js', () => {
expect(logger2.level).to.equal('warn')
})
})

describe('customFormat()', () => {
it('should return format object when logFormat is json', () => {
const format = customFormat(false, 'json')
expect(format).to.be.an('object')
expect(format).to.have.property('transform')
})

it('should return format object when logFormat is text', () => {
const format = customFormat(true, 'text')
expect(format).to.be.an('object')
expect(format).to.have.property('transform')
})

it('should return format object when logFormat is undefined', () => {
const format = customFormat(true, undefined)
expect(format).to.be.an('object')
expect(format).to.have.property('transform')
})
})
})