Skip to content

feat: improve app insights telemetry#226

Open
Shreyas-Microsoft wants to merge 5 commits intodevfrom
psl-sw/37816-app-insights-telemetry
Open

feat: improve app insights telemetry#226
Shreyas-Microsoft wants to merge 5 commits intodevfrom
psl-sw/37816-app-insights-telemetry

Conversation

@Shreyas-Microsoft
Copy link
Copy Markdown
Collaborator

Purpose

This pull request introduces robust Application Insights and OpenTelemetry integration for the backend API, providing production-grade observability while minimizing noise and ingestion costs. It adds environment-driven configuration for telemetry, custom span processors to filter noisy spans (such as per-chunk ASGI responses and Cosmos DB dependencies), and a lightweight helper for emitting structured business events. Logging verbosity for known noisy packages is also clamped to keep logs actionable. The infrastructure Bicep templates are updated to avoid duplicate log ingestion.

Observability and Telemetry Integration:

  • Added Application Insights and OpenTelemetry support, configurable via the APPLICATIONINSIGHTS_CONNECTION_STRING environment variable. This includes wiring up Azure Monitor and FastAPI instrumentation, with exclusion of health/startup probe routes from telemetry to avoid flooding. [1] [2] [3] [4] [5]
  • Introduced libs/logging/event_utils.py with the track_event_if_configured helper for emitting custom App Insights events, which is safe to call even when telemetry is not configured. [1] [2] [3]

Noise Suppression and Filtering:

  • Added custom OpenTelemetry SpanProcessor implementations (DropASGIResponseBodySpanProcessor, DropCosmosDependencySpanProcessor) in libs/logging/span_filters.py to filter out high-volume, low-value spans before export, reducing cost and improving signal-to-noise. [1] [2] [3]
  • Hard-clamped logging levels to WARNING for known noisy packages (e.g., Azure SDK internals, Cosmos diagnostics), regardless of operator configuration, to prevent log flooding in Application Insights. [1] [2]

Infrastructure Improvements:

  • Updated Bicep modules (infra/main.bicep, infra/main_custom.bicep) to document and prevent duplicate log ingestion by removing redundant diagnosticSettings when Application Insights is already wired to Log Analytics via workspaceResourceId. [1] [2]

These changes collectively ensure that the backend API emits actionable telemetry and business events to Application Insights with minimal configuration, while keeping the signal clean and ingestion costs predictable.

Does this introduce a breaking change?

  • Yes
  • No

Golden Path Validation

  • I have tested the primary workflows (the "golden path") to ensure they function correctly without errors.

Deployment Validation

  • I have validated the deployment process successfully and all services are running as expected with this change.

What to Check

Verify that the following are valid

  • ...

Other Information

Shreyas-Microsoft and others added 5 commits May 6, 2026 19:11
Adds the small foundation needed for the rest of the App Insights
wiring on this branch:

- New `libs/logging/event_utils.py` with `track_event_if_configured`,
  a tiny gated wrapper around `azure.monitor.events.extension.track_event`
  that no-ops when `APPLICATIONINSIGHTS_CONNECTION_STRING` is unset.
  Lazy-imports the SDK and swallows export errors so telemetry can
  never break a request. Single warn-once latch keeps logs clean in
  unit tests / local dev.
- New `libs/logging/span_filters.py` with two custom `SpanProcessor`
  implementations:
    * `DropASGIResponseBodySpanProcessor` strips `http.response.body`
      child spans (one per streamed chunk) so streamed downloads / SSE
      do not flood Application Insights.
    * `DropCosmosDependencySpanProcessor` drops Cosmos DB dependency
      spans (matched on `db.system==cosmosdb` and the
      `documents.azure.com` peer host) so the Application Map is not
      dominated by per-call Cosmos operations.
- Adds `azure-monitor-events-extension` and (explicitly)
  `opentelemetry-instrumentation-fastapi` to `pyproject.toml`, then
  refreshes `uv.lock` via `uv lock` (no hand edits).

