-
Notifications
You must be signed in to change notification settings - Fork 53.6k
feat(core): Allow logging JSON to stdout #15766
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
Conversation
Codecov ReportAttention: Patch coverage is
📢 Thoughts on this report? Let us know! |
1723aa5 to
c21e7cd
Compare
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.
cubic found 1 issue across 4 files. Review it in cubic.dev
React with 👍 or 👎 to teach cubic. Tag @cubic-dev-ai to give specific feedback.
| for (const key of Object.keys(metadata)) { | ||
| const value = metadata[key]; | ||
| if (value instanceof Error) { | ||
| metadata[key] = this.detailedErrorStringify(value); | ||
| } | ||
| } | ||
|
|
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.
Would it be reasonable to expect the key to be called error so we don't have to check every key in every metadata object in every log?
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.
I'd rather be safe than sorry. But we should probably adhere to that pattern. I think we could write an eslint rule for that, but that's out of scope for this PR.
…etween info, error and debug logs
3cd87ce to
9b920d4
Compare
ivov
left a comment
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.
Thanks for addressing everything!
Asked about the single-line logs here to confirm.
Workflow Test Results 📊 🔴 1 Failed,
|
| Workflow ID | Workflow Name | Reason |
|---|---|---|
| 237 | BasicLLMChain:AzureChat | Workflow contains 1 deleted data. |
⚠️ Warnings (3)
| Workflow ID | Workflow Name | Reason |
|---|---|---|
| 35 | Slack:User:getPresence info:UserProfile:get update... | Workflow contains new data that previously did not exist. |
| 257 | Agent:auto-fix:anthropic | Workflow contains new data that previously did not exist. |
| 53 | ConvertKit:CustomField:create getAll update delete... | Workflow contains new data that previously did not exist. |
|
✅ All Cypress E2E specs passed |
|
Got released with |
Summary
Before this we would always log text to stdout, which is sub-optimal for production use cases because we loose all of the metadata that log ingestion systems could use:
This PR adds a new environment variable to change the log's output format
N8N_LOG_FORMAT. It can be set todefaultandjsonand defaults todefault, which is the current text based output.Related Linear tickets, Github issues, and Community forum posts
https://linear.app/n8n/issue/PAY-2779/allow-logging-json-to-stdout
Review / Merge checklist
PR Labeled withrelease/backport(if the PR is an urgent fix that needs to be backported)