Rename logging.file to logging.file.name

[TwiN]

As discussed in #12510 with @philwebb:

---

Using properties configuration we can use this:

logging.file=logs/app.log
logging.file.max-history=20
logging.file.max-size=50MB

However, using YAML, we cannot set those at the same time without using the following workaround:

logging.file: logs/app.log
logging:
  file:
    max-history: 20
    max-size: 50MB 

---

Thus, to prevent this specific issue from happening with logging.file, I suggest logging.file should be renamed to logging.file.name.

Furthermore, I modified additional-spring-configuration-metadata.json accordingly:

The new property entry:

    {
      "name": "logging.file.name",
      "type": "java.lang.String",
      "description": "Log file name (for instance, `myapp.log`). Names can be an exact location or relative to the current directory.",
      "sourceType": "org.springframework.boot.context.logging.LoggingApplicationListener"
    }

And to notify users of the breaking change:

    {
      "name": "logging.file",
      "type": "java.lang.String",
      "description": "Log file name (for instance, `myapp.log`). Names can be an exact location or relative to the current directory.",
      "sourceType": "org.springframework.boot.context.logging.LoggingApplicationListener",
      "deprecation": {
        "replacement": "logging.file.name",
        "level": "error"
      }
    }

This (partially) fixes #12510

[snicoll]

Thank you very much for opening your first PR to Spring Boot. This looks good and very complete. I am just wondering if we want to have a deprecation phase (i.e. still honouring the former property in a deprecated fashion in 2.2, removing it altogether in the next feature release).

Flagging for team attention to see what the rest of the team thinks.

[wilkinsona]

Thanks for the PR, @TwinProduction.

While this will fix the problem with the YAML structure, I don't think we can change logging.file and leave logging.path. logging.path is also specific to file logging so I think it should also move under logging.file to logging.file.path.

[TwiN]

@wilkinsona Good point, when I get home, I'll make the change.

[TwiN]

Fixed @wilkinsona

[philwebb]

I am just wondering if we want to have a deprecation phase

+1

[philwebb]

Looks like we have agreement that a deprecation phase would be useful. @TwinProduction Are you interested in also trying to add that to the existing PR?

[TwiN]

Looks like we have agreement that a deprecation phase would be useful. @TwinProduction Are you interested in also trying to add that to the existing PR?

Sure, I'll give it a shot.

[TwiN]

@philwebb I've looked around for a bit, and while I could certainly keeping looking for a while, I think it'll be more time efficient if I just ask you directly instead.

At a glance, it seems like there's already a property migration implementation that should take care of every properties in the META-INF/additional-spring-configuration-metadata.json files with the following:

"deprecation": {
        "replacement": "..."
}

If this is not the case, then can you give me an example of a property that went through a deprecation phase? The only ones I could find after a quick search didn't go through a deprecation phase, such as banner.charset which was renamed to spring.banner.charset, though I might be wrong.

Alternatively, do you simply want me to just hard code it somewhere? I mean it's certainly easy to add an hard coded block that reads the properties logging.file as well as logging.path and copy them to logging.file.name and logging.file.path respectively, but I have a feeling there's already a deprecation system in place, in which case, can you point me to the related classes?

Thanks.

[snicoll]

@TwinProduction the properties migration is yet another thing that we do to help users but we've decided to deprecate this property in 2.2 and remove it in the feature release.

Concretely, it means that every place we are looking for logging.file.name in your current PR, we should look for logging.file if the former is not set. The property should be flagged as deprecated (with the default error level).

We should have tests that set the deprecated property and assert the feature still work as expected. Those tests should be flagged as @Deprecated so that we get a reminder when the next feature release opens.

If you have more questions, please head to Gitter and we can chat. Thanks!

[TwiN]

Here I changed logging.file.name for LogFile.DEPRECATED_FILE_PROPERTY because that field is annotated with @Deprecated, which will make it easier to find and remove the deprecated code caused by this PR when the deprecation phase is over.

[TwiN]

@philwebb @wilkinsona @snicoll What do you think?

[wilkinsona]

Thanks for the updates. I left a few comments regarding the constants for the property names. Could you take a look and, if you agree, update your proposal accordingly?

[TwiN]

@wilkinsona Updated accordingly, let me know what you think.

[wilkinsona]

This looks great now. Thank you, @TwinProduction. We'll merge this once we've created the 2.1.x branch and master is building 2.2.0 snapshots.

[snicoll]

@TwinProduction thank you very much for making your first contribution to Spring Boot. I've squashed and polished your contribution on master.