Skip to content

Commit

Permalink
Replace NamedLoggerFactory (digital-asset#4097)
Browse files Browse the repository at this point in the history 
* Replace NamedLoggerFactory

CHANGELOG_BEGIN
CHANGELOG_END

* Recover change lost in rebase

* Address digital-asset#4045 (comment)

* Address digital-asset#4045 (comment)

* Address two open review comments

Address digital-asset#4045 (comment)
Address digital-asset#4045 (comment)

* Address outstanding compilation errors

* Replace mocking with in-memory log collector

* Address digital-asset#4097 (review)

* Address digital-asset#4045 (comment)

The generation of code to have logging in the services has been replaced
by helpers classes. This will allow to enrich the context received at
construction by the service implementations.

* Use ContextualizedLogger for TrackerMap

* Remove deleted logging packages from artifacts.yaml

* Remove remaining deleted artifact from artifacts.yaml

* Address digital-asset#4097 (comment)

* Address digital-asset#4097 (comment)

* Address digital-asset#4097 (comment)

* Address digital-asset#4097 (comment)

* Annotate type of references to logErrorOnCall

* Address digital-asset#4097 (comment)
  • Loading branch information
@stefanobaghino-da
stefanobaghino-da authored Jan 21, 2020
1 parent b0ade66 commit 1ad7d6c
Show file tree
Hide file tree
Showing 71 changed files with 795 additions and 1,963 deletions.
2 changes: 1 addition & 1 deletion ledger/api-server-damlonx/reference-v2/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -76,10 +76,10 @@ da_scala_test(
"//ledger/participant-state/kvutils",
"//ledger/participant-state/kvutils:kvutils-tests-lib",
"//ledger/sandbox",
"//ledger/test-common",
"//libs-scala/resources",
"//libs-scala/timer-utils",
"@maven//:ch_qos_logback_logback_classic",
"@maven//:ch_qos_logback_logback_core",
"@maven//:com_typesafe_akka_akka_actor_2_12",
"@maven//:com_typesafe_akka_akka_stream_2_12",
"@maven//:io_dropwizard_metrics_metrics_core",
Expand Down
106 changes: 54 additions & 52 deletions ...e-v2/src/main/scala/com/daml/ledger/api/server/damlonx/reference/v2/ReferenceServer.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,9 @@ import com.digitalasset.daml.lf.archive.DarReader
import com.digitalasset.daml_lf_dev.DamlLf.Archive
import com.digitalasset.ledger.api.auth.{AuthService, AuthServiceWildcard}
import com.digitalasset.platform.apiserver.{ApiServerConfig, StandaloneApiServer}
import com.digitalasset.platform.common.logging.NamedLoggerFactory
import com.digitalasset.platform.indexer.{IndexerConfig, StandaloneIndexerServer}
import com.digitalasset.platform.logging.LoggingContext
import com.digitalasset.platform.logging.LoggingContext.newLoggingContext
import com.digitalasset.resources.akka.AkkaResourceOwner
import com.digitalasset.resources.{Resource, ResourceOwner}
import org.slf4j.LoggerFactory
Expand All @@ -40,55 +41,57 @@ object ReferenceServer extends App {
implicit val materializer: Materializer = Materializer(system)
implicit val executionContext: ExecutionContext = system.dispatcher

val resource = for {
// Take ownership of the actor system and materializer so they're cleaned up properly.
// This is necessary because we can't declare them as implicits within a `for` comprehension.
_ <- AkkaResourceOwner.forActorSystem(() => system).acquire()
_ <- AkkaResourceOwner.forMaterializer(() => materializer).acquire()
ledger <- ResourceOwner
.forCloseable(() => new InMemoryKVParticipantState(config.participantId))
.acquire()
_ <- Resource.sequenceIgnoringValues(config.archiveFiles.map { file =>
val submissionId = SubmissionId.assertFromString(UUID.randomUUID().toString)
for {
dar <- ResourceOwner
.forTry(() =>
DarReader { case (_, x) => Try(Archive.parseFrom(x)) }
.readArchiveFromFile(file))
.acquire()
_ <- ResourceOwner
.forCompletionStage(() => ledger.uploadPackages(submissionId, dar.all, None))
.acquire()
} yield ()
})
_ <- startIndexerServer(config, readService = ledger)
_ <- startApiServer(
config,
readService = ledger,
writeService = ledger,
authService = AuthServiceWildcard,
)
_ <- Resource.sequenceIgnoringValues(
for {
(extraParticipantId, port, jdbcUrl) <- config.extraParticipants
} yield {
val participantConfig = config.copy(
port = port,
participantId = extraParticipantId,
jdbcUrl = jdbcUrl,
)
val resource = newLoggingContext { implicit logCtx =>
for {
// Take ownership of the actor system and materializer so they're cleaned up properly.
// This is necessary because we can't declare them as implicits within a `for` comprehension.
_ <- AkkaResourceOwner.forActorSystem(() => system).acquire()
_ <- AkkaResourceOwner.forMaterializer(() => materializer).acquire()
ledger <- ResourceOwner
.forCloseable(() => new InMemoryKVParticipantState(config.participantId))
.acquire()
_ <- Resource.sequenceIgnoringValues(config.archiveFiles.map { file =>
val submissionId = SubmissionId.assertFromString(UUID.randomUUID().toString)
for {
_ <- startIndexerServer(participantConfig, readService = ledger)
_ <- startApiServer(
participantConfig,
readService = ledger,
writeService = ledger,
authService = AuthServiceWildcard,
)
dar <- ResourceOwner
.forTry(() =>
DarReader { case (_, x) => Try(Archive.parseFrom(x)) }
.readArchiveFromFile(file))
.acquire()
_ <- ResourceOwner
.forCompletionStage(() => ledger.uploadPackages(submissionId, dar.all, None))
.acquire()
} yield ()
}
)
} yield ()
})
_ <- startIndexerServer(config, readService = ledger)
_ <- startApiServer(
config,
readService = ledger,
writeService = ledger,
authService = AuthServiceWildcard,
)
_ <- Resource.sequenceIgnoringValues(
for {
(extraParticipantId, port, jdbcUrl) <- config.extraParticipants
} yield {
val participantConfig = config.copy(
port = port,
participantId = extraParticipantId,
jdbcUrl = jdbcUrl,
)
for {
_ <- startIndexerServer(participantConfig, readService = ledger)
_ <- startApiServer(
participantConfig,
readService = ledger,
writeService = ledger,
authService = AuthServiceWildcard,
)
} yield ()
}
)
} yield ()
}

resource.asFuture.failed.foreach { exception =>
logger.error("Shutting down because of an initialization error.", exception)
Expand All @@ -97,11 +100,11 @@ object ReferenceServer extends App {

Runtime.getRuntime.addShutdownHook(new Thread(() => Await.result(resource.release(), 10.seconds)))

private def startIndexerServer(config: Config, readService: ReadService): Resource[Unit] =
private def startIndexerServer(config: Config, readService: ReadService)(
implicit logCtx: LoggingContext): Resource[Unit] =
new StandaloneIndexerServer(
readService,
IndexerConfig(config.participantId, config.jdbcUrl, config.startupMode),
NamedLoggerFactory.forParticipant(config.participantId),
SharedMetricRegistries.getOrCreate(s"indexer-${config.participantId}"),
).acquire()

Expand All @@ -110,7 +113,7 @@ object ReferenceServer extends App {
readService: ReadService,
writeService: WriteService,
authService: AuthService,
): Resource[Unit] =
)(implicit logCtx: LoggingContext): Resource[Unit] =
new StandaloneApiServer(
ApiServerConfig(
config.participantId,
Expand All @@ -126,7 +129,6 @@ object ReferenceServer extends App {
readService,
writeService,
authService,
NamedLoggerFactory.forParticipant(config.participantId),
SharedMetricRegistries.getOrCreate(s"ledger-api-server-${config.participantId}"),
).acquire()
}
8 changes: 5 additions & 3 deletions ledger/api-server-damlonx/reference-v2/src/test/resources/logback-test.xml
View file
Original file line number Diff line number Diff line change
@@ -1,10 +1,12 @@
<?xml version="1.0" encoding="UTF-8"?>
<configuration>

<appender name="INDEXER" class="com.daml.ledger.api.server.damlonx.reference.v2.IndexerIT$Appender" />
<appender name="MEM" class="com.digitalasset.platform.testing.LogCollector">
<test>com.daml.ledger.api.server.damlonx.reference.v2.IndexerIT</test>
</appender>

<logger name="com.digitalasset.platform.indexer.RecoveringIndexer:IndexerIT" level="INFO">
<appender-ref ref="INDEXER" />
<logger name="com.digitalasset.platform.indexer.RecoveringIndexer" level="INFO">
<appender-ref ref="MEM" />
</logger>

</configuration>
57 changes: 23 additions & 34 deletions ...e-v2/src/test/suite/scala/com/daml/ledger/api/server/damlonx/reference/v2/IndexerIT.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,22 +9,22 @@ import akka.actor.ActorSystem
import akka.stream.Materializer
import akka.stream.scaladsl.Source
import ch.qos.logback.classic.Level
import ch.qos.logback.classic.spi.ILoggingEvent
import ch.qos.logback.core.UnsynchronizedAppenderBase
import com.codahale.metrics.MetricRegistry
import com.daml.ledger.api.server.damlonx.reference.v2.IndexerIT._
import com.daml.ledger.participant.state.v1.Update.Heartbeat
import com.daml.ledger.participant.state.v1._
import com.digitalasset.daml.lf.data.Ref
import com.digitalasset.daml.lf.data.Ref.LedgerString
import com.digitalasset.platform.common.logging.NamedLoggerFactory
import com.digitalasset.platform.indexer.{
IndexerConfig,
IndexerStartupMode,
RecoveringIndexer,
StandaloneIndexerServer
}
import com.digitalasset.platform.logging.LoggingContext
import com.digitalasset.resources.Resource
import com.digitalasset.platform.sandbox.stores.ledger.sql.dao.{JdbcLedgerDao, LedgerDao}
import com.digitalasset.platform.testing.LogCollector
import com.digitalasset.timer.RetryStrategy
import org.mockito.ArgumentMatchers
import org.mockito.Mockito._
Expand All @@ -43,7 +43,7 @@ class IndexerIT extends AsyncWordSpec with Matchers with BeforeAndAfterEach {
super.beforeEach()
actorSystem = ActorSystem(getClass.getSimpleName)
materializer = Materializer(actorSystem)
clearLog()
LogCollector.clear[this.type]
}

override def afterEach(): Unit = {
Expand All @@ -52,6 +52,8 @@ class IndexerIT extends AsyncWordSpec with Matchers with BeforeAndAfterEach {
super.afterEach()
}

private def readLog(): Seq[(Level, String)] = LogCollector.read[this.type, RecoveringIndexer]

"indexer" should {
"index the participant state" in {
val (participantState, server, ledgerDao) =
Expand Down Expand Up @@ -187,22 +189,23 @@ class IndexerIT extends AsyncWordSpec with Matchers with BeforeAndAfterEach {
val participantState = newParticipantState(participantId, ledgerId)
val jdbcUrl =
s"jdbc:h2:mem:${getClass.getSimpleName.toLowerCase()}-$id;db_close_delay=-1;db_close_on_exit=false"
val serverOwner = new StandaloneIndexerServer(
participantState,
IndexerConfig(
participantId,
jdbcUrl,
startupMode = IndexerStartupMode.MigrateAndStart,
restartDelay = restartDelay,
),
loggerFactory,
new MetricRegistry,
)
val ledgerDaoOwner =
JdbcLedgerDao.owner(jdbcUrl, loggerFactory, new MetricRegistry, ExecutionContext.global)
val server = serverOwner.acquire()
val ledgerDao = ledgerDaoOwner.acquire()
(participantState, server, ledgerDao)
LoggingContext.newLoggingContext { implicit logCtx =>
val serverOwner = new StandaloneIndexerServer(
participantState,
IndexerConfig(
participantId,
jdbcUrl,
startupMode = IndexerStartupMode.MigrateAndStart,
restartDelay = restartDelay,
),
new MetricRegistry,
)
val ledgerDaoOwner =
JdbcLedgerDao.owner(jdbcUrl, new MetricRegistry, ExecutionContext.global)
val server = serverOwner.acquire()
val ledgerDao = ledgerDaoOwner.acquire()
(participantState, server, ledgerDao)
}
}
}

Expand All @@ -211,20 +214,6 @@ object IndexerIT {

private val eventually = RetryStrategy.exponentialBackoff(10, 10.millis)

private val loggerFactory = NamedLoggerFactory(classOf[IndexerIT])

private[this] val log = Vector.newBuilder[(Level, String)]

private def readLog(): Seq[(Level, String)] = log.synchronized { log.result() }
private def clearLog(): Unit = log.synchronized { log.clear() }

final class Appender extends UnsynchronizedAppenderBase[ILoggingEvent] {
override def append(e: ILoggingEvent): Unit =
log.synchronized {
val _ = log += e.getLevel -> e.getFormattedMessage
}
}

private def randomSubmissionId(): LedgerString =
LedgerString.assertFromString(UUID.randomUUID().toString)

Expand Down
1 change: 0 additions & 1 deletion ledger/ledger-api-common/BUILD.bazel
View file
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,6 @@ da_scala_library(
"//ledger/ledger-api-akka",
"//ledger/ledger-api-domain",
"//ledger/ledger-api-health",
"//ledger/ledger-api-scala-logging:ledger-api-scala-logging-base",
"//ledger/participant-state",
"//ledger/participant-state-index",
"//libs-scala/direct-execution-context",
Expand Down
13 changes: 4 additions & 9 deletions .../ledger-api-common/src/main/scala/com/digitalasset/platform/api/grpc/GrpcApiService.scala
View file
Original file line number Diff line number Diff line change
Expand Up @@ -3,15 +3,10 @@

package com.digitalasset.platform.api.grpc

import com.digitalasset.ledger.api.logging.LoggingServiceMarker
import io.grpc.BindableService

/*
Defines a interface that identifies a api service which will be registered with the
ledger api grpc server.
*/
/**
* Defines a interface that identifies a api service which will be registered with the
* ledger api grpc server.
*/
trait GrpcApiService extends BindableService with AutoCloseable

object GrpcApiService {
type LoggingService = GrpcApiService with LoggingServiceMarker
}
37 changes: 0 additions & 37 deletions ...i-common/src/main/scala/com/digitalasset/platform/common/logging/NamedLoggerFactory.scala
View file

This file was deleted.

11 changes: 0 additions & 11 deletions ...on/src/main/scala/com/digitalasset/platform/common/logging/SimpleNamedLoggerFactory.scala
View file

This file was deleted.

Loading

0 comments on commit 1ad7d6c

Please sign in to comment.