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

use boolean flag to ensure serializers are applied once #1976

Conversation

emmyakin
Copy link
Contributor

@emmyakin emmyakin commented May 15, 2024

var logger = pino({
  serializers: {
    extras(data) {
      return { serializedExtras: data };
    },
  },
  browser: {
    serialize: ['extras'],
    asObject: true,
    transmit: {
      send(level, o) {
        is(o.messages[0].extras.serializedExtras, 'world');
      },
    },
  },
})

When opt.browser.transmit is defined, serializers are not applied to write

Conditions when serializers should be applied

  • when opts.serialize is true or defined
    • serializers should be applied to write and transmit operations
  • when opts.serialize is false or undefined
    • serializers should be applied to transmit operation only

@jsumners
Copy link
Member

I am not following. Can you please provide a direct statement of the problem this is solving?

Copy link
Member

@mcollina mcollina left a comment

Choose a reason for hiding this comment

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

lgtm

@mcollina mcollina merged commit 0109b15 into pinojs:main May 15, 2024
2 checks passed
@emmyakin
Copy link
Contributor Author

@jsumners

I am not following. Can you please provide a direct statement of the problem this is solving?

Sure

I had created this PR #1971 to fix serializers being applied twice to logs, i.e, before write and in transmit

The fix only applies serializers if transmit is not defined, which breaks the original implementation, where we want apply serializers for write operations even if transmit is defined, but options.serialize is true