No call sites change in this commit; the new helpers are wired into
`Application.initialize` and the routers in the next two commits.

Implements groundwork for AC #1, #2, #3, #4 of AB#37816.
Work item: https://dev.azure.com/CSACTOSOL/CSA%20Solutioning/_workitems/edit/37816

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ation.initialize

Glues the helpers from the previous commit into the FastAPI startup
path so AC #1, #2, #3, #4 of AB#37816 are satisfied at the code
layer:

`Application.initialize`
- New `_configure_azure_monitor`: when
  `APPLICATIONINSIGHTS_CONNECTION_STRING` is set, calls
  `azure.monitor.opentelemetry.configure_azure_monitor` with
  `enable_live_metrics=True` and the two custom span processors
  (`DropASGIResponseBodySpanProcessor`,
  `DropCosmosDependencySpanProcessor`) added in the previous commit.
  Connection string is read from env, never logged.
- New `_instrument_fastapi`: attaches
  `FastAPIInstrumentor.instrument_app(self.app, excluded_urls="health,startup")`
  AFTER all routers are registered so every business route is
  instrumented but the liveness / startup probes do not flood
  Application Insights with empty request rows.
- Both helpers fail-soft: any exception during telemetry setup is
  logged and swallowed, so a misconfigured App Insights instance can
  never block backend startup.
- Both helpers are no-ops when the connection string is unset, so
  local dev / unit tests behave exactly as they did before.

`Application_Base`
- Adds a `_NOISY_LOGGER_PACKAGES` hard-suppression list
  (`azure.core.pipeline.policies.http_logging_policy`, `azure.cosmos`,
  `opentelemetry.sdk`,
  `azure.monitor.opentelemetry.exporter.export._base`) clamped to
  WARNING regardless of the operator-supplied `AZURE_LOGGING_PACKAGES`.
  Without this clamp, the App Insights logs view is dominated by per-
  request HTTP policy logs and per-call Cosmos diagnostics.
- The clamp is one-way: never lowers a level the operator has
  explicitly raised below WARNING.

`.env.example`
- Documents `APPLICATIONINSIGHTS_CONNECTION_STRING` (commented) so
  local dev mirrors the Bicep-injected production env.

Validation
- `uv run pytest src/tests --no-cov -c /dev/null` -> 44 passed
  (with `PYTHONPATH=src/app`, matching the existing repo convention).
- Manual `Application()` smoke with the env var unset -> single
  INFO line, 26 routes registered, no exceptions.
- Manual `Application()` smoke with a fake conn string -> Azure
  Monitor configured, FastAPIInstrumentor attached, exporter
  fails-soft on DNS resolution, app continues.

Implements AC #1, #2, #3, #4 of AB#37816.
Work item: https://dev.azure.com/CSACTOSOL/CSA%20Solutioning/_workitems/edit/37816

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Adds Application Insights custom-event emission and per-request span
annotation to every business handler in:
- routers/router_process.py
- routers/router_files.py
- routers/router_debug.py

For each handler:
- On success, emits a `<RouteName>Success` event via
  `track_event_if_configured` with the relevant domain ids
  (process_id, file_id, filename, counts, kill_state, ...).
- On exception, emits a `<RouteName>Error` event carrying
  `error`, `error_type`, and (when available) `status_code`.
  Then calls `current_span.record_exception(e)` and
  `set_status(StatusCode.ERROR)` so the failure is also visible in
  the App Insights end-to-end transaction view.
- Stamps domain ids onto the active span via `set_attribute` so
  custom queries on `requests | extend customDimensions` can pivot
  by `process_id` / `file_id` directly without re-parsing message
  strings.

Two small private helpers `_annotate_span` and
`_record_exception_on_span` are duplicated into both `router_process.py`
and `router_files.py` (rather than placed in `libs/logging/`) to keep
the OpenTelemetry import surface visible at the call site and to
avoid a new public helper that the rest of the codebase might
accidentally depend on.

