Skip to content
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

chore: Improve LogFormatter to include Types and Support Additional Options without the Custom attribute #7813

Merged
merged 4 commits into from
Apr 11, 2023

Conversation

dthyresson
Copy link
Contributor

@dthyresson dthyresson commented Mar 14, 2023

Replaces #5235

Fixes #5228 and rewrites the full logger with proper TS types

Breaking change:

All metadata sent to the logger will now be printed.

Previously if you did logger.debug({ user }, 'my user') only "my user" would be printed. Now the full user object will be included in the output

TODO

Docs update to the "custom" log formatter. Should be deprecated as will include additional metadata options now.

@dthyresson dthyresson self-assigned this Mar 14, 2023
@dthyresson dthyresson added release:breaking This PR is a breaking change topic/logging labels Mar 14, 2023
@dthyresson dthyresson requested a review from Tobbe March 14, 2023 16:06
@replay-io
Copy link

replay-io bot commented Mar 14, 2023

16 replays were recorded for d18e1da.

image 0 Failed
image 16 Passed
    requireAuth graphql checks
          ```
          locator.waitFor: Target closed
          =========================== logs ===========================
          waiting for locator('.rw-form-error-title').locator('text=You don\'t have permission to do that') to be visible
          ============================================================
          ```
        </ol>
      </details>
      <li><a href=https://app.replay.io/recording/44b8f802-7792-4135-9c58-46d76b793493>useAuth hook, auth redirects checks</a></li>
      <li><a href=https://app.replay.io/recording/a989589b-0ffe-45f3-9fa8-427dadb930dc>Check that a specific blog post is prerendered</a></li>
      <li><a href=https://app.replay.io/recording/48b1bf78-0aa1-4b0d-a4ef-3c08aaf6977b>Check that about is prerendered</a></li>
      <li><a href=https://app.replay.io/recording/7394f47c-3951-488f-941f-d6cae962c9d9>Check that homepage is prerendered</a></li>
      <li><a href=https://app.replay.io/recording/52937cd4-78ee-4262-8d94-723db342eb3d>Check that meta-tags are rendering the correct dynamic data</a></li>
      <li><a href=https://app.replay.io/recording/70d1cff3-8fb5-4483-a532-4e275314a9c8>Check that you can navigate from home page to specific blog post</a></li>
      <li><a href=https://app.replay.io/recording/e2b426c4-d67e-4bd8-b52c-db6586ace7ae>Waterfall prerendering (nested cells)</a></li>
      <li><a href=https://app.replay.io/recording/f019c359-f0d3-4628-89cf-fdaeb0d95efe>RBAC: Admin user should be able to delete contacts</a></li>
      <li><a href=https://app.replay.io/recording/abbea64b-9329-43f2-961c-8a146fbd5094>RBAC: Should not be able to delete contact as non-admin user</a></li>
      <li><a href=https://app.replay.io/recording/37aa01c8-f2b0-48e4-8a7d-64229faf974d>Smoke test with dev server</a></li>
      <li><a href=https://app.replay.io/recording/f479d94f-3e49-4ad5-af94-cb109e74997d>Smoke test with rw serve</a></li>
      <li><a href=https://app.replay.io/recording/fa4b0346-99ea-465e-82d9-5a86793bcf35>Loads Cell mocks when Cell is nested in another story</a></li>
      <li><a href=https://app.replay.io/recording/268a11db-8f95-49aa-8349-764c57c2eea9>Loads Cell Stories</a></li>
      <li><a href=https://app.replay.io/recording/815c2de6-57fe-4f3e-843b-8944e97def13>Loads MDX Stories</a></li>
      <li><a href=https://app.replay.io/recording/7ea21d64-4805-45ca-b73a-f70c46702a40>Mocks current user, and updates UI while dev server is running</a></li>
      

View test run on Replay ↗︎

@jtoar
Copy link
Contributor

jtoar commented Mar 15, 2023

@dthyresson nice, can I close #5235 just for housekeeping purposes?

@thedavidprice thedavidprice mentioned this pull request Mar 27, 2023
Copy link
Member

@Tobbe Tobbe left a comment

Choose a reason for hiding this comment

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

This is a definite improvement over what we currently have, so I think we should merge this! And since it's breaking I'd rather merge it sooner than later so we get it in for v5

There are some more TS tweaks I'd like to see, but let's merge this first, and then I can look at the TS stuff later

packages/api-server/src/__tests__/logFormatter.test.ts Outdated Show resolved Hide resolved
Co-authored-by: Tobbe Lundberg <tobbe@tlundberg.com>
@dthyresson dthyresson marked this pull request as ready for review April 11, 2023 15:45
@Tobbe Tobbe merged commit 4142805 into redwoodjs:main Apr 11, 2023
@redwoodjs-bot redwoodjs-bot bot added this to the next-release milestone Apr 11, 2023
@Tobbe Tobbe changed the title chore: DRAFT Improve LogFormatter to include Types and Support Additional Options without the Custom attribute chore: Improve LogFormatter to include Types and Support Additional Options without the Custom attribute Apr 11, 2023
@jtoar jtoar modified the milestones: next-release, v5.0.0 Apr 27, 2023
@Philzen
Copy link
Contributor

Philzen commented Dec 8, 2024

@Tobbe / @dthyresson Am i correct that after having upgraded to RW 5, this doc section does not apply any longer and needs to be updated?

https://docs.redwoodjs.com/docs/logger#with-a-custom-payload

What we did in out code base was effectively to remove all custom: { dataToLog } wrapper around dataToLog.

@Tobbe
Copy link
Member

Tobbe commented Dec 8, 2024

I see your comment @Philzen. This is so old, I can't really remember the context. I'll try to take a look next week (if I can remember 😅)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
release:breaking This PR is a breaking change topic/logging
Projects
None yet
Development

Successfully merging this pull request may close these issues.

logFormatter doesn't print metadata
4 participants