Josh-Walker-GM pushed a commit to redwoodjs/redwood that referenced this pull request Jul 21, 2024
[![Mend
Renovate](https://app.renovatebot.com/images/banner.svg)](https://renovatebot.com)

This PR contains the following updates:

| Package | Change | Age | Adoption | Passing | Confidence |
|---|---|---|---|---|---|
| [pino](https://getpino.io)
([source](https://togithub.com/pinojs/pino)) | [`8.21.0` ->
`9.3.1`](https://renovatebot.com/diffs/npm/pino/8.21.0/9.3.1) |
[![age](https://developer.mend.io/api/mc/badges/age/npm/pino/9.3.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![adoption](https://developer.mend.io/api/mc/badges/adoption/npm/pino/9.3.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![passing](https://developer.mend.io/api/mc/badges/compatibility/npm/pino/8.21.0/9.3.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|
[![confidence](https://developer.mend.io/api/mc/badges/confidence/npm/pino/8.21.0/9.3.1?slim=true)](https://docs.renovatebot.com/merge-confidence/)
|

---

### Release Notes

<details>
<summary>pinojs/pino (pino)</summary>

### [`v9.3.1`](https://togithub.com/pinojs/pino/releases/tag/v9.3.1)

[Compare
Source](https://togithub.com/pinojs/pino/compare/v9.3.0...v9.3.1)

**Full Changelog**:
pinojs/pino@v9.3.0...v9.3.1

###
[`v9.3.0`](https://togithub.com/pinojs/pino/compare/v9.2.0...92f2cee98e83d3f864b228a6e1dc29a31e54ba49)

[Compare
Source](https://togithub.com/pinojs/pino/compare/v9.2.0...v9.3.0)

### [`v9.2.0`](https://togithub.com/pinojs/pino/releases/tag/v9.2.0)

[Compare
Source](https://togithub.com/pinojs/pino/compare/v9.1.0...v9.2.0)

#### What's Changed

- shallow clone target options by
[@&#8203;mcollina](https://togithub.com/mcollina) in
[pinojs/pino#1973
- add logger instance as third parameter by
[@&#8203;emmyakin](https://togithub.com/emmyakin) in
[pinojs/pino#1977
- use boolean flag to ensure serializers are applied once by
[@&#8203;emmyakin](https://togithub.com/emmyakin) in
[pinojs/pino#1976
- suppport messageKey in pino/browser by
[@&#8203;emmyakin](https://togithub.com/emmyakin) in
[pinojs/pino#1980
- Fixing browser side child log issue
([#&#8203;960](https://togithub.com/pinojs/pino/issues/960)) child level
can now be set at cr… by
[@&#8203;stevel032](https://togithub.com/stevel032) in
[pinojs/pino#1986

#### New Contributors

- [@&#8203;stevel032](https://togithub.com/stevel032) made their first
contribution in
[pinojs/pino#1986

**Full Changelog**:
pinojs/pino@v9.1.0...v9.2.0

### [`v9.1.0`](https://togithub.com/pinojs/pino/releases/tag/v9.1.0)

[Compare
Source](https://togithub.com/pinojs/pino/compare/v9.0.0...v9.1.0)

#### What's Changed

- fix(transport-stream): Fix import error when using pkg with node v20
by [@&#8203;nagyszabi](https://togithub.com/nagyszabi) in
[pinojs/pino#1949
- Update LTS doc by [@&#8203;jsumners](https://togithub.com/jsumners) in
[pinojs/pino#1955
- Update pino types for browser.formatters by
[@&#8203;KatelynKim](https://togithub.com/KatelynKim) in
[pinojs/pino#1946
- add node v22 to CI by
[@&#8203;mcollina](https://togithub.com/mcollina) in
[pinojs/pino#1953
- Add Platformatic to sponsors by
[@&#8203;mcollina](https://togithub.com/mcollina) in
[pinojs/pino#1956
- Update h3 example by [@&#8203;mcollina](https://togithub.com/mcollina)
in
[pinojs/pino#1968
- Support file URLs when configuring multiple transports by
[@&#8203;haines](https://togithub.com/haines) in
[pinojs/pino#1961
- Adding support for mix\&match pipelines by
[@&#8203;dbacarel](https://togithub.com/dbacarel) in
[pinojs/pino#1954
- apply serializers to args once before asObject or transmit by
[@&#8203;emmyakin](https://togithub.com/emmyakin) in
[pinojs/pino#1971
- build(deps-dev): bump pino-pretty from 10.3.1 to 11.0.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[pinojs/pino#1933
- build(deps): bump sonic-boom from 3.8.1 to 4.0.1 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[pinojs/pino#1960
- build(deps): bump pino-std-serializers from 6.2.2 to 7.0.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[pinojs/pino#1957
- build(deps-dev): bump tsd from 0.30.7 to 0.31.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[pinojs/pino#1937
- build(deps): bump actions/dependency-review-action from 3 to 4 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[pinojs/pino#1894
- build(deps): bump pnpm/action-setup from 2.4.0 to 3.0.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[pinojs/pino#1913
- Drop yarn support and update pnpm by
[@&#8203;mcollina](https://togithub.com/mcollina) in
[pinojs/pino#1972
- build(deps): bump thread-stream from 2.7.0 to 3.0.0 by
[@&#8203;dependabot](https://togithub.com/dependabot) in
[pinojs/pino#1959

#### New Contributors

- [@&#8203;nagyszabi](https://togithub.com/nagyszabi) made their first
contribution in
[pinojs/pino#1949
- [@&#8203;haines](https://togithub.com/haines) made their first
contribution in
[pinojs/pino#1961
- [@&#8203;emmyakin](https://togithub.com/emmyakin) made their first
contribution in
[pinojs/pino#1971

**Full Changelog**:
pinojs/pino@v9.0.0...v9.1.0

###
[`v9.0.0`](https://togithub.com/pinojs/pino/compare/v8.21.0...4f8ea32aa69ec94b2fb5561716a7701aec991ce7)

[Compare
Source](https://togithub.com/pinojs/pino/compare/v8.21.0...v9.0.0)

</details>

---

### Configuration

📅 **Schedule**: Branch creation - At any time (no schedule defined),
Automerge - At any time (no schedule defined).

🚦 **Automerge**: Disabled by config. Please merge this manually once you
are satisfied.

♻ **Rebasing**: Whenever PR becomes conflicted, or you tick the
rebase/retry checkbox.

🔕 **Ignore**: Close this PR and you won't be reminded about this update
again.

---

- [ ] <!-- rebase-check -->If you want to rebase/retry this PR, check
this box

---

This PR has been generated by [Mend
Renovate](https://www.mend.io/free-developer-tools/renovate/). View
repository job log
[here](https://developer.mend.io/github/redwoodjs/redwood).

<!--renovate-debug:eyJjcmVhdGVkSW5WZXIiOiIzNy40MzEuNCIsInVwZGF0ZWRJblZlciI6IjM3LjQzMS40IiwidGFyZ2V0QnJhbmNoIjoibWFpbiIsImxhYmVscyI6W119-->

Co-authored-by: renovate[bot] <29139614+renovate[bot]@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants