[← Back to Reviews Index](../Stewards%20Reviews%20Index.md)

# Python Observability Review — Azure-AI-RAG-CSharp-Semantic-Kernel-Functions

| Field | Value |
|---|---|
| Project | Azure-AI-RAG-CSharp-Semantic-Kernel-Functions |
| Steward | Python Observability Steward |
| Run Date | 2026-03-22 |
| Scope | `src/DocumentLoaderFunction/` |
| Summary | 🔴 1 critical · 🟡 5 notable · 🟢 3 minor · ℹ️ 2 info |

---

## 1. Observability Architecture Overview

The `DocumentLoaderFunction` is a single-file Python Azure Function (`function_app.py`) that listens for blob-trigger events on the `load` container. When a blob arrives, the function:

1. Reads the blob content and parses it as HTML.
2. Acquires an Azure AD token and creates AI Search / OpenAI clients.
3. Creates or verifies the Azure AI Search index.
4. Generates vector embeddings via LangChain / Azure OpenAI.
5. Uploads documents to the search index.
6. Moves the processed blob to a `completed` container and deletes the source blob.

**Observability stack (as deployed):**

- Python `logging` module — used directly via module-level calls (`logging.info(...)`, `logging.error(...)`).
- Azure Functions host integration — the Functions runtime forwards Python logging to Application Insights automatically when `APPINSIGHTS_INSTRUMENTATIONKEY` is set.
- `host.json` — configures Application Insights sampling with `Request` type excluded from sampling.
- Infra (Bicep) — provisions Application Insights backed by a Log Analytics workspace; diagnostic settings forward `FunctionAppLogs` and `AllMetrics` to Log Analytics.

**Key observability gaps:**

- Application Insights is wired via `APPINSIGHTS_INSTRUMENTATIONKEY` (legacy key) rather than the modern `APPLICATIONINSIGHTS_CONNECTION_STRING`.
- No `azure-monitor-opentelemetry` or `opencensus-ext-azure` package is listed in `requirements.txt` — relying entirely on the Functions host's built-in forwarding.
- No logger name is set (`logging.getLogger(__name__)` is not used); all log records originate from the root logger.
- Token value is written to an environment variable (`OPENAI_API_KEY`) during execution; if the runtime ever logs environment state, the token would be exposed.
- Blob content is logged raw in one location (`logging.info(f"Blob content as JSON: {json_data}")`), which can expose document PII.
- The blob delete at line 99–100 executes outside the try/except block, meaning a `NameError` on `blob_service_client` is raised as an unhandled exception if the main block fails early.

---

## 2. Logging Configuration Assessment

### 2.1 Python `logging` module usage

The `logging` module is imported at line 4 and used throughout. No `print()` statements are present. This is correct.

### 2.2 Logger instantiation

All calls use the root logger directly: `logging.info(...)`, `logging.error(...)`. The function never calls `logging.getLogger(__name__)`. This means:

- All log records appear under the root logger name (`root`) in Application Insights — not `DocumentLoaderFunction.function_app`.
- Log filtering (e.g., setting DEBUG level only for this module) is impossible without affecting the entire process.

### 2.3 Log levels

| Location | Call | Assessment |
|---|---|---|
| Blob trigger entry | `logging.info(...)` | Appropriate — INFO for function entry |
| JSON parse start | `logging.info(f"Blob content as JSON: {json_data}")` | Problematic — logs document content at INFO (PII risk) |
| Embedding creation | `logging.info("Create embeddings")` | Appropriate level; message is terse |
| Index verification | `self.logger.info("Verifying if AI Search index exists...")` | Appropriate |
| Index not found | `self.logger.info("AI Search index not found, creating index...")` | Should be WARNING — index absent is an exceptional initial state |
| Upload succeeded | `self.logger.info("Upload of new document succeeded: ...")` | Appropriate |
| AI Search error | `self.logger.error("Error in AI Search: %s", ex)` | Good — uses `%`-style, no exc_info |
| JSON decode error | `logging.error(f"Failed to parse blob content as JSON: {e}")` | Uses f-string; logs traceback separately |
| General exception | `logging.error(f"loader Failed: {e}")` | Uses f-string; logs traceback separately |
| Blob delete | `logging.info(f"Deleted blob: {myblob.name}")` | Appropriate |

### 2.4 Formatting style

The code mixes f-strings (`logging.info(f"...")`) with `%`-style formatting (`self.logger.error("Error in AI Search: %s", ex)`). The recommended approach is `%`-style (lazy evaluation) or structured key-value logging. F-strings in log calls evaluate unconditionally even when the log level is filtered out — a minor performance concern and a correctness risk when the formatted value could raise.

---

## 3. Azure Monitor Integration Assessment

### 3.1 Application Insights provisioning

The Bicep module (`infra/core/monitor/applicationinsights.bicep`) provisions an Application Insights resource backed by a Log Analytics workspace. Diagnostic settings on the function app forward `FunctionAppLogs` and `AllMetrics`. This is well-structured.

### 3.2 Connection string vs. instrumentation key

The function app Bicep (`infra/app/loader-function.bicep`, line 75–77) configures only `APPINSIGHTS_INSTRUMENTATIONKEY`:

```
name: 'APPINSIGHTS_INSTRUMENTATIONKEY'
value: appInsights.properties.InstrumentationKey
```

The modern and recommended approach is to set `APPLICATIONINSIGHTS_CONNECTION_STRING` instead. Microsoft deprecated direct use of the instrumentation key for new workloads. The connection string contains the endpoint URL and supports sovereign cloud environments and future transport changes.

The `applicationinsights.bicep` output does produce a `connectionString` (line 28), but it is never consumed by the loader function Bicep — only `instrumentationKey` is used.

### 3.3 OpenTelemetry / azure-monitor-opentelemetry package

`requirements.txt` does not include `azure-monitor-opentelemetry` or `opencensus-ext-azure`. The function relies on the Azure Functions host's built-in Application Insights integration (environment variable + host.json). This is functional for basic telemetry (invocations, exceptions surfaced to the runtime) but provides no custom span creation, custom metrics, or enriched distributed tracing context.

### 3.4 Sampling configuration

`host.json` enables sampling and excludes `Request` type from sampling. This is intentional — blob-triggered functions do not produce HTTP request telemetry, so the exclusion is harmless. Dependency and trace telemetry will be sampled at default rates.

---

## 4. Custom Spans / Tracing Assessment

### 4.1 Distributed tracing

No custom OpenTelemetry spans are created. Without `azure-monitor-opentelemetry`, there are no spans for:

- HTML parsing (`html_to_json`)
- Embedding generation (LangChain / Azure OpenAI call)
- Azure AI Search index creation
- Document upload

The only tracing available is the built-in function invocation telemetry emitted by the Functions host, which does not drill into internal steps.

### 4.2 Blob context propagation

The blob name (`myblob.name`) is logged at function entry (line 35–37) and at deletion (line 101). However, blob context (name, size) is not consistently attached to error log messages. For example:

- Line 93: `"Failed to parse blob content as JSON: {e}"` — no blob name included.
- Line 96: `"loader Failed: {e}"` — no blob name included.
- Line 184: `"Error in AI Search: %s", ex` — no blob name included.

This makes it difficult to correlate errors to specific documents in Application Insights queries without joining on invocation ID.

### 4.3 Operation correlation

Azure Functions automatically propagates a `traceparent` / operation ID through the host. Log records emitted via the Python `logging` module are correlated to the invocation under the root logger. A named logger (`logging.getLogger(__name__)`) would not change correlation — it is already handled by the Functions host adapter. However, custom enrichment (e.g., including blob name in a logging `extra` dict) would improve query-ability.

---

## 5. Error Observability Assessment

### 5.1 Stack trace logging

The main `Loader` function catches exceptions and logs tracebacks with `traceback.format_exc()`:

```python
logging.error(f"Failed to parse blob content as JSON: {e}")
logging.error(traceback.format_exc())
```

This produces two separate log entries rather than a single correlated record. The recommended approach is `logging.exception(...)` or `logging.error(..., exc_info=True)`, which attaches the traceback to the same log record and is correctly picked up by Application Insights as an exception telemetry item.

### 5.2 `AISearchIndexLoader.populate_search_index` error handling

Line 184: `self.logger.error("Error in AI Search: %s", ex)` does not include `exc_info=True` or use `self.logger.exception(...)`. The original exception is re-raised (`raise ex`), which causes the Azure Functions runtime to surface it — however, Application Insights will record the exception via the re-raise path, not the explicit log call. The log call at line 184 therefore adds an error log without an attached stack trace, creating ambiguity.

