zap icon indicating copy to clipboard operation
zap copied to clipboard

Allow retrieval of previously added fields from the Core

Open adrianlungu opened this issue 7 months ago • 3 comments

As per the feature requested opened here:

  • extended the Core interface with Fields() which can be used to retrieve fields that have been previously added to the core
  • updated the ioCore.With to store the fields in an internal private array
  • updated the ioCore.Clone to clone the fields as well
  • added unit tests to cover these cases
  • updated the types implementing Core to also have Fields()

adrianlungu avatar Dec 13 '23 00:12 adrianlungu

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.


Adrian Lungu seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
You have signed the CLA already but the status is still pending? Let us recheck it.

CLAassistant avatar Dec 13 '23 00:12 CLAassistant

Hey @adrianlungu, thanks for the PR.

As it stands currently, this is a breaking change. Adding a method to an exported interface will cause all custom implementations to no longer satisfy the interface.

I'm also curious about your use case. Usually I see observer used in-place of a normal logger, passed into some functionality where you want to test what messages get logged. I don't usually see it used in combination with another logger. If you want to test what fields get added to a logger, is there any way you can pass an observer before the fields get added?

Maybe you can create a separate implementation of Core that wraps an underlying core but keeps track of the fields you add (similar to what this PR is having ioCore do), and use that in your code instead of baking it into ioCore by default.

JacobOaks avatar Dec 13 '23 16:12 JacobOaks

Hello @JacobOaks , thank you for the review and insight!

I have a logger built on top of zap which has some mandatory fields that should be present, populated from the environment, but the logger can also be customized with additional fields via context or options.

While trying to write a unit test to assert that the required fields are there, and then that the optional fields are there, I stumbled into the Context fields which does not return the fields if they are initialized via the InitialFields property, or if they are added to the core before I get a chance to inject the Observable core.

Using a config Build() and wrapping the core in there also doesn't help because the WrapCore func would be called after the fields would be added to the core so they would not be seen by the observer core.

adrianlungu avatar Dec 14 '23 09:12 adrianlungu