[ASM] Log endpoints collection failures as warnings, not errors - #9030
Draft
dromanol wants to merge 3 commits into
Draft
[ASM] Log endpoints collection failures as warnings, not errors#9030dromanol wants to merge 3 commits into
dromanol wants to merge 3 commits into
Conversation
Reading EndpointDataSource.Endpoints executes third-party code that can throw for reasons outside our control. In the Azure Functions isolated worker, FunctionsEndpointDataSource builds its endpoints lazily and rejects malformed route templates, so collecting endpoints at Kestrel startup surfaces a RoutePatternException. Endpoints collection is best-effort and the failure does not affect the application, so it should not be reported to telemetry as an error. The other failure paths in the same method already use Log.Warning. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 186e74ff58
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| // rejects a malformed route template). Endpoints collection is best-effort, so a failure | ||
| // here degrades the feature without affecting the application: warn rather than report | ||
| // an error to telemetry. | ||
| Log.Warning(ex, "API Security: Failed to collect endpoints."); |
There was a problem hiding this comment.
When the exception originates in Datadog's own collection path rather than the third-party EndpointDataSource.Endpoints getter, this broad catch now suppresses it from error telemetry too. For example, failures in EndpointsCollection.CollectEndpoints or ReportEndpoints are tracer defects that should remain errors; narrow the warning handling to evaluation of the third-party getter and retain error-level reporting for unexpected collector exceptions. The repository logging guidance explicitly requires tracer bugs and unexpected outer-catch failures to use Log.Error.
AGENTS.md reference: AGENTS.md:L205-L210
Useful? React with 👍 / 👎.
Downgrading the whole catch also suppressed error telemetry for defects in our own collection path (EndpointsCollection.CollectEndpoints, ReportEndpoints, duck casts), which the logging guidance requires to stay at Log.Error. Wrap only the EndpointDataSource.Endpoints evaluation, the third-party code we cannot control, and restore Log.Error on the outer catch. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
BenchmarksBenchmark execution time: 2026-08-12 10:42:27 Comparing candidate commit f68775a in PR branch Found 0 performance improvements and 1 performance regressions! Performance is the same for 71 metrics, 0 unstable metrics, 67 known flaky benchmarks, 59 flaky benchmarks without significant changes.
|
Execution-Time Benchmarks Report ⏱️Execution-time results for samples comparing This PR (9030) and master. ✅ No regressions detected |
dromanol
marked this pull request as draft
August 12, 2026 11:56
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.
https://app.datadoghq.com/error-tracking/issue/fc6528a2-9617-11f1-a90b-da7ad0900002
Summary of changes
Wrap the
EndpointDataSource.Endpointsevaluation inKestrelServerImplStartAsyncIntegration.GatherEndpointsin its owntry/catchthat logs atLog.Warning. The outer catch keepsLog.Error.Reason for change
API Security endpoints collection reads
EndpointDataSource.Endpointsat Kestrel startup. Evaluating that property executes third-party code that can throw for reasons outside our control: in the Azure Functions isolated worker,FunctionsEndpointDataSourcebuilds its endpoints lazily from function metadata and rejects malformed route templates, producing aRoutePatternExceptionthat shows up in Error Tracking.Because routing only enumerates endpoints on the first request, we are the first — often the only — caller to trigger that build, so a latent app-side defect surfaces as a tracer error. It is non-fatal:
CompositeEndpointDataSourcecaches only on success, so the application still builds its endpoints normally when a request arrives.Implementation details
Only the third-party getter is downgraded. Exceptions from our own collection path —
EndpointsCollection.CollectEndpoints,ReportEndpoints, duck casts — are tracer defects and still reach the outer catch atLog.Error, per the logging guidance inAGENTS.md.Test coverage
None — log level only, no behaviour change.
Other details
APPSEC-69625. Deliberately does not skip collection under Azure Functions: specialization env vars are applied before the customer app loads, so eager evaluation reads correct metadata, and skipping would regress Azure Functions AAP coverage.
Separately tracked on the ticket: the endpoints we collect from
FunctionsEndpointDataSourceare themselves unvalidated (RoutePattern.RawTextkeeps a leading slash for these endpoints, and azure-functions-dotnet-worker#3009 means the route prefix can disagree with what the host routes on). Out of scope here.🤖 Generated with Claude Code