Skip to content

Commit b410f8f

Browse files
authored
Merge pull request #96 from lwalden/fix/s5-004r-func-host-boot
fix: align ApplicationInsights packages so func host boots (S5-004r)
2 parents f105dcf + 364c9eb commit b410f8f

4 files changed

Lines changed: 41 additions & 12 deletions

File tree

DECISIONS.md

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -346,6 +346,8 @@ Select which sleeve(s) activate first and final initial capital split/account ma
346346
| KD-002 | Claude API key not provisioned — NOT required under default posture: ADR-029/030 gateway-or-rules with `DirectApiFallbackEnabled=false` means zero metered spend (gateway bearer key provisioned via S5-004). Becomes relevant only if the owner explicitly enables the metered fallback | Conditional — regime integration shipped, degrades to deterministic rules | 2026-02-16 |
347347
| KD-003 | Backtest pipeline paths use Python (not .NET) | Must install Python + deps separately for backtesting | 2026-04-07 |
348348
| KD-004 | ✅ RESOLVED 2026-05-29 (S3-001, PR #70). ~~CachingMarketDataService.cs ~360 lines, exceeds 300-line architecture-fitness threshold (grew across S2-001/003/005/006)~~ — decomposed into a 125-line facade + new 280-line MarketRegimeProvider (internal composition, no DI change, zero behavior change) per ADR-017 | Maintainability — closed | 2026-05-29 |
349+
| KD-005 | **ApplicationInsights 2.x→3.x upgrade gate (S5-004r).** `Microsoft.ApplicationInsights.WorkerService` is pinned to the classic 2.x line (2.23.0): AI 3.x is OTel-based and removed classic API types (`ITelemetryInitializer`) that `Microsoft.Azure.Functions.Worker.ApplicationInsights` 2.50.0 is compiled against — pairing them crashes the isolated worker at DI bootstrap with `TypeLoadException`. Upgrade is BLOCKED until the Functions Worker AI package ships a 3.x-compatible release; when unblocking, re-evaluate the OpenTelemetry.Api pin removed in S5-004r (see comment in `TradingSystem.Functions.csproj`) and clear KD-006 in the same change | Stuck on classic 2.x telemetry SDK; no OTel-native pipeline. Boot-blocking if upgraded prematurely | 2026-06-10 |
350+
| KD-006 | **discord.com URI-redaction gate before enabling App Insights (S5-004r).** AI 2.x *dependency* telemetry records the full request URL of outbound HTTP calls — for the Discord named clients that URL IS the token-bearing webhook URL. This path bypasses the `System.Net.Http.HttpClient` ILogger filter in `Program.cs` (that filter only covers log telemetry) and is dormant solely because `APPLICATIONINSIGHTS_CONNECTION_STRING` is unset everywhere. Do NOT set that variable until a telemetry processor/initializer that redacts or drops discord.com dependency URLs is in place (or AI 3.x with OTel redaction supersedes it — KD-005) | Secret-leak risk into App Insights if telemetry is ever enabled without redaction | 2026-06-10 |
349351

350352
---
351353

docs/paper-validation-runbook.md

Lines changed: 7 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -43,7 +43,13 @@ Run this check each trading morning **before 5:00 AM PT in winter (PST) / 6:00 A
4343
classification (ADR-029/ADR-030) — **this is expected behavior, not an incident**.
4444
Restart the gateway (`D:\Source\claude-gateway`) when convenient.
4545
3. **Functions worker** started (`func start` per section 1).
46-
4. **Startup-log smoke check:** the worker banner lists both timer functions —
46+
4. **Host boot smoke:** after `func start`, the function-list banner must appear with **no
47+
`TypeLoadException`** (or repeated "worker process" restarts) in the startup output.
48+
The banner proves the isolated worker process actually loaded its DI graph — a
49+
package-version mismatch (S5-004r) can pass the full unit suite yet crash the worker
50+
at bootstrap, because tests never load the Functions host. Run this check first: the
51+
function list in step 5 is meaningless until the worker has provably booted.
52+
5. **Startup-log smoke check:** the worker banner lists both timer functions —
4753
`DailyOrchestrator_PreMarket` and `DailyOrchestrator_EndOfDay`. If either is missing,
4854
the build is broken or the wrong project started; do not assume the day will run.
4955

src/TradingSystem.Functions/Program.cs

Lines changed: 20 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,12 @@
4040
context.Configuration.GetSection("TradingSystem"));
4141
services.Configure<TacticalConfig>(
4242
context.Configuration.GetSection("TradingSystem:Tactical"));
43+
// S5-004r: OptionsSleeveManager takes the bare TacticalConfig (its siblings take
44+
// IOptions<TacticalConfig>), so the bare type must be resolvable or the worker dies
45+
// at DI validation on boot. This was masked by the AI-package TypeLoadException —
46+
// the unit suite never builds the full host graph, only the real `func start` does.
47+
services.AddSingleton(sp =>
48+
sp.GetRequiredService<Microsoft.Extensions.Options.IOptions<TacticalConfig>>().Value);
4349
services.Configure<IBKRConfig>(
4450
context.Configuration.GetSection("IBKR"));
4551
services.Configure<LocalStorageConfig>(
@@ -53,7 +59,12 @@
5359
services.Configure<ReportingConfig>(
5460
context.Configuration.GetSection("Reporting"));
5561

56-
// Application Insights
62+
// Application Insights — registration is inert until APPLICATIONINSIGHTS_CONNECTION_STRING
63+
// is set (it is NOT set in local.settings.json or any deployed config today): with no
64+
// connection string the SDK builds a disabled TelemetryConfiguration and sends nothing.
65+
// Before ever setting that variable, the discord.com URI-redaction gate in DECISIONS.md
66+
// (Known Debt KD-005/KD-006) must be satisfied — AI 2.x dependency telemetry records full
67+
// request URLs, which for the Discord named clients are token-bearing webhook URLs.
5768
services.AddApplicationInsightsTelemetryWorkerService();
5869
services.ConfigureFunctionsApplicationInsights();
5970

@@ -124,8 +135,14 @@
124135
builder.SetMinimumLevel(LogLevel.Information);
125136
// S5-004 (review): IHttpClientFactory's LogicalHandler logs the full request URI at
126137
// Information on every send — for Discord named clients that URI IS the token-bearing
127-
// webhook URL. Warning+ only for the HttpClient categories keeps the secret out of
128-
// console/App Insights logs while preserving error visibility.
138+
// webhook URL. Warning+ for every "System.Net.Http.HttpClient.*" ILogger category
139+
// keeps the secret out of console logs and out of App Insights *log* telemetry while
140+
// preserving error visibility. Scope note (S5-004r review): this filter governs only
141+
// ILogger output — it does NOT cover App Insights *dependency* telemetry, which the
142+
// AI 2.x SDK emits with the full request URL outside the logging pipeline. That path
143+
// is dormant solely because APPLICATIONINSIGHTS_CONNECTION_STRING is unset (see the
144+
// enablement-gate comment at the App Insights registration above and DECISIONS.md
145+
// Known Debt).
129146
builder.AddFilter("System.Net.Http.HttpClient", LogLevel.Warning);
130147
});
131148
})

