Skip to content

fix(observe): correct ACA observability content (fixes for #1642) - #1712

Closed
Simon J (simonjj) wants to merge 2 commits into
microsoft:mainfrom
simonjj:fix/pass-equity-gap-10
Closed

fix(observe): correct ACA observability content (fixes for #1642)#1712
Simon J (simonjj) wants to merge 2 commits into
microsoft:mainfrom
simonjj:fix/pass-equity-gap-10

Conversation

@simonjj

Copy link
Copy Markdown
Contributor

Summary

Fixes technical errors in the ACA Observability reference file from #1642, validated against official Microsoft ACA documentation.

Changes

container-apps.md (appinsights-instrumentation skill):

  • Fix: LA workspace is not required (--logs-destination none\ is valid)
  • Fix: add missing --logs-workspace-key\ CLI param
  • Fix: metric dimensions now include
    eplica\ (was missing on 5/6 metrics) and \statusCode\ on Requests
  • Fix: rename Java 'Auto-agent JAR' to 'Agent JAR (manual)' — ACA has no auto-instrumentation
  • Fix: Dapr \�ndpointAddress\ placeholder clarified as OTel Collector, not App Insights directly
  • Fix: ARG query checks all containers via \mv-expand, not just \containers[0]\
  • Fix: remove undocumented \ReplicaCount_d\ column from scaling KQL
  • Fix: rename misleading 'Request latency' to 'Console log volume' (was counting log lines)
  • Add: actual request latency KQL using App Insights
    equests\ table

Validation

  • All fixes verified against official docs at \MicrosoftDocs/azure-docs/articles/container-apps/\
  • \�ppinsights-instrumentation\ tests: 35/35 passed
  • Token budget: ~1527/2000 tokens

Builds on #1642.

Paul Yuknewicz (paulyuk) and others added 2 commits April 1, 2026 10:36
Closes microsoft#1618 | Parent: microsoft#1608
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
- Fix: LA workspace not required (was 'requires', now 'by default')
- Fix: add missing --logs-workspace-key CLI param
- Fix: metric dimensions now include replica (was missing on 5/6 metrics)
- Fix: rename Java 'Auto-agent JAR' to 'Agent JAR (manual)' (ACA has no auto-instrumentation)
- Fix: Dapr endpoint placeholder clarified as OTel Collector, not App Insights
- Fix: ARG query checks all containers, not just containers[0]
- Fix: remove undocumented ReplicaCount_d column
- Fix: rename misleading 'Request latency' section to 'Console log volume'
- Add: actual request latency KQL using App Insights requests table

All fixes validated against official Microsoft ACA documentation.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Copilot AI review requested due to automatic review settings April 3, 2026 21:48
@simonjj

Copy link
Copy Markdown
Contributor Author

Closing — will push fixes directly to #1642's branch instead.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Updates the Azure Container Apps observability reference for the appinsights-instrumentation skill to correct and clarify guidance around Log Analytics configuration, built-in metrics dimensions, Dapr tracing, Azure Resource Graph (ARG) discovery, and KQL examples.

Changes:

  • Refreshes environment-level logging guidance (including --logs-workspace-key and valid --logs-destination options).
  • Corrects/clarifies observability specifics (metrics dimensions, Java agent wording, Dapr OTel collector endpoint guidance).
  • Updates ARG/KQL examples (multi-container inspection, removes undocumented columns, adds request-latency query via App Insights).

Comment on lines +113 to +114
| mv-expand envVar = container.env
| summarize hasAppInsights = countif(envVar.name == "APPLICATIONINSIGHTS_CONNECTION_STRING") by name, resourceGroup

Copilot AI Apr 3, 2026

Copy link

Choose a reason for hiding this comment

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

The ARG query uses mv-expand envVar = container.env, which will drop container apps/containers where env is null or empty, so apps with no env vars may not appear in results (even though they should be considered missing APPLICATIONINSIGHTS_CONNECTION_STRING). Consider avoiding the second mv-expand and instead summarizing on a null-safe string/dynamic check (e.g., tostring(container.env) / isempty() patterns) so apps without env arrays are still included.

Suggested change
| mv-expand envVar = container.env
| summarize hasAppInsights = countif(envVar.name == "APPLICATIONINSIGHTS_CONNECTION_STRING") by name, resourceGroup
| extend envVars = tostring(container.env)
| summarize hasAppInsights = countif(envVars has "\"name\":\"APPLICATIONINSIGHTS_CONNECTION_STRING\"") by name, resourceGroup

Copilot uses AI. Check for mistakes.
Comment on lines +123 to +127
ContainerAppConsoleLogs_CL
| where Log_s contains "error" or Log_s contains "exception"
| project TimeGenerated, ContainerAppName_s, RevisionName_s, Log_s
| order by TimeGenerated desc
| take 50

Copilot AI Apr 3, 2026

Copy link

Choose a reason for hiding this comment

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

These KQL snippets don’t include a time filter (e.g., TimeGenerated > ago(...) / timestamp > ago(...)). Without a time predicate, queries can be very slow/expensive in larger workspaces and are inconsistent with other KQL guidance in the repo. Add an explicit time window to this (and the other queries in this section) to keep them performant and predictable.

Copilot uses AI. Check for mistakes.
```kql
requests
| where cloud_RoleName has "<app-name>"
| summarize avgDuration = avg(duration), p95 = percentile(duration, 95) by cloud_RoleInstance, bin(timestamp, 5m)

Copilot AI Apr 3, 2026

Copy link

Choose a reason for hiding this comment

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

The section title says “Request latency by revision”, but the query groups by cloud_RoleInstance. Unless cloud_RoleInstance is guaranteed to encode the ACA revision name, this will chart per instance (replica) rather than per revision. Either adjust the grouping to an actual revision identifier (if available) or rename the heading to match what the query is doing.

Copilot uses AI. Check for mistakes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants