Skip to content

Commit

Permalink
WX-1122 Use legacy AppInsights to get better control over logging (#7157
Browse files Browse the repository at this point in the history
)
  • Loading branch information
jgainerdewar authored Jun 21, 2023
1 parent e266135 commit 23e3919
Show file tree
Hide file tree
Showing 7 changed files with 16 additions and 25 deletions.
1 change: 0 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -55,4 +55,3 @@ tesk_application.conf
**/venv/
exome_germline_single_sample_v1.3/
**/*.pyc
applicationinsights.log
6 changes: 3 additions & 3 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -6,9 +6,9 @@

WDL `size` engine function now works for HTTP files.

### Azure Instrumentation Support
Cromwell can now send logs and process information to Azure Application Insights. To enable, set environment
variable `APPLICATIONINSIGHTS_CONNECTION_STRING` to your connection string. [See here for information.](https://learn.microsoft.com/en-us/azure/azure-monitor/app/sdk-connection-string)
### Azure ApplicationInsights Logging Support
Cromwell can now send logs to Azure Application Insights. To enable, set environment
variable `APPLICATIONINSIGHTS_INSTRUMENTATIONKEY` to your account's key. [See here for information.](https://learn.microsoft.com/en-us/azure/azure-monitor/app/sdk-connection-string)


## 85 Release Notes
Expand Down
9 changes: 7 additions & 2 deletions project/Dependencies.scala
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,12 @@ object Dependencies {
// https://github.com/sbt/sbt/issues/4531
private val azureStorageBlobNioV = "12.0.0-beta.19"
private val azureIdentitySdkV = "1.9.0-beta.2"
private val azureAppInsightsV = "3.4.12"
// We are using the older AppInsights 2 because we want to use the
// logback appender to send logs. AppInsights 3 does not have a standalone
// appender, and its auto-hoovering of logs didn't meet our needs.
// (Specifically, the side-by-side root logger and workflow logger resulted in
// duplicate messages in AI. See WX-1122.)
private val azureAppInsightsLogbackV = "2.6.4"
private val betterFilesV = "3.9.1"
private val jsonSmartV = "2.4.10"
/*
Expand Down Expand Up @@ -213,7 +218,7 @@ object Dependencies {
"com.fasterxml.jackson.dataformat" % "jackson-dataformat-xml" % jacksonV,
"com.azure.resourcemanager" % "azure-resourcemanager" % "2.18.0",
"net.minidev" % "json-smart" % jsonSmartV,
"com.microsoft.azure" % "applicationinsights-runtime-attach" % azureAppInsightsV,
"com.microsoft.azure" % "applicationinsights-logging-logback" % azureAppInsightsLogbackV,
)

val wsmDependencies: List[ModuleID] = List(
Expand Down
3 changes: 3 additions & 0 deletions project/Merging.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,9 @@ object Merging {
MergeStrategy.filterDistinctLines
case PathList(ps@_*) if ps.last == "logback.xml" =>
MergeStrategy.first
// Merge mozilla/public-suffix-list.txt if duplicated
case PathList(ps@_*) if ps.last == "public-suffix-list.txt" =>
MergeStrategy.last
// AWS SDK v2 configuration files - can be discarded
case PathList(ps@_*) if Set("codegen.config" , "service-2.json" , "waiters-2.json" , "customization.config" , "examples-1.json" , "paginators-1.json").contains(ps.last) =>
MergeStrategy.discard
Expand Down
7 changes: 0 additions & 7 deletions server/src/main/resources/applicationinsights.json

This file was deleted.

3 changes: 3 additions & 0 deletions server/src/main/resources/logback.xml
Original file line number Diff line number Diff line change
Expand Up @@ -94,11 +94,14 @@
</filter>
</appender>

<appender name="AppInsightsAppender" class="com.microsoft.applicationinsights.logback.ApplicationInsightsAppender" />

<root level="${LOG_LEVEL}">
<appender-ref ref="STANDARD_APPENDER" />
<appender-ref ref="PRETTY_APPENDER" />
<appender-ref ref="FILEROLLER_APPENDER" />
<appender-ref ref="Sentry" />
<appender-ref ref="AppInsightsAppender" />
</root>

<logger name="liquibase" level="WARN"/>
Expand Down
12 changes: 0 additions & 12 deletions server/src/main/scala/cromwell/CromwellEntryPoint.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@ import cats.data.Validated._
import cats.effect.{ContextShift, IO}
import cats.syntax.apply._
import cats.syntax.validated._
import com.microsoft.applicationinsights.attach.ApplicationInsights
import com.typesafe.config.{Config, ConfigFactory}
import common.exception.MessageAggregation
import common.validation.ErrorOr._
Expand Down Expand Up @@ -46,11 +45,6 @@ object CromwellEntryPoint extends GracefulStopSupport {
private val dnsCacheTtl = config.getOrElse("system.dns-cache-ttl", 3 minutes)
java.security.Security.setProperty("networkaddress.cache.ttl", dnsCacheTtl.toSeconds.toString)

// The presence of this env var tells us that the user is trying to send instrumentation data and
// logs to Azure Application Insights. If it's present, we'll attach the ApplicationInsights agent.
// To configure the behavior of this agent, see server/src/main/resources/applicationinsights.json
private lazy val useAzureInstrumentation = sys.env.contains("APPLICATIONINSIGHTS_CONNECTION_STRING")

/**
* Run Cromwell in server mode.
*/
Expand Down Expand Up @@ -151,12 +145,6 @@ object CromwellEntryPoint extends GracefulStopSupport {
*/
private def initLogging(command: Command): Unit = {

// Enable Azure instrumentation and log-slurping if desired. Running ApplicationInsights.attach()
// without checking for the presence of the relevant env var doesn't cause any failures, but does
// print an error that would be confusing for non-Azure users.
if (useAzureInstrumentation)
ApplicationInsights.attach()

val logbackSetting = command match {
case Server => "STANDARD"
case Submit => "PRETTY"
Expand Down

0 comments on commit 23e3919

Please sign in to comment.