-
Notifications
You must be signed in to change notification settings - Fork 212
Adds structured logging API. #665
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
Changes from all commits
a6f77a0
3c614e1
2735dfc
a0be017
b837799
484a4af
9228bb4
b728e01
1b9356f
58ab50f
1ae83f9
f1acceb
a8001d1
f95aa28
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
- Adds `functions.logger` SDK to enable structured logging in the Node.js 10 runtime. For example: | ||
|
||
```js | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Wow, does this work? In the d.ts files we use ```javascript, but a shorter form would be nice. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yup 😄 |
||
const functions = require('firebase-functions'); | ||
|
||
functions.logger.debug('example log with structured data', { | ||
uid: user.uid, | ||
authorized: true, | ||
}); | ||
``` | ||
|
||
The logger can also override default behavior of `console.*` methods through a special require: | ||
|
||
```js | ||
require('firebase-functions/logger/compat'); | ||
``` | ||
|
||
In older runtimes, logger prints to the console, and no structured data is saved. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,105 @@ | ||
import { expect } from 'chai'; | ||
import * as sinon from 'sinon'; | ||
import * as logger from '../src/logger'; | ||
|
||
const SUPPORTS_STRUCTURED_LOGS = | ||
parseInt(process.versions?.node?.split('.')?.[0] || '8', 10) >= 10; | ||
laurenzlong marked this conversation as resolved.
Show resolved
Hide resolved
|
||
|
||
describe(`logger (${ | ||
SUPPORTS_STRUCTURED_LOGS ? 'structured' : 'unstructured' | ||
})`, () => { | ||
let sandbox: sinon.SinonSandbox; | ||
let stdoutStub: sinon.SinonStub; | ||
let stderrStub: sinon.SinonStub; | ||
|
||
beforeEach(() => { | ||
sandbox = sinon.createSandbox(); | ||
stdoutStub = sandbox.stub(process.stdout, 'write'); | ||
stderrStub = sandbox.stub(process.stderr, 'write'); | ||
}); | ||
|
||
function expectOutput(stdStub: sinon.SinonStub, entry: any) { | ||
if (SUPPORTS_STRUCTURED_LOGS) { | ||
return expect( | ||
JSON.parse((stdStub.getCalls()[0].args[0] as string).trim()) | ||
).to.deep.eq(entry); | ||
} else { | ||
// legacy logging is not structured, but do a sanity check | ||
return expect(stdStub.getCalls()[0].args[0]).to.include(entry.message); | ||
} | ||
} | ||
|
||
function expectStdout(entry: any) { | ||
return expectOutput(stdoutStub, entry); | ||
} | ||
|
||
function expectStderr(entry: any) { | ||
return expectOutput(stderrStub, entry); | ||
} | ||
|
||
describe('logging methods', () => { | ||
let writeStub: sinon.SinonStub; | ||
beforeEach(() => { | ||
writeStub = sinon.stub(logger, 'write'); | ||
}); | ||
|
||
afterEach(() => { | ||
writeStub.restore(); | ||
}); | ||
|
||
it('should coalesce arguments into the message', () => { | ||
logger.log('hello', { middle: 'obj' }, 'end message'); | ||
expectStdout({ | ||
severity: 'INFO', | ||
message: "hello { middle: 'obj' } end message", | ||
}); | ||
sandbox.restore(); // to avoid swallowing test runner output | ||
}); | ||
|
||
it('should merge structured data from the last argument', () => { | ||
logger.log('hello', 'world', { additional: 'context' }); | ||
expectStdout({ | ||
severity: 'INFO', | ||
message: 'hello world', | ||
additional: 'context', | ||
}); | ||
sandbox.restore(); // to avoid swallowing test runner output | ||
}); | ||
}); | ||
|
||
describe('write', () => { | ||
describe('structured logging', () => { | ||
describe('write', () => { | ||
for (const severity of ['DEBUG', 'INFO', 'NOTICE']) { | ||
it(`should output ${severity} severity to stdout`, () => { | ||
let entry: logger.LogEntry = { | ||
severity: severity as logger.LogSeverity, | ||
message: 'test', | ||
}; | ||
logger.write(entry); | ||
expectStdout(entry); | ||
sandbox.restore(); // to avoid swallowing test runner output | ||
}); | ||
} | ||
|
||
for (const severity of [ | ||
'WARNING', | ||
'ERROR', | ||
'CRITICAL', | ||
'ALERT', | ||
'EMERGENCY', | ||
]) { | ||
it(`should output ${severity} severity to stderr`, () => { | ||
let entry: logger.LogEntry = { | ||
severity: severity as logger.LogSeverity, | ||
message: 'test', | ||
}; | ||
logger.write(entry); | ||
expectStderr(entry); | ||
sandbox.restore(); // to avoid swallowing test runner output | ||
}); | ||
} | ||
}); | ||
}); | ||
}); | ||
}); |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,135 @@ | ||
import { format } from 'util'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this import got in the middle of two lines of comments? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Oops, fixed. |
||
|
||
// safely preserve unpatched console.* methods in case of compat require | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Will these be parsed into the reference by our TypeDoc script? If so, suggest "//Safely . . . require." with init cap and period. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nope. |
||
const unpatchedConsole = { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think this still won't work if somebody did this:
I think there's many opportunities for there to be weird bugs and edge cases with 'compat', and I'm not sure it's worth the incremental convenience of providing this functionality. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It does work, because the first thing This wasn't in my original proposal but Kato asked if it was possible and it turned out to be easier than expected. I do worry about the chance for bugs here but this is just one way to use the SDK and provides a convenience. Anyone who encounters bugs we can just tell to use the other ways of using the SDK. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah yes, compat does import logger first, this is really subtle behavior, it's good you added a comment to clarify. I'm still not a fan of |
||
debug: console.debug, | ||
info: console.info, | ||
log: console.log, | ||
warn: console.warn, | ||
error: console.error, | ||
}; | ||
|
||
// Determine if structured logs are supported (node >= 10). If something goes wrong, | ||
// assume no since unstructured is safer. | ||
const SUPPORTS_STRUCTURED_LOGS = | ||
parseInt(process.versions?.node?.split('.')?.[0] || '8', 10) >= 10; | ||
|
||
// Map LogSeverity types to their equivalent `console.*` method. | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. If this goes to public docs, let's backtick There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Won't be in public docs. |
||
const CONSOLE_SEVERITY: { | ||
[severity: string]: 'debug' | 'info' | 'warn' | 'error'; | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. (I'm not sure if TS would let you do this, but if it does). This would be clearer as: There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. TS doesn't let you do this 😢 There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 😢 |
||
} = { | ||
DEBUG: 'debug', | ||
INFO: 'info', | ||
NOTICE: 'info', | ||
WARNING: 'warn', | ||
ERROR: 'error', | ||
CRITICAL: 'error', | ||
ALERT: 'error', | ||
EMERGENCY: 'error', | ||
}; | ||
|
||
/** | ||
* `LogSeverity` indicates the detailed severity of the log entry. See [LogSeverity](https://cloud.google.com/logging/docs/reference/v2/rest/v2/LogEntry#logseverity) for more. | ||
*/ | ||
export type LogSeverity = | ||
| 'DEBUG' | ||
| 'INFO' | ||
| 'NOTICE' | ||
| 'WARNING' | ||
| 'ERROR' | ||
| 'CRITICAL' | ||
| 'ALERT' | ||
| 'EMERGENCY'; | ||
|
||
/** | ||
* `LogEntry` represents a structured Cloud Logging entry. All keys aside from `severity` and `message` are | ||
* included in the `jsonPayload` of the logged entry. | ||
*/ | ||
export interface LogEntry { | ||
severity: LogSeverity; | ||
message?: string; | ||
[key: string]: any; | ||
} | ||
|
||
/** | ||
* Writes a `LogEntry` to `stdout`/`stderr` (depending on severity). | ||
* @param entry The LogEntry including severity, message, and any additional structured metadata. | ||
*/ | ||
export function write(entry: LogEntry) { | ||
if (SUPPORTS_STRUCTURED_LOGS) { | ||
unpatchedConsole[CONSOLE_SEVERITY[entry.severity]](JSON.stringify(entry)); | ||
return; | ||
} | ||
|
||
let message = entry.message || ''; | ||
const jsonPayload: { [key: string]: any } = {}; | ||
let jsonKeyCount = 0; | ||
for (const k in entry) { | ||
if (!['severity', 'message'].includes(k)) { | ||
jsonKeyCount++; | ||
jsonPayload[k] = entry[k]; | ||
} | ||
} | ||
if (jsonKeyCount > 0) { | ||
message = `${message} ${JSON.stringify(jsonPayload, null, 2)}`; | ||
} | ||
unpatchedConsole[CONSOLE_SEVERITY[entry.severity]](message); | ||
} | ||
|
||
/** | ||
* Writes a `DEBUG` severity log. If the last argument provided is a plain object, | ||
* it will be added to the `jsonPayload` in the Cloud Logging entry. | ||
* @param args Arguments, concatenated into the log message with space separators. | ||
*/ | ||
export function debug(...args: any[]) { | ||
write(entryFromArgs('DEBUG', args)); | ||
} | ||
|
||
/** | ||
* Writes an `INFO` severity log. If the last argument provided is a plain object, | ||
* it will be added to the `jsonPayload` in the Cloud Logging entry. | ||
* @param args Arguments, concatenated into the log message with space separators. | ||
*/ | ||
export function log(...args: any[]) { | ||
write(entryFromArgs('INFO', args)); | ||
} | ||
|
||
/** | ||
* Writes an `INFO` severity log. If the last argument provided is a plain object, | ||
* it will be added to the `jsonPayload` in the Cloud Logging entry. | ||
* @param args Arguments, concatenated into the log message with space separators. | ||
*/ | ||
export function info(...args: any[]) { | ||
write(entryFromArgs('INFO', args)); | ||
} | ||
|
||
/** | ||
* Writes a `WARNING` severity log. If the last argument provided is a plain object, | ||
* it will be added to the `jsonPayload` in the Cloud Logging entry. | ||
* @param args Arguments, concatenated into the log message with space separators. | ||
*/ | ||
export function warn(...args: any[]) { | ||
write(entryFromArgs('WARNING', args)); | ||
} | ||
|
||
/** | ||
* Writes an `ERROR` severity log. If the last argument provided is a plain object, | ||
* it will be added to the `jsonPayload` in the Cloud Logging entry. | ||
* @param args Arguments, concatenated into the log message with space separators. | ||
*/ | ||
export function error(...args: any[]) { | ||
write(entryFromArgs('ERROR', args)); | ||
} | ||
|
||
function entryFromArgs(severity: LogSeverity, args: any[]): LogEntry { | ||
let entry = {}; | ||
const lastArg = args[args.length - 1]; | ||
if (typeof lastArg == 'object' && lastArg.constructor == Object) { | ||
entry = args.pop(); | ||
} | ||
return Object.assign({}, entry, { | ||
severity, | ||
// mimic `console.*` behavior, see https://nodejs.org/api/console.html#console_console_log_data_args | ||
message: format.apply(null, args), | ||
}); | ||
} |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,8 @@ | ||
import { debug, info, warn, error } from '../logger'; | ||
|
||
// IMPORTANT -- "../logger" must be imported before monkeypatching! | ||
console.debug = debug; | ||
console.info = info; | ||
console.log = info; | ||
console.warn = warn; | ||
console.error = error; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A nit, but in a text flow we prefer "For example:"
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.