### 5.3 Unhandled code path outside try/except

Lines 99–101 execute unconditionally outside the main `try/except` block:

```python
blob_client = blob_service_client.get_blob_client(container=container_name, blob=blob_name)
blob_client.delete_blob()
logging.info(f"Deleted blob: {myblob.name}")
```

If the exception branch at lines 92–97 is taken, `blob_service_client` and `container_name` are undefined, causing a `NameError` that propagates to the Functions runtime as an unhandled exception. This is both a correctness bug and an observability gap — the runtime will emit an exception telemetry item for the wrong reason (cleanup `NameError` rather than the original parse/processing failure).

### 5.4 Token refresh errors

`credential.get_token(...)` (line 51) is called without error handling. A token acquisition failure will raise an `azure.core.exceptions.ClientAuthenticationError` that propagates through the outer `try/except Exception as e` block. However, this exception is logged as `"loader Failed: {e}"` with no indication that the root cause is authentication, making diagnosis difficult.

---

## 6. Log Scrubbing Assessment

### 6.1 Document content logged at INFO

Line 68:

```python
logging.info(f"Blob content as JSON: {json_data}")
```

`json_data` is the full parsed document including title, description, resolution steps, reference code, and product ID. If these documents contain PII (e.g., customer names in error descriptions, product identifiers correlated to personal accounts), this logs it in full to Application Insights. Application Insights logs are retained and queryable by anyone with workspace access.

### 6.2 Token written to environment variable

Lines 52–54:

```python
environ["OPENAI_API_KEY"] = token
environ["AZURE_OPENAI_AD_TOKEN"] = environ["OPENAI_API_KEY"]
```

The Azure AD bearer token is written to environment variables. If the Azure Functions runtime or any diagnostic tool ever dumps environment state (e.g., via the Kudu debug console, diagnostic dumps), this token would appear in plain text. This is not strictly a logging issue but falls within the log scrubbing / sensitive data scope. The token is not logged directly, so this is a minor concern here (the Security Steward owns the broader secret handling concern).

### 6.3 Connection strings and secrets in logs

No connection strings, API keys, or secrets are logged directly. The `AZURE_OPENAI_API_KEY` environment variable value is set from a token, not a static secret, and is not referenced in any log call.

---

## 7. Findings

| Severity | ID | Title | File |
|---|---|---|---|
| 🔴 Critical | PYOTL-ERR-001 | Blob delete outside try/except causes NameError masking original failure | `src/DocumentLoaderFunction/function_app.py` |
| 🟡 Notable | PYOTL-MON-001 | APPINSIGHTS_INSTRUMENTATIONKEY used instead of APPLICATIONINSIGHTS_CONNECTION_STRING | `infra/app/loader-function.bicep` |
| 🟡 Notable | PYOTL-LOG-001 | Root logger used instead of named module logger | `src/DocumentLoaderFunction/function_app.py` |
| 🟡 Notable | PYOTL-ERR-002 | Exceptions logged as two separate records instead of using logging.exception() | `src/DocumentLoaderFunction/function_app.py` |
| 🟡 Notable | PYOTL-LOG-002 | Full document content logged at INFO — potential PII exposure in Application Insights | `src/DocumentLoaderFunction/function_app.py` |
| 🟡 Notable | PYOTL-TRC-001 | Blob name absent from error log messages — hinders cross-document error correlation | `src/DocumentLoaderFunction/function_app.py` |
| 🟢 Minor | PYOTL-LOG-003 | F-string formatting in log calls instead of lazy % formatting | `src/DocumentLoaderFunction/function_app.py` |
| 🟢 Minor | PYOTL-ERR-003 | AI Search error log missing exc_info — no stack trace attached to log record | `src/DocumentLoaderFunction/function_app.py` |
| 🟢 Minor | PYOTL-LOG-004 | Index-not-found logged at INFO instead of WARNING | `src/DocumentLoaderFunction/function_app.py` |
| ℹ️ Info | PYOTL-MON-002 | Application Insights and Log Analytics are provisioned and diagnostic settings are configured | `infra/app/loader-function.bicep` |
| ℹ️ Info | PYOTL-MON-003 | No print() statements used — logging module is used throughout | `src/DocumentLoaderFunction/function_app.py` |

---

## 8. Recommended Improvements

