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

Non-breaking structured logging implementation #1962

Open
hugo-vrijswijk opened this issue Sep 28, 2023 · 2 comments
Open

Non-breaking structured logging implementation #1962

hugo-vrijswijk opened this issue Sep 28, 2023 · 2 comments

Comments

@hugo-vrijswijk
Copy link
Contributor

hugo-vrijswijk commented Sep 28, 2023

The structured logging from #1938 is very cool. But it also introduces a breaking change with the extra context parameters compared to sttp.client3, which does not have the context. This is a breaking change for anyone implementing their own logger (which is pretty common for wrapping sttp logging into your own logger). IMO, migrating from sttp 3 to 4 should be as easy as possible, so I think it would be a good idea to avoid source-breaking without good reason.

Some options, maybe:

  • Add an apply with the 'old' signature, deprecate it (potentially) and give it a default implementation of calling apply with empty context. This way, users can choose when to migrate to using context for the logger
  • Add a 'contextless' logger next to the structured logger, and add a type alias of sttp.client4.logging.Logger to the contextless logger. The type alias could be deprecated. But it could be beneficial to keep as a 'simple' logger for implementations that don't have the concept of context
    • What I'm imagining is some sealed trait of BaseLogger and a ContextlessLogger/StructuredLogger extends BaseLogger
@adamw
Copy link
Member

adamw commented Sep 29, 2023

I agree that it's best to minimize necessary code changes. But on the other hand, when's a good time to introduce them, if not in a major release? I'm not so sure about adding deprecated members in a major release, which will have to essentially stay there forever (unless we do a sttp 5 ;) )

@adamw
Copy link
Member

adamw commented Sep 29, 2023

(that said, I'm open for other voices for the community, if this indeed would be important to a larger group of people, let's introduce the deprecated members)

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

No branches or pull requests

2 participants