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

Load the correct logback file for the http json service respecting the deployment situation #9938

Merged
merged 4 commits into from
Jun 9, 2021
Merged

Load the correct logback file for the http json service respecting the deployment situation #9938

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
c3bfee5
Load the correct logback file for the http json service respecting th…
realvictorprm Jun 9, 2021
aeab224
Move import statement, remove some braces and reformat
realvictorprm Jun 9, 2021
e795cfc
Move system prop dependend logic to cliopts Logging.scala
realvictorprm Jun 9, 2021
2b830ae
Remove PathKind type in cliopts Logging.scala as it's not necessary a…
realvictorprm Jun 9, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
29 changes: 21 additions & 8 deletions ledger-service/cli-opts/src/main/scala/cliopts/Logging.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,29 +7,42 @@ import ch.qos.logback.classic.{Level => LogLevel}

object Logging {

def reconfigure(clazz: Class[_], pathToLogbackFileInJarFile: String): Unit = {
sealed trait PathKind
object PathKind {
final case class OutsideOfJar(path: String) extends PathKind
final case class InsideOfJar(path: String) extends PathKind
}

def reconfigure(clazz: Class[_], path: PathKind): Unit = {
// Try reconfiguring the library
import ch.qos.logback.core.joran.spi.JoranException
import ch.qos.logback.classic.LoggerContext
import ch.qos.logback.classic.joran.JoranConfigurator
import org.slf4j.LoggerFactory
import scala.util.Using
Using.resource(clazz.getClassLoader.getResource(pathToLogbackFileInJarFile).openStream()) {
stream =>
import java.io.InputStream
import java.io.FileInputStream
def reloadConfig(stream: => InputStream): Unit =
Using.resource(stream) { stream =>
try {
val context = LoggerFactory.getILoggerFactory.asInstanceOf[LoggerContext]
val configurator = new JoranConfigurator
val configurator = new JoranConfigurator()
configurator.setContext(context)
context.reset()
configurator.doConfigure(stream)
} catch {
case je: JoranException =>
// Fallback to System.err.println because the logger won't work in any way anymore.
System.err.println(s"reconfigured failed using url $pathToLogbackFileInJarFile: $je")
System.err.println(
s"reconfigured failed using url $path: $je"
)
je.printStackTrace(System.err)
} finally {
stream.close()
}
} finally stream.close()
}
path match {
realvictorprm marked this conversation as resolved.
Show resolved Hide resolved
case PathKind.OutsideOfJar(path) => reloadConfig(new FileInputStream(path))
case PathKind.InsideOfJar(pathToLogbackFileInJarFile) =>
reloadConfig(clazz.getClassLoader.getResource(pathToLogbackFileInJarFile).openStream())
}
}

Expand Down
9 changes: 7 additions & 2 deletions ledger-service/http-json/src/main/scala/com/digitalasset/http/Main.scala
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ import java.nio.file.Path
import akka.actor.ActorSystem
import akka.http.scaladsl.Http.ServerBinding
import akka.stream.Materializer
import com.daml.cliopts.Logging.LogEncoder
import com.daml.cliopts.Logging.{LogEncoder, PathKind}
import com.daml.grpc.adapter.{AkkaExecutionSequencerPool, ExecutionSequencerFactory}
import com.daml.runtime.JdbcDrivers
import com.daml.scalautil.Statement.discard
Expand Down Expand Up @@ -46,7 +46,12 @@ object Main {
case LogEncoder.Plain => () // This is the default
case LogEncoder.Json =>
Logging.setUseJsonLogEncoderSystemProp()
Logging.reconfigure(getClass, "logback.xml")
val path: PathKind =
Copy link
Contributor

Choose a reason for hiding this comment

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

The fact that you only do this in the JSON API seems a bit worrying. We handle logback files the same for all components. Should we move the logic into reconfigure?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That was my thought too, but so far I see only the json api does this because no other service spits out json logs yet if I remember correctly.

Considering that the system prop is named same for every other service I think this should be safe to move there 🤔

Copy link
Contributor

Choose a reason for hiding this comment

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

I’m hoping that the fact that no other sevrice supports this is a temporary state and eventually everything produces json logs :)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

ok adressed with e795cfc

fine for you @cocreature ?

System.getProperty("logback.configurationFile") match {
case null => PathKind.InsideOfJar("logback.xml")
case path => PathKind.OutsideOfJar(path)
}
Logging.reconfigure(getClass, path)
}
// Here we set all things which are related to logging but not to
// any env vars in the logback.xml file.
Expand Down