Cherry-pick: LogLevel bug fixes#3524
Draft
RubenCerna2079 wants to merge 2 commits intorelease/2.0from
Draft
Conversation
Closes #3267 Alters the validation logic in the following way. Is top-level config with data-source-files? (we call this a `Root` config file) ├── YES │ ├── Has datasource? → ValidateEntityPresence (same rules as non-root) │ ├── No datasource but has entities/autoentities? → ERROR │ └── No datasource, no entities → VALID (children provide everything) │ └── For each child → ValidateNonRootConfig(child, filename) │ └── NO (standalone or child config) ├── No datasource? → ERROR: "data source is required" └── Has datasource → ValidateEntityPresence Note: A top-level config file without any children data-source files is NOT considered a root. And an intermediary config file, ie: is a child, that also has child configs is NOT a root. Only a top-level config with children configs is a Root. Count resolved autoentities from AutoentityResolutionCounts total = manual entities + resolved autoentities total == 0? → ERROR: "No entities found" total > 0 but autoentities discovered nothing? → WARN: "Autoentities configured but none discovered" No double messaging. If total is 0, only the error is recorded, not the warning. Variables (`1` = present / non-empty, `0` = absent / empty): - **DSF** — `data-source-files` present - **DS** — `data-source` present - **E** — manual `entities` count > 0 - **AE** — `autoentities` count > 0 (presence, *not* resolved count) Path is determined by `IsRootConfig = (DSF == 1) && !IsChildConfig`. | # | DSF | DS | E | AE | AE resolved | Path | Expected | Test | |---|:---:|:--:|:-:|:--:|:-----------:|------|----------|------| | 1 | 0 | 0 | 0 | 0 | — | Non-root | **Error**: "data source is required" | `TestNonRootWithNoDataSourceProducesError` | | 2 | 0 | 0 | 0 | 1 | — | Non-root | **Error**: "data source is required" | _covered by #1 — DS check fires first_ | | 3 | 0 | 0 | 1 | 0 | — | Non-root | **Error**: "data source is required" | _covered by #1_ | | 4 | 0 | 0 | 1 | 1 | — | Non-root | **Error**: "data source is required" | _covered by #1_ | | 5 | 0 | 1 | 0 | 0 | — | Non-root | **Error**: "No entities found" | `TestNonRootWithDataSourceAndNoEntitiesProducesError` | | 6a | 0 | 1 | 0 | 1 | 0 | Non-root | **Error**: "No entities found" | `TestNonRootWithDataSourceAndAutoentitiesResolvingZeroProducesError` | | 6b | 0 | 1 | 0 | 1 | >0 | Non-root | **Valid** | `TestNonRootWithDataSourceAndAutoentitiesResolvingEntitiesIsValid` | | 7 | 0 | 1 | 1 | 0 | — | Non-root | **Valid** | `TestNonRootWithDataSourceAndEntitiesIsValid` | | 8a | 0 | 1 | 1 | 1 | 0 | Non-root | **Valid** + **Warn** | `TestNonRootWithEntitiesAndAutoentitiesResolvingZeroLogsWarning` | | 8b | 0 | 1 | 1 | 1 | >0 | Non-root | **Valid** | _covered by #7 / #6b combined_ | | 9 | 1 | 0 | 0 | 0 | — | Root | **Valid** (children carry the load) | `TestRootWithNoDataSourceAndNoEntitiesIsValid`, `TestRootConfigWithNoDataSourceAndNoEntitiesParses` | | 10 | 1 | 0 | 0 | 1 | — | Root | **Error**: "must not define entities or autoentities" | `TestRootWithNoDataSourceButAutoentitiesProducesError` | | 11 | 1 | 0 | 1 | 0 | — | Root | **Error**: "must not define entities" | `TestRootWithNoDataSourceButEntitiesProducesError` | | 12 | 1 | 0 | 1 | 1 | — | Root | **Error** | _covered by #11_ | | 13 | 1 | 1 | 0 | 0 | — | Root (with own DS) | **Error**: "No entities found" | `TestRootWithDataSourceAndNoEntitiesProducesError` | | 14a | 1 | 1 | 0 | 1 | 0 | Root (with own DS) | **Error**: "No entities found" | `TestRootWithDataSourceAndAutoentitiesResolvingZeroProducesError` | | 14b | 1 | 1 | 0 | 1 | >0 | Root (with own DS) | **Valid** | `TestRootWithDataSourceAndAutoentitiesResolvingEntitiesIsValid` | | 15 | 1 | 1 | 1 | 0 | — | Root (with own DS) | **Valid** | `TestRootWithDataSourceAndEntitiesIsValid` | | 16a | 1 | 1 | 1 | 1 | 0 | Root (with own DS) | **Valid** + **Warn** | `TestRootWithEntitiesAndAutoentitiesResolvingZeroLogsWarning` | | 16b | 1 | 1 | 1 | 1 | >0 | Root (with own DS) | **Valid** | _covered by #15 / #14b combined_ | `root.ChildConfigs`) Children are always treated as non-root regardless of their own `data-source-files`. | # | DS | E | AE | AE resolved | Expected | Test | |---|:--:|:-:|:--:|:-----------:|----------|------| | C1 | 0 | 0 | 0 | — | **Error** naming the child file: "data source is required" | `TestChildWithNoDataSourceProducesNamedError` | | C2 | 0 | * | * | — | **Error** naming the child file: "data source is required" | _covered by C1_ | | C3 | 1 | 0 | 0 | — | **Error** naming the child file: "No entities found" | `TestChildWithDataSourceAndNoEntitiesProducesNamedError` | | C4a | 1 | 0 | 1 | 0 | **Error** naming the child file: "No entities found" | `TestChildWithDataSourceAndAutoentitiesResolvingZeroProducesNamedError` | | C4b | 1 | 0 | 1 | >0 | **Valid** | _covered by C5 (resolved entities behave the same as manual entities)_ | | C5 | 1 | 1 | 0 | — | **Valid** | _implicitly via `TestRootWithDataSourceAndEntitiesIsValid` setup_ | | C6a | 1 | 1 | 1 | 0 | **Valid** + **Warn** naming the child file | `TestChildWithEntitiesAndAutoentitiesResolvingZeroLogsNamedWarning` | | C6b | 1 | 1 | 1 | >0 | **Valid** | _covered by C5_ | | Scenario | Expected | Test | |----------|----------|------| | Connection-string error gates entity validation (no entity error fires when DB unreachable) | `IsConfigValid == false` due to connection error only | `TestValidateNonRootZeroEntitiesWithInvalidConnectionString` | | Config with no entities parses cleanly (constructor no longer throws) and `IsConfigValid` returns false without throwing | parse OK, validate fails | `TestValidateConfigWithNoEntitiesProducesCleanError` _(modified)_ | | Root parses successfully without a data source | parse OK, `IsRootConfig == true` | `TestRootConfigWithNoDataSourceAndNoEntitiesParses` | | Non-root with DS and no entities parses successfully | parse OK, `IsRootConfig == false` | `TestNonRootConfigWithDataSourceAndNoEntitiesParses` | | Autoentities present but resolve to nothing — must not crash, must not double-message with "No entities found" | no crash; only "No entities found" if total = 0 | `ValidateAutoentitiesConfiguration` _(modified to `isValidateOnly: true`)_ | New tests: `TestRootConfigWithNoDataSourceAndNoEntitiesParses` Root config (has data-source-files) without datasource parses OK `TestNonRootConfigWithDataSourceAndNoEntitiesParses` Non-root config with datasource + no entities parses OK (validation catches it later) `TestNonRootWithDataSourceAndNoEntitiesProducesError` Calls ValidateDataSourceAndEntityPresence directly, error recorded `TestNonRootWithNoDataSourceProducesError` No datasource, error with "data source is required" `TestNonRootWithDataSourceAndEntitiesIsValid` Datasource + entities, no errors `TestRootWithNoDataSourceAndNoEntitiesIsValid` Root with child, no own datasource, valid `TestRootWithNoDataSourceButEntitiesProducesError` Root with entities but no datasource, error `TestRootWithDataSourceAndEntitiesIsValid` Root with own datasource + entities, valid `TestChildWithDataSourceAndNoEntitiesProducesNamedError` Child with no entities, error names the child file `TestChildWithNoDataSourceProducesNamedError` Child with no datasource, error names the child file `TestNonRootWithDataSourceAndAutoentitiesResolvingZeroProducesError` Non-root with only autoentities that resolve to 0 `TestNonRootWithDataSourceAndAutoentitiesResolvingEntitiesIsValid` Non-root with only autoentities resolving > 0 entities `TestNonRootWithEntitiesAndAutoentitiesResolvingZeroLogsWarning` Non-root with manual entities + autoentities resolving 0 `TestRootWithNoDataSourceButAutoentitiesProducesError` Root with no datasource but autoentities defined `TestRootWithDataSourceAndNoEntitiesProducesError` Root with own datasource and zero entities/autoentities `TestRootWithDataSourceAndAutoentitiesResolvingZeroProducesError` Root with own datasource and autoentities resolving 0 `TestRootWithDataSourceAndAutoentitiesResolvingEntitiesIsValid` Root with own datasource and autoentities resolving > 0 `TestRootWithEntitiesAndAutoentitiesResolvingZeroLogsWarning` Root with own datasource, manual entities, and autoentities resolving 0 `TestChildWithDataSourceAndAutoentitiesResolvingZeroProducesNamedError` Child with autoentities-only resolving 0 `TestChildWithEntitiesAndAutoentitiesResolvingZeroLogsNamedWarning` Child with manual entities + autoentities resolving 0 Modified tests: `TestValidateConfigWithNoEntitiesProducesCleanError` Replaced main's version (expected parse failure) with ours: parse succeeds, IsConfigValid returns false `ValidateAutoentitiesConfiguration` Changed to isValidateOnly: true, asserts no crashes instead of zero errors --------- Co-authored-by: Anusha Kolan <anushakolan10@gmail.com>
Enables MCP clients (like MCP Inspector, Claude Desktop, VS Code Copilot) to receive real-time log output via MCP `notifications/message`. Related: #3274 (depends on PR #3419) When `logging/setLevel` is called with a level other than "none", logs are sent to MCP clients as JSON-RPC notifications: ```json { "jsonrpc": "2.0", "method": "notifications/message", "params": { "level": "info", "logger": "Azure.DataApiBuilder.Service.Startup", "data": "Starting Data API builder..." } } ``` - `McpLogNotificationWriter.cs` - Writes logs as MCP notifications to stdout - `McpLogger.cs` / `McpLoggerProvider.cs` - ILogger implementation for .NET logging pipeline - `McpLogNotificationTests.cs` - Unit tests (8 tests) - `Program.cs` - Registers `McpNotificationWriter` and `McpLoggerProvider` for MCP mode - `McpStdioServer.cs` - Enables notifications when `logging/setLevel` is called - Unit tests: 6 tests covering level mapping, enable/disable, JSON format - Manual testing with MCP Inspector: verified notifications appear when `logging/setLevel` is sent This PR targets `dev/anushakolan/set-log-level` (PR #3419) as it depends on the `logging/setLevel` implementation.
RubenCerna2079
changed the title
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
3 participants
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.
Why make this change?
This change cherry-picks the PRs that fix the LogLevel bugs
What is this change?
nonebug #3318How was this tested?
Existing tests in cherry-pick cover new changes.
Sample Request(s)
N/A