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.

Sign up for GitHub

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

Jump to bottom

Register logging system's shutdown hook by default #25046

Closed
philwebb opened this issue Jan 29, 2021 · 3 comments
Closed

Register logging system's shutdown hook by default #25046

philwebb opened this issue Jan 29, 2021 · 3 comments
Labels
type: enhancement A general enhancement
Milestone
2.5.0-M3

Comments

@philwebb
Copy link
Member

The discussion in #24507 makes me think that we should consider enabling the property by default. Whilst the arguments in #4026 are still valid, they aren't really typical Spring Boot deployments.
@philwebb philwebb added type: task A general task for: team-attention An issue we'd like other members of the team to review labels Jan 29, 2021
@philwebb philwebb added this to the 2.5.x milestone Jan 29, 2021
@wilkinsona
Copy link
Member

I think it makes sense to flip the default now.

@philwebb philwebb removed the for: team-attention An issue we'd like other members of the team to review label Feb 22, 2021
@wilkinsona wilkinsona changed the title Consider switching logging.register-shutdown-hook default Register logging system's shutdown hook by default Feb 24, 2021
@wilkinsona wilkinsona added type: enhancement A general enhancement and removed type: task A general task labels Feb 24, 2021
@wilkinsona
Copy link
Member

wilkinsona commented Feb 24, 2021 •
edited

The changes in this branch flip the default. While working on them, it occurred to me that it'd be nice to disable it automatically in a war deployment. We already disable the application's shutdown hook in SpringBootServletInitializer and this feels similar. It's tricky to pass the decision from the initialiser into the logging application listener. Perhaps we could add a lowest precedence property source to the environment to leave it disabled by default?

@wilkinsona wilkinsona added the for: team-meeting An issue we'd like to discuss as a team to make progress label Feb 24, 2021
@philwebb philwebb removed the for: team-meeting An issue we'd like to discuss as a team to make progress label Feb 24, 2021
@wilkinsona
Copy link
Member

We're going to try setting something via the servlet context in the web application case to disable the shutdown hook.

@wilkinsona wilkinsona modified the milestones: 2.5.x, 2.5.0-M3 Feb 26, 2021
@philwebb philwebb mentioned this issue May 27, 2021
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
type: enhancement A general enhancement
Projects
None yet
Milestone
2.5.0-M3
Development

No branches or pull requests
2 participants
@philwebb @wilkinsona