Conditional based profiles for workers

[shreyas-gopalakrishna]

resolves #8184

Design doc: https://dev.azure.com/msazure/One/_git/AAPT-Antares-Docs/pullrequest/5715621?_a=files

Issue describing the changes in this PR

Profiles enable workers to change how the worker is configured based on certain conditions, for example a worker may want to configure
itself differently for a consumption application vs a dedicated application. The profiles will be defined in the worker.config.json file
and will essentially override the default worker description based on a set of conditions.

The Java worker team is working on a prototype where an Application Insights (AI) agent is deployed along side the Java worker
to handle telemetry and logging to AI. Depending on the SKU and environment variables, the AI agent should be used.
Profiles will be used to add arguments required to enable the agent.

Sample worker.config.json with profiles

{
  "description": { ... }, // this will always be the default worker configuration
  "profiles": [
    {
      "profileName": "profile1",
      "conditions": [
        { "conditionType": "environment", "conditionName": "APP_SETTING_NAME", "conditionExpression": "app_setting_value" }
      ],
      "description": {
        "arguments": ["-DmyNewArg"]
      }
    },
    {
      "profileName": "profile2",
      "conditions": [
        { "conditionType": "hostProperty", "conditionName": "sku", "conditionExpression": "consumption" }
      ],
      "description": {
        "defaultWorkerPath": "",
        "arguments": []
      }
    }
  ]
}

Sample worker.config.json with Java app insights - profiles

{
    "description": {
        "language": "java",
        "extensions": [".jar"],
        "defaultExecutablePath": "%JAVA_HOME%/bin/java",
        "defaultWorkerPath": "azure-functions-java-worker.jar",
        "arguments": ["-XX:+TieredCompilation -XX:TieredStopAtLevel=1 -noverify -Djava.net.preferIPv4Stack=true -jar", "%JAVA_OPTS%", "%AZURE_FUNCTIONS_MESH_JAVA_OPTS%"]
    },
    "profiles":
    [
        {
            "profileName":"AppInsightsConsumptionOptOut",
            "conditions":[
                {"conditionType":"environment","conditionName":"languageWorkers:java:arguments","conditionExpression":"-DaiAgentOptOut"},
                {"conditionType":"hostProperty","conditionName":"sku","conditionExpression":"Dynamic"}
            ],
            "description":{
                "arguments": ["-XX:+TieredCompilation -XX:TieredStopAtLevel=1 -noverify -Djava.net.preferIPv4Stack=true -jar", "%JAVA_OPTS%", "%AZURE_FUNCTIONS_MESH_JAVA_OPTS%"]
            }
        },
        {
            "profileName":"AppInsightsConsumption",
            "conditions":[
                {"conditionType":"hostProperty","conditionName":"sku","conditionExpression":"Dynamic"}
            ],
            "description":{
                "arguments":["-XX:+TieredCompilation -XX:TieredStopAtLevel=1 -noverify -Djava.net.preferIPv4Stack=true -javaagent:\"{workerDirectoryPath}/agent/applicationinsights-agent.jar\" -DLazySetOptIn=false -jar", "%JAVA_OPTS%", "%AZURE_FUNCTIONS_MESH_JAVA_OPTS%"]
            }
        },
        {
            "profileName":"AppInsightsEnabledDedicated",
            "conditions":[
                {"conditionType":"environment","conditionName":"APPLICATIONINSIGHTS_ENABLE_AGENT","conditionExpression":"true"},
                {"conditionType":"hostProperty","conditionName":"sku","conditionExpression":"(Standard|ElasticPremium)"}
            ],
            "description":{
                "arguments": ["-XX:+TieredCompilation -XX:TieredStopAtLevel=1 -noverify -Djava.net.preferIPv4Stack=true -javaagent:\"{workerDirectoryPath}/agent/applicationinsights-agent.jar\" -jar", "%JAVA_OPTS%", "%AZURE_FUNCTIONS_MESH_JAVA_OPTS%"]
            }
        }
    ]
}

Pull request checklist

• My changes do not require documentation changes
  • Otherwise: Documentation issue linked to PR
• My changes should not be added to the release notes for the next release
  • Otherwise: I've added my notes to release_notes.md
• My changes do not need to be backported to a previous version
  • Otherwise: Backport tracked by issue/PR #issue_or_pr
• I have added all required tests (Unit tests, E2E tests)

[liliankasem]

Was only able to get through half of this and will do a second pass tomorrow. Please make sure that all the classes are documented with comments

[liliankasem]

I think this looks great, nice work! Just adding some minor comments. Biggest takeaways for me is that the code needs to be commented, and unit tests need to be implemented for public methods

[ghost]

This pull request has been automatically marked as stale because it has been marked as requiring author feedback but has not had any activity for 7 days. It will be closed if no further activity occurs within 7 days of this comment.

[liliankasem]

@soninaren @shreyas-gopalakrishna could diagnostic events be useful here? In a situation where a user edits the worker.config file, we might want to warn them about that?

[surgupta-msft]

I vaguely remember that there was a design document for this work. If it exists, can you please link that design document either to this PR or the linked issue?

[shreyas-gopalakrishna]

While running locally, this code block as well as RpcWorkerConfigFactory section was called thrice. Each the they had the same worker config. Checking if it is safe to do this assignment.

[liliankasem]

@fabiocav thoughts on this? I'm assuming this is expected, we could always check to see if the profile already exists in the dictionary before trying to assign / instead of overriding every time

[fabiocav]

Was it on different instances of the WorkerProfileManager?

[shreyas-gopalakrishna]

@fabiocav Is hashcode a good way to verify the instance? I tried Console.WriteLine(this.GetHashCode()); after this line and get 3 outputs. The first hashcode is different. The second and third are the same.

[fabiocav]

Without tracing the calls it would be difficult to say. For the instances, you can create and inspect the object ID while debugging, but I do expect a call in the parent scope and one in the inner scope (web and job host levels for workers initialized at each)

[liliankasem]

Looking good! Should be good to go when we get Brett and Fabio to sign off.

It would be great to make sure the documentation is updated (I think a class diagram would be useful here) and we have an issue to track the detector work required

[liliankasem]

@fabiocav thoughts on this? I'm assuming this is expected, we could always check to see if the profile already exists in the dictionary before trying to assign / instead of overriding every time