fix(mobile): HttpClient instrumentation via DelegatingHandler for mobile↔API correlation#172
Merged
Merged
Conversation
…tion PR #165 (mobile) + PR #166 (server) shipped the App Insights pipeline end to end, but the mobile↔API correlation join in App Insights was returning zero rows. Every server request had `operation_Id == operation_ParentId` — i.e., no `traceparent` header was arriving from the device. Diagnosis (see #171): - `OpenTelemetry.Instrumentation.Http`'s `AddHttpClientInstrumentation()` was already on the MAUI TracerProvider since commit 216a2da and a trim-disabled Release build on DX24 produced the same zero-span result, so neither registration nor trimming was the problem. - Mobile logs had empty `operation_Id` across the board, confirming no ambient `Activity` ever existed on the device. - Root cause (tracked in #171): MAUI's `MauiApp` doesn't run `IHostedService` instances, so the `TelemetryHostedService` that would normally materialize the TracerProvider and attach its listeners never runs. Logs work because they hook `ILoggerFactory` synchronously; the tracer path needs the hosted-service startup. This PR: - Adds `ApiActivityHandler`, a `DelegatingHandler` that starts a `Client` Activity per outbound API call using a dedicated `ActivitySource` (`SentenceStudio.Mobile.HttpClient`). With an Activity current, HttpClient's built-in `DiagnosticsHandler` auto-injects the W3C `traceparent` header. - Registers the new ActivitySource on the mobile TracerProvider via `.AddSource(...)` in `MauiServiceDefaults.Extensions` so the spans actually export. - Wires the handler onto every API-bound HttpClient: CoreSync's `HttpClientToServer`, the auth client, the four typed API clients, and `VersionCheckService`. The handler is placed FIRST in the chain so the span wraps the full operation including auth token attachment. - Hardens `OpenTelemetryInitializer` to call `GetRequiredService<T>()` instead of the nullable `GetService<T>()` for all three providers, so a misregistration fails loudly at startup instead of silently breaking telemetry at runtime. Out of scope (explicitly): - Root-cause fix for the IHostedService gap — tracked in #171. - The raw `new HttpClient()` in `SentenceStudio.Shared/Services/AiService.cs:93` — bypasses `HttpClientFactory` entirely. Separate refactor. - The KQL in `docs/deploy-runbook.md` is still wrong (joins requests to requests; should be dependencies to requests). Separate doc PR. Verification: Mac Catalyst Debug + Release both build clean. Post-merge verification will be an iOS publish to DX24 + KQL query for non-empty `operation_ParentId` on server requests.
4514a2f to
20b6b77
Compare
…recording Code review feedback on #172: exceptions should be recorded as Activity events (via AddException/RecordException), not raw tags. Emits the standard OTel 'exception' event with type/message/stacktrace, which surfaces in App Insights' exception timeline rather than being tag-only. Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
davidortinau
added a commit
that referenced
this pull request
Apr 22, 2026
…tion (#173) PR #172 got mobile HttpClient dependency spans emitting with operation_Id, but the correlation join against API requests still returned zero rows: the API saw every incoming request without a traceparent header and started a fresh operation_Id. Root cause: HttpClient's built-in DiagnosticsHandler only injects traceparent automatically when an OTel-style ActivityListener is attached to "System.Net.Http". On MAUI the listener never attaches because OpenTelemetry's TelemetryHostedService — which wires listeners to the TracerProvider — relies on IHostedService, and MauiApp doesn't run hosted services (issue #171). Fix: have ApiActivityHandler explicitly call DistributedContextPropagator.Current.Inject(...) on the outbound request headers after starting its Activity. Guards against double-injection if a caller or a resilience retry already set traceparent. This is the user-space workaround to #171. Framework fix is still desirable but now lower priority. Verification plan: re-run the App Insights correlation join; expect requests | join dependencies on operation_Id to return > 0 rows for the mobile role name. Refs: #165 #166 #172 #171
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
What
Restores mobile↔API correlation in Application Insights by wrapping every API-bound mobile
HttpClientwith a manualApiActivityHandlerDelegatingHandlerthat starts aClientActivity per outbound request. WithActivity.Currentnon-null, HttpClient auto-injects the W3Ctraceparentheader, which propagatesoperation_Idfrom mobile through the server request.Why
PR #165 (mobile App Insights bootstrap) + PR #166 (server companion) shipped the end-to-end telemetry plumbing, but the
requests | join traces on operation_Idcorrelation query returned zero rows. Every server request showedoperation_Id == operation_ParentId, meaning notraceparentwas arriving from the device.Two incorrect diagnoses during this investigation (see session log), finally landing on:
AddHttpClientInstrumentation()was already wired (since commit 216a2da, March 4) — my first "fix" would have been a no-op.<MtouchLink>None</MtouchLink>(136 MB, 333 DLLs) on DX24 produced the same 0-dependency result.MauiAppdoesn't runIHostedServiceinstances, soTelemetryHostedService— which normally materializes theTracerProviderand attaches its listeners — never runs. Logs still work (they hookILoggerFactorysynchronously), but auto HTTP instrumentation does not.How
ApiActivityHandler— dedicatedActivitySource(SentenceStudio.Mobile.HttpClient),Clientkind, OTel-conformant tags (http.request.method,url.full,server.address,http.response.status_code), error status on exception +>=400, properActivitydisposal.MauiServiceDefaults.Extensions:.AddSource("SentenceStudio.Mobile.HttpClient")on theTracerProviderso the new spans actually export.GetRequiredService<T>()instead of nullableGetService<T>()for all three providers so silent misregistration becomes a loud startup failure.ServiceCollectionExtentions+SentenceStudioAppBuilder— handler placed FIRST in each chain (ahead ofAuthenticatedHttpMessageHandler) so the span covers the full operation including auth token attachment. Registered viaTryAddTransient<>()so the several entry points don't step on each other.Verification
0 Error(s)/ 571 warnings ✅0 Error(s)/ 689 warnings ✅operation_ParentIdon server requests — Captain's call on timing.Out of scope
IHostedServicegap (MAUI OTel: force TracerProvider materialization since IHostedService doesn't run #171).new HttpClient()inSentenceStudio.Shared/Services/AiService.cs:93— bypasses the factory; separate refactor.Risk
Source.StartActivity(...)yields null (no listener attached), so behavior is identical to the current code path on any surface where the source isn't registered.GetRequiredServicechange converts a silent failure into a startup crash if providers aren't registered. This is intentional — today's silent failure is the reason we're here.Follow-ups
TracerProvidermaterialization withoutIHostedService