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

Lagom Scala dev mode does not detect custom logback.xml #534

Closed
TimMoore opened this Issue Feb 23, 2017 · 7 comments

Comments

Projects
None yet
4 participants
@TimMoore
Member

TimMoore commented Feb 23, 2017

Lagom Version

1.3.0

API

Scala

Expected Behavior

Please describe the expected behavior of the issue, starting from the first action.

  1. Create a project with sbt new lagom/lagom-scala.g8
  2. Add a custom logback.xml to hello-impl/src/main/resources with some obvious configuration change (e.g., change the filename as described in the workaround for #533).
  3. sbt runAll
  4. The hello-impl service should use the custom log configuration.

Actual Behavior

The default development log configuration is used instead.

Background

Lagom's LogbackLoggerConfigurator is invoked automatically by Play's GuiceApplicationBuilder when starting a Java service.

In the Scala API, we use a custom application loader, so Play does not automatically configure our logging configurator. This is documented at https://www.playframework.com/documentation/2.5.x/SettingsLogger#Using-a-custom-application-loader

Workaround

You can adapt the instructions from the Play documentation linked above by configuring logging in your service loader's loadDevMode method.

For example:

class HelloLoader extends LagomApplicationLoader {
  // ...
  override def loadDevMode(context: LagomApplicationContext): LagomApplication = {
    val environment = context.playContext.environment
    LoggerConfigurator(environment.classLoader).foreach {
      _.configure(environment)
    }
    new HelloApplication(context) with LagomDevModeComponents
  }
  // ...
}
@yannickdeturck

This comment has been minimized.

Show comment
Hide comment
@yannickdeturck

yannickdeturck May 26, 2017

Contributor

The workaround with adding the above code snippet seems to work but, something to take note of is that when generating a new Lagom project, the default root level seems to be set to WARN whereas the root level in logback-lagom-dev.xml is set to INFO which may make it look like adding the above code snippet broke your logging if all your log statements are set to INFO.

Contributor

yannickdeturck commented May 26, 2017

The workaround with adding the above code snippet seems to work but, something to take note of is that when generating a new Lagom project, the default root level seems to be set to WARN whereas the root level in logback-lagom-dev.xml is set to INFO which may make it look like adding the above code snippet broke your logging if all your log statements are set to INFO.

@erip

This comment has been minimized.

Show comment
Hide comment
@erip

erip Jun 21, 2017

Contributor

Related to #36?

Contributor

erip commented Jun 21, 2017

Related to #36?

guizmaii added a commit to guizmaii/lagom that referenced this issue Jul 18, 2017

@guizmaii guizmaii referenced this issue Jul 18, 2017

Merged

Fix https://github.com/lagom/lagom/issues/534 #889

6 of 6 tasks complete

renatocaval pushed a commit that referenced this issue Jul 18, 2017

guizmaii added a commit to guizmaii/lagom that referenced this issue Jul 18, 2017

Format code with scalariform
Fix lagom#534 (lagom#889)

Format code with scalariform

renatocaval pushed a commit that referenced this issue Jul 27, 2017

Refactor/deprecate play config (#881)
* bump sbt version to 0.13.15 and remove deprecated syntax.

* deprecate public methods and fields using play.Configuration or play.api.Configuration in lieu of preferred com.typesafe.config.Config.

* fixed maven plugun RunMojo (#886)

* Fix #534 (#889)

* Format code with scalariform (#890)

* Celan whitelist (#868)

* Expose CircuitBreakers as a public API (#841)

* fixes compilation

* fixes compilation.

* fixes compilation error due to bad merge

* now fixing the tests

* removes commented out `BuiltInComponents`

* removes obsolete `BuiltInComponents` import

* Reindent ConfigurationServiceLocator

* Clean up some unused imports

* added some extra @deprecated annotations  + removed @link inside it
@lexn82

This comment has been minimized.

Show comment
Hide comment
@lexn82

lexn82 Dec 6, 2017

The issue still persist when testing, even after the fix. The test setup described in the lagom documentation does not call the load method used to configure logging.

   "say hello" in ServiceTest.withServer(ServiceTest.defaultSetup) { ctx =>
      new HelloApplication(ctx) with LocalServiceLocator
    } { server => ... }

lexn82 commented Dec 6, 2017

The issue still persist when testing, even after the fix. The test setup described in the lagom documentation does not call the load method used to configure logging.

   "say hello" in ServiceTest.withServer(ServiceTest.defaultSetup) { ctx =>
      new HelloApplication(ctx) with LocalServiceLocator
    } { server => ... }
@erip

This comment has been minimized.

Show comment
Hide comment
@erip

erip Dec 6, 2017

Contributor

@lexn82 Agreed. In fact, I find that only the first test logs. Do you experience this, too?

Contributor

erip commented Dec 6, 2017

@lexn82 Agreed. In fact, I find that only the first test logs. Do you experience this, too?

@lexn82

This comment has been minimized.

Show comment
Hide comment
@lexn82

lexn82 Dec 6, 2017

@erip I see logging for some initialization code, then nothing during tests.

lexn82 commented Dec 6, 2017

@erip I see logging for some initialization code, then nothing during tests.

@TimMoore

This comment has been minimized.

Show comment
Hide comment
@TimMoore

TimMoore Dec 12, 2017

Member

@lexn82 / @erip could one of you open a new issue with details please?

Member

TimMoore commented Dec 12, 2017

@lexn82 / @erip could one of you open a new issue with details please?

@erip

This comment has been minimized.

Show comment
Hide comment
@erip

erip Dec 12, 2017

Contributor

@TimMoore I can try to do that today.

Contributor

erip commented Dec 12, 2017

@TimMoore I can try to do that today.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment