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

WX-1122 Enable Azure ApplicationInsights #7143

Merged
merged 8 commits into from
May 26, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -55,3 +55,4 @@ tesk_application.conf
**/venv/
exome_germline_single_sample_v1.3/
**/*.pyc
applicationinsights.log
7 changes: 6 additions & 1 deletion CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,12 @@

### HTTPFilesystem Improvements

* WDL `size` engine function now works for HTTP files.
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)


## 85 Release Notes

Expand Down
4 changes: 3 additions & 1 deletion project/Dependencies.scala
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@ 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"
private val betterFilesV = "3.9.1"
private val jsonSmartV = "2.4.10"
/*
Expand Down Expand Up @@ -210,7 +211,8 @@ object Dependencies {
"com.azure" % "azure-core-management" % "1.7.1",
"com.fasterxml.jackson.dataformat" % "jackson-dataformat-xml" % jacksonV,
"com.azure.resourcemanager" % "azure-resourcemanager" % "2.18.0",
"net.minidev" % "json-smart" % jsonSmartV
"net.minidev" % "json-smart" % jsonSmartV,
"com.microsoft.azure" % "applicationinsights-runtime-attach" % azureAppInsightsV,
)

val wsmDependencies: List[ModuleID] = List(
Expand Down
7 changes: 7 additions & 0 deletions server/src/main/resources/applicationinsights.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
{
"instrumentation": {
"jdbc": {
"enabled": false
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Without this, we send an event for every single sql statement, which is... a lot. I saw around 1k per minute from a single idle instance.

Copy link
Collaborator

Choose a reason for hiding this comment

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

👍 it seems like the RDBMS instance's own query log would be the right place to look for these if we needed them.

}
}
}
15 changes: 15 additions & 0 deletions server/src/main/scala/cromwell/CromwellEntryPoint.scala
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ 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 @@ -45,6 +46,11 @@ 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 @@ -140,8 +146,17 @@ object CromwellEntryPoint extends GracefulStopSupport {
*
* Also copies variables from config/system/environment/defaults over to the system properties.
* Fixes issue where users are trying to specify Java properties as environment variables.
*
* Also also enables Azure log handling when appropriate
*/
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