`routers/http_probes.py` is left untouched — its routes match the
`excluded_urls="health,startup"` configured on `FastAPIInstrumentor`
in the previous commit and so are intentionally not telemetered.

The pre-existing `ILoggerService.log_info(f"...")` / `log_error(...)`
calls are preserved as-is. The new code emitted by this commit uses
`%s` lazy formatting in its log strings.

Validation
- `uv run pytest src/tests --no-cov -c /dev/null` -> 44 passed
  (with `PYTHONPATH=src/app`).
- `Application()` smoke-init -> 26 routes registered, all routers
  import cleanly.

Implements AC #3 of AB#37816 (telemetry visibility) and contributes
to AC #4 (continuous log collection).
Work item: https://dev.azure.com/CSACTOSOL/CSA%20Solutioning/_workitems/edit/37816

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ts gating

Adds the first dedicated test package for the new
`libs/logging/event_utils.py` helper at
`src/tests/logging/test_event_utils.py` (mirroring the existing
`src/tests/<area>/test_<thing>.py` layout — no new tooling, no new
config).

Coverage:

- `test_no_op_when_connection_string_unset`: with the env var absent,
  the helper does NOT call `track_event` and the SDK module is never
  imported (verified via a `sys.modules` fake).
- `test_warning_fires_only_once_per_process`: subsequent unconfigured
  calls are silent (one-shot warning latch).
- `test_unconfigured_warning_message_is_stable`: pins the exact
  warning template that the rest of the system asserts against.
- `test_forwards_to_track_event_when_configured`: with the env var
  set, the helper forwards the (name, properties) pair through to
  `azure.monitor.events.extension.track_event`.
- `test_none_properties_normalised_to_empty_dict`: `properties=None`
  surfaces as `{}` to the SDK so `customDimensions` is always an
  object.
- `test_whitespace_only_connection_string_is_treated_as_unset`:
  belt-and-braces gating against accidentally-blank env values.
- `test_sdk_exception_is_swallowed`: a `RuntimeError` thrown by the
  SDK is caught and logged; telemetry must never break a request.

The tests use a `sys.modules` fake for `azure.monitor.events.extension`
so they never import the real Azure Monitor SDK at runtime — keeps
the suite hermetic, fast (<1s), and CI-friendly.

Validation
- `uv run pytest src/tests --no-cov -c /dev/null` -> 51 passed
  (44 pre-existing + 7 new), with `PYTHONPATH=src/app`.

Implements verification scope of AC #3 of AB#37816.
Work item: https://dev.azure.com/CSACTOSOL/CSA%20Solutioning/_workitems/edit/37816

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
…ts AVM module

The AVM `insights/component@0.6.0` module already wires Application
Insights to Log Analytics through the `workspaceResourceId` parameter
(workspace-based App Insights). Adding a separate `diagnosticSettings`
entry pointing at the SAME workspace causes the platform-level
`AppAvailabilityResults`, `AppRequests`, etc. tables to be ingested
twice — once via the workspace-based App Insights pipeline and again
via the diagnostic-settings forwarder.

Removes the redundant line from both:
- `infra/main.bicep`           (line 305)
- `infra/main_custom.bicep`    (line 283)

The custom-event / OpenTelemetry path the rest of this branch wires
up sends data straight to App Insights via
`APPLICATIONINSIGHTS_CONNECTION_STRING`, which is already injected
into the backend-api and processor container apps by the same Bicep
files (no change required to those env-var blocks).

Validation
- `bicep build infra/main.bicep --outfile <tmp>`        -> OK, no diagnostics.
- `bicep build infra/main_custom.bicep --outfile <tmp>` -> OK, only a
  pre-existing BCP334 warning at line 694 (unrelated to this edit).

Implements AC #4 of AB#37816 (continuous, non-duplicated log collection).
Reference: microsoft/Conversation-Knowledge-Mining-Solution-Accelerator#811
Work item: https://dev.azure.com/CSACTOSOL/CSA%20Solutioning/_workitems/edit/37816

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@github-actions
Copy link
Copy Markdown

