[Resources.Azure] Add support for Container App Jobs

[hansmbakker]

Fixes #2036
Design discussion issue #

Changes

This adds support for the detection of Container App Jobs and fixes the issue that normal Container App are recognized but that containers running as a ACA Job are shown as unknown_service:dotnet.

Merge requirement checklist

• CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
• Unit tests added/updated
• Appropriate CHANGELOG.md files updated for non-trivial changes
• Changes in public API reviewed (if applicable)

[linux-foundation-easycla]

The committers listed above are authorized under a signed CLA.

• ✅ login: hansmbakker / name: Hans Bakker (07dd520, 2d16d93, 703447b, d75fa62, 51844af, fa5e938, db4005b, 9789fb6, f20605c, 5fad1c6, d6dad2c, 1d35b51, f6ecf3a, 31dd016)
• ✅ login: cijothomas / name: Cijo Thomas (1bc40b8)
• ✅ login: Kielek / name: Piotr Kiełkowicz (cfdb25c, 676444d, 8a5cc32, 2897aa2, 9d053ea)

[Kielek]

Please update CHANGELOG.md and README.md.

I will leave logic review for @rajkumar-rangaraj and @TimothyMothra.

[hansmbakker]

Please update CHANGELOG.md and README.md.

@Kielek done

[cijothomas]

thanks.

[TimothyMothra]

Hi @hansmbakker, thanks for the contribution. Were you able to test this in a sample app to confirm that this Environment Variable exists?

@rajkumar-rangaraj, do we need to get this reviewed from anyone on the service team to confirm we can take a dependency on this environment variable? Do we need to share this with the other languages to make the same change?

[hansmbakker]

@TimothyMothra I created a sample app and added it in the last commit. I connected it to application insights.

⚠️ Note that in order to access the internal AzureContainerAppsResourceDetector class, I had to add [InternalsVisibleTo(...)] to its project when building that app. I did not commit that change but documented it in the sample apps README.md.

Application map (note that it now correctly shows the job name instead of unknown_service:dotnet):

Example activity making a call to microsoft.com and throwing an example exception:

Environment variables:

[hansmbakker]

do we need to get this reviewed from anyone on the service team to confirm we can take a dependency on this environment variable

@TimothyMothra if you're reaching out to the service team anyway, it would be good to request more useful environment variables - it would be nice to also log the image name and image tag for example.

[rajkumar-rangaraj]

I’ve engaged the Azure Container Apps team to confirm whether we can reliably use the environment variables CONTAINER_APP_JOB_NAME and CONTAINER_APP_JOB_EXECUTION_NAME. Until we receive confirmation, I recommend putting this PR on hold.

[cijothomas]

I’ve engaged the Azure Container Apps team to confirm whether we can reliably use the environment variables CONTAINER_APP_JOB_NAME and CONTAINER_APP_JOB_EXECUTION_NAME. Until we receive confirmation, I recommend putting this PR on hold.

I am okay to merge the PR, given this package is experimental anyway. If we chose to merge now, lets add an explicit warning to the ACAJobs methods/doc that this is highly experimental and subject to even more change (or similar wording to indicate the unstability part).

@hansmbakker do you prefer to merge now (with a additional doc warning suggested above) OR wait for an official confirmation from ACA team on this?

[TimothyMothra]

I am okay to merge the PR, given this package is experimental anyway.

I'm in favor of waiting before we merge. Users aren't blocked and can write their own resource detector for the short term.

@rajkumar-rangaraj do you have any estimate on how long till we get a response from the server team? As this week is Hackathon, I wouldn't expect a response this week.

@cijothomas would you be comfortable waiting 2-3 weeks for a response? I'd like to avoid extra churn if possible :)

[hansmbakker]

@hansmbakker do you prefer to merge now (with a additional doc warning suggested above) OR wait for an official confirmation from ACA team on this?

@cijothomas I'm ok with waiting for a confirmation if it's a reasonable timeframe 👍🏼 I understand it's better to avoid unnecessary changes if we can prevent them.

[rajkumar-rangaraj]

LGTM

I received confirmation that we can rely on these environment variables for ACA jobs.

[Kielek]

@hansmbakker, tests are failing. Could you please fix it?

[hansmbakker]

@Kielek the app name / job name env vars were not cleared in between tests which caused the test failure. This should be fixed now

[codecov]

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 83.89%. Comparing base (71655ce) to head (2897aa2).
Report is 446 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #2064      +/-   ##
==========================================
+ Coverage   73.91%   83.89%   +9.97%     
==========================================
  Files         267        6     -261     
  Lines        9615      149    -9466     
==========================================
- Hits         7107      125    -6982     
+ Misses       2508       24    -2484     

Flag	Coverage Δ
unittests-Resources.Azure	83.89% <100.00%> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files with missing lines	Coverage Δ
...ources.Azure/AzureContainerAppsResourceDetector.cs	90.90% <100.00%> (ø)

... and 272 files with indirect coverage changes