| Finding | Recommended Action | Priority |
|---|---|---|
| PYOTL-ERR-001 | Move the blob cleanup (lines 99–101) inside a `finally` block, or guard with `if blob_service_client` checks to prevent masking the original exception | High |
| PYOTL-MON-001 | Replace `APPINSIGHTS_INSTRUMENTATIONKEY` with `APPLICATIONINSIGHTS_CONNECTION_STRING` in `loader-function.bicep`; use the `connectionString` output already produced by `applicationinsights.bicep` | High |
| PYOTL-LOG-002 | Remove or replace `logging.info(f"Blob content as JSON: {json_data}")` with a safe summary log such as the document reference code and title only — never log full document content | High |
| PYOTL-TRC-001 | Include `myblob.name` in all error log messages inside the `Loader` function; consider using a logging `LoggerAdapter` with `{"blob_name": myblob.name}` as extra context | Medium |
| PYOTL-ERR-002 | Replace the two-step `logging.error(...)` + `logging.error(traceback.format_exc())` pattern with `logging.exception(...)` or `logging.error(..., exc_info=True)` so Application Insights receives a single correlated exception telemetry item | Medium |
| PYOTL-LOG-001 | Replace all `logging.info(...)` / `logging.error(...)` root-logger calls with `logger = logging.getLogger(__name__)` at module level and use `logger.info(...)` / `logger.error(...)` | Medium |
| PYOTL-ERR-003 | Add `exc_info=True` to `self.logger.error("Error in AI Search: %s", ex)` so the stack trace is attached to the log record | Low |
| PYOTL-LOG-003 | Replace f-string log arguments with `%`-style lazy formatting or structured keyword arguments to avoid unnecessary string evaluation when the log level is filtered | Low |
| PYOTL-LOG-004 | Change the `"AI Search index not found, creating index..."` log call from `self.logger.info(...)` to `self.logger.warning(...)` to signal an unexpected initial state | Low |

### Suggested code pattern for blob-context logging

```python
import logging

logger = logging.getLogger(__name__)

@app.blob_trigger(arg_name="myblob", path="load", connection="BlobTriggerConnection")
def Loader(myblob: func.InputStream):
    blob_name = myblob.name
    logger.info("Blob trigger fired. name=%s size=%d", blob_name, myblob.length)

    try:
        # ... processing ...
    except Exception as e:
        logger.exception("Loader failed. blob_name=%s", blob_name)
        raise
    finally:
        # blob cleanup — always runs, avoids NameError masking
        ...
```

---

## 9. Observability Maturity Checklist

| Check | Status | Notes |
|---|---|---|
| Python `logging` module used (no print statements) | PASS | No print() statements found |
| Named module logger (`logging.getLogger(__name__)`) | FAIL | Root logger used throughout |
| Appropriate log levels assigned | PARTIAL | INFO used where WARNING is more appropriate in one case |
| F-string formatting avoided in log calls | FAIL | F-strings used in most log calls |
| Application Insights provisioned in infra | PASS | Bicep provisions App Insights + Log Analytics |
| APPLICATIONINSIGHTS_CONNECTION_STRING configured | FAIL | Instrumentation key used instead of connection string |
| azure-monitor-opentelemetry package present | FAIL | Not in requirements.txt; relying on host auto-instrumentation only |
| Exceptions logged with stack traces (single record) | FAIL | Two-call pattern used instead of logging.exception() |
| Blob name included in all error messages | FAIL | Error messages omit blob name context |
| Document content not logged at INFO/DEBUG | FAIL | Full JSON content logged at INFO (PII risk) |
| Sensitive tokens not exposed in logs | PASS | Token is set in env vars, not logged directly |
| Blob cleanup inside try/finally | FAIL | Cleanup is outside try/except block — NameError risk |
| Custom spans for critical operations | FAIL | No custom spans; no azure-monitor-opentelemetry |
| Diagnostic settings forward logs to Log Analytics | PASS | FunctionAppLogs + AllMetrics forwarded |

---

## Footer

This review is based on static analysis of source files and infrastructure templates as of 2026-03-22. No runtime execution, dynamic profiling, or build-time validation was performed. Findings reflect the code as written — actual runtime behavior may differ if environment variables or host configuration are applied outside the files reviewed.

Generated by: Python Observability Steward (`python-otl-steward`)