src/TradingSystem.Functions/TradingSystem.Functions.csproj

Lines changed: 12 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,21 @@
1717
<PackageReference Include="Microsoft.Azure.Functions.Worker.Extensions.Http" Version="3.3.0" />
1818
<PackageReference Include="Microsoft.Azure.Functions.Worker.Extensions.Timer" Version="4.3.1" />
1919
<PackageReference Include="Microsoft.Azure.Functions.Worker.Sdk" Version="1.16.2" />
20-
<PackageReference Include="Microsoft.ApplicationInsights.WorkerService" Version="3.0.0" />
20+
<!-- S5-004r: MUST stay on the classic 2.x line. AI 3.x is OTel-based and REMOVED classic
21+
API types (e.g. ITelemetryInitializer) that Functions.Worker.ApplicationInsights is
22+
built against — pairing 3.x with it makes the isolated worker crash at DI bootstrap
23+
with TypeLoadException and the host recycles until the worker-restart cap. -->
24+
<PackageReference Include="Microsoft.ApplicationInsights.WorkerService" Version="2.23.0" />
2125
<PackageReference Include="Microsoft.Azure.Functions.Worker.ApplicationInsights" Version="2.50.0" />
2226
<PackageReference Include="Azure.Identity" Version="1.18.0" />
2327
<PackageReference Include="Azure.Extensions.AspNetCore.Configuration.Secrets" Version="1.5.0" />
24-
<!-- Transitive lift: GHSA-g94r-2vxg-569j (OpenTelemetry.Api < 1.15.3, excessive memory
25-
allocation parsing propagation headers). Pulled in at 1.15.0 via
26-
Microsoft.ApplicationInsights.WorkerService 3.0.0 -> Azure.Monitor.OpenTelemetry.Exporter.
27-
Pinned to the first patched version; remove once ApplicationInsights.WorkerService ships
28-
a patched chain. Tests and SmokeTest inherit this pin via their project reference to
29-
Functions — if either drops that reference, it needs its own pin. -->
30-
<PackageReference Include="OpenTelemetry.Api" Version="1.15.3" />
28+
<!-- S5-004r: the S5-005 OpenTelemetry.Api 1.15.3 pin that used to live here was REMOVED,
29+
not lost. The vulnerable transitive chain it patched (GHSA-g94r-2vxg-569j, via
30+
ApplicationInsights.WorkerService 3.0.0 -> Azure.Monitor.OpenTelemetry.Exporter) does
31+
not exist on the classic 2.23.0 line above — a transitive vulnerability scan
32+
(dotnet list package, vulnerable + include-transitive) is clean without it. If
33+
WorkerService is ever upgraded back to 3.x (see DECISIONS.md Known Debt),
34+
re-evaluate whether an OTel pin is needed. -->
3135
</ItemGroup>
3236

3337
<ItemGroup>

0 commit comments

Comments
 (0)