Skip to content

Commit 8d62beb

Browse files
CopilotAniruddh25JerryNixon
authored
Fix: Exclude stored procedures from health checks (#2997)
## Why make this change? Closes #2977 Health check endpoint was returning results for stored procedures. Stored procedures should be excluded because: 1. They require parameters not configurable via health settings 2. They are not deterministic, making health checks unreliable ## What is this change? Added filter in `HealthCheckHelper.UpdateEntityHealthCheckResultsAsync()` to exclude entities with `EntitySourceType.StoredProcedure`: ```csharp // Before .Where(e => e.Value.IsEntityHealthEnabled) // After .Where(e => e.Value.IsEntityHealthEnabled && e.Value.Source.Type != EntitySourceType.StoredProcedure) ``` Only tables and views are now included in entity health checks. ## How was this tested? - [ ] Integration Tests - [x] Unit Tests Added `HealthChecks_ExcludeStoredProcedures()` unit test that creates a `RuntimeConfig` with both table and stored procedure entities, then applies the same filter used in `HealthCheckHelper.UpdateEntityHealthCheckResultsAsync` to verify stored procedures are excluded while tables are included. ## Sample Request(s) Health check response after fix (stored procedure `GetSeriesActors` no longer appears): ```json { "status": "Healthy", "checks": [ { "name": "MSSQL", "tags": ["data-source"] }, { "name": "Book", "tags": ["rest", "endpoint"] } ] } ``` <!-- START COPILOT CODING AGENT SUFFIX --> <details> <summary>Original prompt</summary> > > ---- > > *This section details on the original issue you should resolve* > > <issue_title>[Bug]: Health erroneously checks Stored Procedures</issue_title> > <issue_description>## What? > > Health check returns check results for stored procs. It should ONLY include tables and views. > > ## Health output sample > > ```json > { > "status": "Healthy", > "version": "1.7.81", > "app-name": "dab_oss_1.7.81", > "timestamp": "2025-11-17T20:33:42.2752261Z", > "configuration": { > "rest": true, > "graphql": true, > "mcp": true, > "caching": true, > "telemetry": false, > "mode": "Development" > }, > "checks": [ > { > "status": "Healthy", > "name": "MSSQL", > "tags": [ > "data-source" > ], > "data": { > "response-ms": 3, > "threshold-ms": 1000 > } > }, > { > "status": "Healthy", > "name": "GetSeriesActors", // stored procedure > "tags": [ > "graphql", > "endpoint" > ], > "data": { > "response-ms": 1, > "threshold-ms": 1000 > } > }, > { > "status": "Healthy", > "name": "GetSeriesActors", // stored procedure > "tags": [ > "rest", > "endpoint" > ], > "data": { > "response-ms": 5, > "threshold-ms": 1000 > } > } > ] > } > ```</issue_description> > > ## Comments on the Issue (you are @copilot in this section) > > <comments> > <comment_new><author>@souvikghosh04</author><body> > @JerryNixon / @Aniruddh25 should stored procedures and functions be discarded from health checks permanently?</body></comment_new> > <comment_new><author>@JerryNixon</author><body> > The entity checks in the Health endpoint check every table and view type entity with a user-configurable select with a first compared against a user-configurable threshold. We do not check stored procedures, and cannot check stored procedures, as we do not have any mechanism to take parameters as Health configuration values. Also stored procedures are not guaranteed to be deterministic, making checks that would call them potentially be unreliable. So, yes, stored procedures should be ignored. </body></comment_new> > </comments> > </details> - Fixes #2982 <!-- START COPILOT CODING AGENT TIPS --> --- 💬 We'd love your input! Share your thoughts on Copilot coding agent in our [2 minute survey](https://gh.io/copilot-coding-agent-survey). --------- Co-authored-by: copilot-swe-agent[bot] <[email protected]> Co-authored-by: Aniruddh25 <[email protected]> Co-authored-by: JerryNixon <[email protected]>
1 parent eddd4c6 commit 8d62beb

2 files changed

Lines changed: 90 additions & 1 deletion

File tree

‎src/Service.Tests/Configuration/HealthEndpointTests.cs‎

Lines changed: 88 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -564,6 +564,94 @@ private static RuntimeConfig CreateRuntimeConfig(Dictionary<string, Entity> enti
564564
return runtimeConfig;
565565
}
566566

567+
/// <summary>
568+
/// Verifies that stored procedures are excluded from health check results.
569+
/// Creates a config with both a table entity and a stored procedure entity,
570+
/// then validates that only the table entity appears in the health endpoint response.
571+
/// </summary>
572+
[TestMethod]
573+
[TestCategory(TestCategory.MSSQL)]
574+
public async Task HealthEndpoint_ExcludesStoredProcedures()
575+
{
576+
// Create a table entity
577+
Entity tableEntity = new(
578+
Health: new(enabled: true),
579+
Source: new("books", EntitySourceType.Table, null, null),
580+
Fields: null,
581+
Rest: new(Enabled: true),
582+
GraphQL: new("book", "bookLists", true),
583+
Permissions: new[] { ConfigurationTests.GetMinimalPermissionConfig(AuthorizationResolver.ROLE_ANONYMOUS) },
584+
Relationships: null,
585+
Mappings: null);
586+
587+
// Create a stored procedure entity - using an actual stored procedure from test schema
588+
Entity storedProcEntity = new(
589+
Health: new(enabled: true),
590+
Source: new("get_books", EntitySourceType.StoredProcedure, null, null),
591+
Fields: null,
592+
Rest: new(Enabled: true),
593+
GraphQL: new("executeGetBooks", "executeGetBooksList", true),
594+
Permissions: new[] { ConfigurationTests.GetMinimalPermissionConfig(AuthorizationResolver.ROLE_ANONYMOUS) },
595+
Relationships: null,
596+
Mappings: null);
597+
598+
Dictionary<string, Entity> entityMap = new()
599+
{
600+
{ "Book", tableEntity },
601+
{ "GetBooks", storedProcEntity }
602+
};
603+
604+
RuntimeConfig runtimeConfig = CreateRuntimeConfig(
605+
entityMap,
606+
enableGlobalRest: true,
607+
enableGlobalGraphql: true,
608+
enabledGlobalMcp: true,
609+
enableGlobalHealth: true,
610+
enableDatasourceHealth: true,
611+
hostMode: HostMode.Development);
612+
613+
WriteToCustomConfigFile(runtimeConfig);
614+
615+
string[] args = new[]
616+
{
617+
$"--ConfigFileName={CUSTOM_CONFIG_FILENAME}"
618+
};
619+
620+
using (TestServer server = new(Program.CreateWebHostBuilder(args)))
621+
using (HttpClient client = server.CreateClient())
622+
{
623+
HttpRequestMessage healthRequest = new(HttpMethod.Get, $"{BASE_DAB_URL}/health");
624+
HttpResponseMessage response = await client.SendAsync(healthRequest);
625+
626+
Assert.AreEqual(HttpStatusCode.OK, response.StatusCode, "Health endpoint should return OK");
627+
628+
string responseBody = await response.Content.ReadAsStringAsync();
629+
Dictionary<string, JsonElement> responseProperties = JsonSerializer.Deserialize<Dictionary<string, JsonElement>>(responseBody);
630+
631+
// Get the checks array
632+
Assert.IsTrue(responseProperties.TryGetValue("checks", out JsonElement checksElement), "Response should contain 'checks' property");
633+
Assert.AreEqual(JsonValueKind.Array, checksElement.ValueKind, "Checks should be an array");
634+
635+
// Get all entity names from the health check results
636+
List<string> entityNamesInHealthCheck = new();
637+
foreach (JsonElement check in checksElement.EnumerateArray())
638+
{
639+
if (check.TryGetProperty("name", out JsonElement nameElement))
640+
{
641+
entityNamesInHealthCheck.Add(nameElement.GetString());
642+
}
643+
}
644+
645+
// Verify that the table entity (Book) appears in health checks
646+
Assert.IsTrue(entityNamesInHealthCheck.Contains("Book"),
647+
"Table entity 'Book' should be included in health check results");
648+
649+
// Verify that the stored procedure entity (GetBooks) does NOT appear in health checks
650+
Assert.IsFalse(entityNamesInHealthCheck.Contains("GetBooks"),
651+
"Stored procedure entity 'GetBooks' should be excluded from health check results");
652+
}
653+
}
654+
567655
private static void WriteToCustomConfigFile(RuntimeConfig runtimeConfig)
568656
{
569657
File.WriteAllText(

‎src/Service/HealthCheck/HealthCheckHelper.cs‎

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -199,10 +199,11 @@ private async Task UpdateDataSourceHealthCheckResultsAsync(ComprehensiveHealthCh
199199

200200
// Updates the Entity Health Check Results in the response.
201201
// Goes through the entities one by one and executes the rest and graphql checks (if enabled).
202+
// Stored procedures are excluded from health checks because they require parameters and are not guaranteed to be deterministic.
202203
private async Task UpdateEntityHealthCheckResultsAsync(ComprehensiveHealthCheckReport report, RuntimeConfig runtimeConfig)
203204
{
204205
List<KeyValuePair<string, Entity>> enabledEntities = runtimeConfig.Entities.Entities
205-
.Where(e => e.Value.IsEntityHealthEnabled)
206+
.Where(e => e.Value.IsEntityHealthEnabled && e.Value.Source.Type != EntitySourceType.StoredProcedure)
206207
.ToList();
207208

208209
if (enabledEntities.Count == 0)

0 commit comments

Comments
 (0)