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

Updated references of logback.groovy to logback-spring.xml #936

Open
wants to merge 3 commits into
base: 7.0.x
Choose a base branch
from

Conversation

@@ -1,4 +1,4 @@
Since Grails 3.0, logging is handled by the http://logback.qos.ch[Logback logging framework] and can be configured with the `grails-app/conf/logback.xml` file.
Since Grails 3.0, logging is handled by the http://logback.qos.ch[Logback logging framework] and can be configured with the `grails-app/conf/logback-spring.xml` file.
Copy link
Contributor

Choose a reason for hiding this comment

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

Should we mention that logback.xml still works?
For example: '... and can be configured with the grails-app/conf/logback-spring.xml or grails-app/conf/logback.xml files.'

Copy link
Author

@gsartori gsartori Dec 20, 2024

Choose a reason for hiding this comment

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

If we are moving to logback-spring.xml my opinion is to not document it. logback.xml will work for applications that are ported to Grails 7, but I would not make it an "official" feature documenting it.

Copy link
Author

Choose a reason for hiding this comment

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

I've documented logback-spring.xml with a link to the Spring documentation for all the other options. Let me know if that convinces you

@@ -1,4 +1,4 @@
Since Grails 3.0, logging is handled by the http://logback.qos.ch[Logback logging framework] and can be configured with the `grails-app/conf/logback.xml` file.
Since Grails 3.0, logging is handled by the http://logback.qos.ch[Logback logging framework] and can be configured with the `grails-app/conf/logback-spring.xml` file.

NOTE: Since Grails 5.1.2 support for groovy configuration (`grails-app/conf/logback.groovy`) has been removed (by logback 1.2.9). It is possible to add back groovy configuration by adding the https://github.com/virtualdogbert/logback-groovy-config[logback-groovy-config] library to your project.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we need this library updated to Groovy 4 in order to make this claim for Grails 7.

Copy link
Author

Choose a reason for hiding this comment

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

We may wait to see if something moves here:
virtualdogbert/logback-groovy-config#15

Copy link
Contributor

Choose a reason for hiding this comment

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

And we can have grails-forge generate an exclude to make it work with 7, in the interim.

@@ -276,7 +276,7 @@ Although the link:../ref/Command%20Line/create-plugin.html[create-plugin] comman
* `grails-app/build.gradle` (although it is used to generate `dependencies.groovy`)
* `grails-app/conf/application.yml` (renamed to plugin.yml)
* `grails-app/conf/spring/resources.groovy`
* `grails-app/conf/logback.groovy`
* `grails-app/conf/logback-spring.xml`
Copy link
Contributor

Choose a reason for hiding this comment

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

logback.xml and application.groovy are also be excluded

Copy link
Author

Choose a reason for hiding this comment

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

To your knowledge will application.groovy be renamed to plugin.groovy as well?

Copy link
Author

Choose a reason for hiding this comment

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

Also, my opinion is to not document logback.xml if we decided to go with logback-spring.xml

Copy link
Contributor

Choose a reason for hiding this comment

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

To your knowledge will application.groovy be renamed to plugin.groovy as well?

To my knowledge, it does not get automatically renamed. That comment is a bit cryptic. To my knowledge, if a plugin author wants config from the plugin to be included in consuming applications, he/she should put it in a file named plugin.yml.

Also, my opinion is to not document logback.xml if we decided to go with logback-spring.xml

My opinion is to document both, for the time being. Otherwise, people might think they have to switch.

Copy link
Author

Choose a reason for hiding this comment

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

Cheking the Spring Boot docs, both names can be used (preferring logback-spring.yml) https://docs.spring.io/spring-boot/reference/features/logging.html#features.logging.custom-log-configuration

Why not just documenting logback-spring.xml adding a link to the Spring documentation letting people see all the quirks and quarks of configuring the logs?

On a side note, I just found out that we can also configure the logs directly in the application.yml it may be a good option so we can remove the logback-spring.xml file altogether (I'll give this a try and maybe open a feature request).

- `grails-app/conf/plugin.yml`
- `grails-app/conf/plugin.groovy`

These files are the equivalent of `application.yml` or `application.groovy` in a Grails application.
Copy link
Contributor

Choose a reason for hiding this comment

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

@gsartori This is not entirely correct. You can have an application.yml or application.groovy in a plugin project. These configs will just not make it into the consuming application. Examples of config settings that could be in a plugins application.yml file:

grails:
    profile: web-plugin
    codegen:
        defaultPackage: my.groovy.plugin

Copy link
Author

Choose a reason for hiding this comment

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

What about:

A Grails plugin can be configured in one of the following files:

  • grails-app/conf/plugin.yml
  • grails-app/conf/plugin.groovy

These files will be included in the plugin package while the application.yml file is used when the plugin is run as an application. See Notes on excluded Artefacts.


NOTE: Since Grails 5.1.2 support for groovy configuration (`grails-app/conf/logback.groovy`) has been removed (by logback 1.2.9). It is possible to add back groovy configuration by adding the https://github.com/virtualdogbert/logback-groovy-config[logback-groovy-config] library to your project.
- See the https://docs.spring.io/spring-boot/reference/features/logging.html[Spring Boot Logging] for all the available configuration options.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should use the following links, since they go directly to the logback based configuration that we use.

https://docs.spring.io/spring-boot/how-to/logging.html
https://docs.spring.io/spring-boot/reference/features/logging.html#features.logging.logback-extensions

@@ -4,18 +4,18 @@ If you set the configuration property `logging.config`, you can instruct `Logbac
.grails-app/conf/application.yml
----
logging:
config: /Users/me/config/logback.groovy
config: /Users/me/config/logback.xml
Copy link
Contributor

Choose a reason for hiding this comment

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

Should these 3 be logback-spring.xml?

//
// NOTE: Since Grails 5.1.2 support for groovy configuration (`grails-app/conf/logback.groovy`) has been removed (by logback 1.2.9). It is possible to add back groovy configuration by adding the https://github.com/virtualdogbert/logback-groovy-config[logback-groovy-config] library to your project.

Copy link
Contributor

Choose a reason for hiding this comment

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

I think it would be beneficial to include an example logback-spring.xml file in this section and explain <springProfile name="development">, since that is the main advantage.

<?xml version="1.0" encoding="UTF-8"?>
<configuration>
    <include resource="org/springframework/boot/logging/logback/defaults.xml"/>
    <include resource="org/springframework/boot/logging/logback/console-appender.xml"/>
    <root level="ERROR">
        <appender-ref ref="CONSOLE"/>
    </root>

    <springProfile name="development">
        <logger name="StackTrace" level="ERROR" additivity="false"/>
        <logger name="your.package" level="DEBUG"/>
        <logger name="org.hibernate.orm.deprecation" level="OFF"/>
        <root level="WARN"/>
    </springProfile>
</configuration>

Copy link
Contributor

@jamesfredley jamesfredley left a comment

Choose a reason for hiding this comment

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

Thank you, I added a few things in comments.

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