github-actions Bot commented May 6, 2026

Coverage

Coverage Report •
FileStmtsMissCoverMissing
__init__.py440%1, 3–5
application.py60600%1–3, 5–8, 12–16, 21–22, 25–26, 28, 34, 37, 43–44, 46–47, 49, 57, 63, 65, 78, 80–82, 87, 89, 101, 104–105, 109, 111–112, 114, 124, 127–128, 133, 140, 144–146, 148, 151, 155–156, 161, 170, 177–178, 180, 187, 235–236
libs/base
   application_base.py491373%37, 41, 62, 71, 74, 77–78, 83, 92, 100–103
libs/logging
   __init__.py00100% 
   event_utils.py260100% 
   span_filters.py22220%30, 32–33, 35–36, 38, 41, 54, 86, 99–101, 108–117
routers
   router_debug.py25250%1–6, 8, 15–16, 18–19, 21, 23, 39–42, 48–52, 55–57
   router_files.py88880%1–2, 4, 11, 14–26, 29, 31–40, 43, 45–52, 55, 62–63, 65, 75–76, 81–82, 84–86, 88–89, 91–93, 95–96, 98–100, 102, 104–107, 109–110, 116, 120, 127–128, 130, 134–136, 138–139, 143, 150, 160, 166–168, 177–181, 189–190
   router_process.py4274270%1–5, 7–20, 28, 33, 39–48, 51, 53–60, 63, 70–79, 82–86, 88–90, 92–93, 95, 97–99, 101–102, 105–108, 116–120, 124–125, 128–129, 131, 133–134, 137, 140, 142–144, 147–149, 157–158, 161–162, 164, 166–167, 170, 173, 175–177, 180–182, 190–191, 194–195, 206–207, 209–210, 213–214, 217–218, 221–222, 224–225, 228, 230–232, 235–236, 242, 244, 247, 249, 253, 258, 260, 265, 280–281, 283, 290, 298–301, 309–310, 313–314, 325–326, 328–329, 332–333, 336–337, 340–341, 343–344, 347, 350, 352, 355, 357, 362, 377–378, 380, 387, 395, 397–398, 406–407, 411–413, 422–423, 426–427, 437–438, 440–441, 446–447, 450–451, 453–454, 457, 460, 462, 467, 475–476, 478, 481, 485, 487–489, 497–498, 501–502, 511–512, 514–515, 518–519, 522–523, 525–526, 529, 535, 543, 545, 548–549, 551–552, 555, 561–563, 571–572, 577–578, 586–587, 589–590, 595–596, 598–599, 602, 605, 607–608, 613–615, 617, 619, 621, 625, 631, 637, 645–647, 656–660, 668–669, 674–675, 683–684, 686–687, 692–693, 695–696, 699, 702, 705, 714, 718, 721, 726, 728–730, 739–743, 751–752, 757–758, 767–768, 770–771, 776–777, 779–780, 783, 786, 790, 794–795, 800, 802–803, 811–812, 816–817, 825–826, 830–832, 842–846, 855–856, 861–862, 872–873, 875–877, 880–881, 883–884, 887–889, 892–894, 897–898, 902–903, 908, 910–912, 917–918, 921, 926, 928, 932, 940, 948–950, 957–958, 962–964, 971–972, 976–977, 986–990, 998–999, 1004–1005, 1013–1014, 1016–1018, 1021–1022, 1024–1025, 1028–1030, 1033–1035, 1038–1039, 1043–1044, 1048, 1050–1052, 1057–1058, 1061, 1066, 1068, 1071, 1073–1075, 1079–1080, 1084–1086, 1090–1091, 1095–1096, 1105–1109, 1117–1118
TOTAL296426759% 

Tests Skipped Failures Errors Time
51 0 💤 0 ❌ 0 🔥 12.956s ⏱️

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.

1 participant