Conversation
Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
…configuration Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
… and telemetry integration Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
…ate empty embeddings Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
…oint/health sub-objects Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
…orization Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
Co-authored-by: JerryNixon <1749983+JerryNixon@users.noreply.github.com>
…lthCheckConfig in converter
robertopc1
requested review from
Alekhya-Polavarapu,
RubenCerna2079,
aaronburtle,
anushakolan,
neeraj-sharma2592,
rusamant,
sourabh1007,
souvikghosh04 and
vadeveka
as code owners
There was a problem hiding this comment.
Pull request overview
Adds a new internal embeddings/vectorization subsystem to DAB, including runtime configuration (runtime.embeddings), an HTTP-backed embedding service with caching/telemetry, an optional REST endpoint, and health-check reporting integration.
Changes:
- Introduces
EmbeddingsOptionsconfig models + JSON schema + custom converter and runtime validation. - Adds
IEmbeddingService/EmbeddingService(OpenAI/Azure OpenAI) with FusionCache L1 caching and OpenTelemetry instrumentation. - Integrates embeddings status into health reporting and adds a new
/embed-style endpoint controller plus CLI configuration flags.
Reviewed changes
Copilot reviewed 24 out of 24 changed files in this pull request and generated 15 comments.
Show a summary per file
| File | Description |
|---|---|
| src/Service/Startup.cs | Registers embeddings services/options and wires embeddings into service startup logging. |
| src/Service/HealthCheck/Model/ConfigurationDetails.cs | Extends health check configuration details to include embeddings flags. |
| src/Service/HealthCheck/HealthCheckHelper.cs | Adds embeddings health check execution + reporting. |
| src/Service/Controllers/EmbeddingController.cs | New REST endpoint controller for embedding generation. |
| src/Service.Tests/UnitTests/EmbeddingsOptionsTests.cs | Unit tests for embeddings config deserialization/serialization and env var replacement. |
| src/Service.Tests/UnitTests/EmbeddingServiceTests.cs | Unit tests for EmbeddingService option behaviors and disabled-path failures. |
| src/Service.Tests/UnitTests/ConfigValidationUnitTests.cs | Adds validation test coverage for embeddings config constraints. |
| src/Core/Services/Embeddings/IEmbeddingService.cs | Defines embeddings service contract + result types. |
| src/Core/Services/Embeddings/EmbeddingTelemetryHelper.cs | Adds embeddings-specific metrics and tracing helpers. |
| src/Core/Services/Embeddings/EmbeddingService.cs | Implements embedding calls, caching, and telemetry hooks. |
| src/Core/Configurations/RuntimeConfigValidator.cs | Adds validation for runtime.embeddings settings (URLs, required fields, endpoint conflicts, etc.). |
| src/Config/RuntimeConfigLoader.cs | Registers the embeddings JSON converter in runtime config serialization options. |
| src/Config/ObjectModel/RuntimeOptions.cs | Adds Embeddings to runtime options and exposes IsEmbeddingsConfigured. |
| src/Config/ObjectModel/Embeddings/EmbeddingsOptions.cs | New embeddings config model with defaults and “effective” helper properties. |
| src/Config/ObjectModel/Embeddings/EmbeddingsHealthCheckConfig.cs | New embeddings health-check config model. |
| src/Config/ObjectModel/Embeddings/EmbeddingsEndpointOptions.cs | New embeddings endpoint config + role checks. |
| src/Config/ObjectModel/Embeddings/EmbeddingProviderType.cs | Defines supported providers (azure-openai, openai). |
| src/Config/HotReloadEventHandler.cs | Registers a new embeddings-related hot-reload event slot. |
| src/Config/HealthCheck/HealthCheckConstants.cs | Adds an “embedding” health-check tag constant. |
| src/Config/DabConfigEvents.cs | Adds an embeddings service config-changed event constant. |
| src/Config/Converters/EmbeddingsOptionsConverterFactory.cs | Custom JSON converter for embeddings config. |
| src/Cli/ConfigGenerator.cs | Adds config generation/update flow for embeddings options. |
| src/Cli/Commands/ConfigureOptions.cs | Adds CLI flags for runtime.embeddings.* configuration. |
| schemas/dab.draft.schema.json | Adds JSON schema definition for runtime.embeddings. |
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
| public class EmbeddingController : ControllerBase | ||
| { | ||
| private readonly IEmbeddingService? _embeddingService; | ||
| private readonly RuntimeConfigProvider _runtimeConfigProvider; | ||
| private readonly ILogger<EmbeddingController> _logger; | ||
|
|
||
| /// <summary> | ||
| /// Constructor. | ||
| /// </summary> | ||
| public EmbeddingController( | ||
| RuntimeConfigProvider runtimeConfigProvider, | ||
| ILogger<EmbeddingController> logger, | ||
| IEmbeddingService? embeddingService = null) | ||
| { | ||
| _runtimeConfigProvider = runtimeConfigProvider; | ||
| _logger = logger; | ||
| _embeddingService = embeddingService; | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// POST endpoint for generating embeddings. | ||
| /// Accepts plain text body and returns embedding vector as comma-separated floats. | ||
| /// </summary> | ||
| /// <param name="route">The route path after the "embed" prefix.</param> | ||
| /// <returns>Plain text embedding vector or error response.</returns> | ||
| [HttpPost] | ||
| [Route("embed/{*route}")] | ||
| [Consumes("text/plain", "application/json")] | ||
| [Produces("text/plain")] | ||
| public async Task<IActionResult> PostAsync(string? route) | ||
| { | ||
| // Get embeddings configuration | ||
| EmbeddingsOptions? embeddingsOptions = _runtimeConfigProvider.GetConfig()?.Runtime?.Embeddings; | ||
| EmbeddingsEndpointOptions? endpointOptions = embeddingsOptions?.Endpoint; | ||
|
|
||
| // Check if embeddings and endpoint are enabled | ||
| if (embeddingsOptions is null || !embeddingsOptions.Enabled) | ||
| { | ||
| return NotFound(); | ||
| } | ||
|
|
||
| if (endpointOptions is null || !endpointOptions.Enabled) | ||
| { | ||
| return NotFound(); | ||
| } | ||
|
|
||
| // Check if the full request path matches the configured endpoint path. | ||
| // Use Request.Path for comparison since the route prefix "embed" is already | ||
| // consumed by the route template and not included in the route parameter. | ||
| string expectedPath = endpointOptions.EffectivePath; | ||
| if (!expectedPath.StartsWith('/')) | ||
| { | ||
| expectedPath = "/" + expectedPath; | ||
| } | ||
|
|
||
| if (!string.Equals(Request.Path.Value, expectedPath, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| return NotFound(); | ||
| } | ||
|
|
||
| // Check if embedding service is available | ||
| if (_embeddingService is null || !_embeddingService.IsEnabled) | ||
| { | ||
| _logger.LogWarning("Embedding endpoint called but embedding service is not available or disabled."); | ||
| return StatusCode((int)HttpStatusCode.ServiceUnavailable, "Embedding service is not available."); | ||
| } | ||
|
|
||
| // Check authorization | ||
| bool isDevelopmentMode = _runtimeConfigProvider.GetConfig()?.Runtime?.Host?.Mode == HostMode.Development; | ||
| string clientRole = GetClientRole(); | ||
|
|
||
| if (!endpointOptions.IsRoleAllowed(clientRole, isDevelopmentMode)) | ||
| { | ||
| _logger.LogWarning("Embedding endpoint access denied for role: {Role}", clientRole); | ||
| return StatusCode((int)HttpStatusCode.Forbidden, "Access denied. Role not authorized."); | ||
| } | ||
|
|
||
| // Read request body as plain text | ||
| string text; | ||
| try | ||
| { | ||
| using StreamReader reader = new(Request.Body); | ||
| text = await reader.ReadToEndAsync(); | ||
|
|
||
| // Handle JSON-wrapped string | ||
| if (Request.ContentType?.Contains("application/json", StringComparison.OrdinalIgnoreCase) == true) | ||
| { | ||
| try | ||
| { | ||
| text = JsonSerializer.Deserialize<string>(text) ?? text; | ||
| } | ||
| catch (JsonException) | ||
| { | ||
| // Not valid JSON string, use as-is | ||
| _logger.LogDebug("Request body is not a valid JSON string, using as plain text."); | ||
| } | ||
| } | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| _logger.LogError(ex, "Failed to read request body for embedding."); | ||
| return BadRequest("Failed to read request body."); | ||
| } | ||
|
|
||
| if (string.IsNullOrWhiteSpace(text)) | ||
| { | ||
| return BadRequest("Request body cannot be empty."); | ||
| } | ||
|
|
||
| // Generate embedding | ||
| EmbeddingResult result = await _embeddingService.TryEmbedAsync(text); | ||
|
|
||
| if (!result.Success) | ||
| { | ||
| _logger.LogError("Embedding request failed: {Error}", result.ErrorMessage); | ||
| return StatusCode((int)HttpStatusCode.InternalServerError, result.ErrorMessage ?? "Failed to generate embedding."); | ||
| } | ||
|
|
||
| if (result.Embedding is null || result.Embedding.Length == 0) | ||
| { | ||
| _logger.LogError("Embedding request returned empty result."); | ||
| return StatusCode((int)HttpStatusCode.InternalServerError, "Failed to generate embedding."); | ||
| } | ||
|
|
||
| // Return embedding as comma-separated float values (plain text) | ||
| string embeddingText = string.Join(",", result.Embedding.Select(f => f.ToString("G", CultureInfo.InvariantCulture))); | ||
| return Content(embeddingText, MediaTypeNames.Text.Plain); | ||
| } | ||
|
|
||
| /// <summary> | ||
| /// Gets the client role from request headers. | ||
| /// </summary> | ||
| private string GetClientRole() | ||
| { | ||
| StringValues roleHeader = Request.Headers[AuthorizationResolver.CLIENT_ROLE_HEADER]; | ||
| string? firstRole = roleHeader.Count == 1 ? roleHeader[0] : null; | ||
|
|
||
| if (!string.IsNullOrEmpty(firstRole)) | ||
| { | ||
| return firstRole.ToLowerInvariant(); | ||
| } | ||
|
|
||
| return EmbeddingsEndpointOptions.ANONYMOUS_ROLE; | ||
| } | ||
| } |
There was a problem hiding this comment.
No tests found for EmbeddingController. The controller includes significant logic that should be tested:
- Route matching and path validation
- Authorization checks (role-based access control)
- Development vs production mode behavior
- Request body parsing (plain text and JSON)
- Empty request body validation
- Service availability checks
- Error response handling
- Integration with IEmbeddingService
Add unit tests or integration tests for the EmbeddingController to ensure the endpoint behaves correctly under various scenarios.
| private string CreateCacheKey(string text) | ||
| { | ||
| // Include provider and model in hash to avoid cross-provider/model collisions | ||
| string keyInput = $"{_options.Provider}:{_options.EffectiveModel}:{text}"; | ||
| byte[] textBytes = Encoding.UTF8.GetBytes(keyInput); | ||
| byte[] hashBytes = SHA256.HashData(textBytes); | ||
| string hashHex = Convert.ToHexString(hashBytes); | ||
|
|
||
| StringBuilder cacheKeyBuilder = new(); | ||
| cacheKeyBuilder.Append(CACHE_KEY_PREFIX); | ||
| cacheKeyBuilder.Append(KEY_DELIMITER); | ||
| cacheKeyBuilder.Append(hashHex); | ||
|
|
||
| return cacheKeyBuilder.ToString(); | ||
| } |
There was a problem hiding this comment.
The cache key generation does not include the dimensions parameter, which can affect the embedding output. If a user changes the dimensions configuration for the same provider/model/text combination, they will get the cached embedding with the old dimensions instead of generating a new one with the updated dimensions.
Consider including the dimensions value (if user-provided) in the cache key to ensure cache keys are unique per configuration: $"{_options.Provider}:{_options.EffectiveModel}:{_options.Dimensions}:{text}"
| [Route("embed/{*route}")] | ||
| [Consumes("text/plain", "application/json")] | ||
| [Produces("text/plain")] | ||
| public async Task<IActionResult> PostAsync(string? route) | ||
| { | ||
| // Get embeddings configuration | ||
| EmbeddingsOptions? embeddingsOptions = _runtimeConfigProvider.GetConfig()?.Runtime?.Embeddings; | ||
| EmbeddingsEndpointOptions? endpointOptions = embeddingsOptions?.Endpoint; | ||
|
|
||
| // Check if embeddings and endpoint are enabled | ||
| if (embeddingsOptions is null || !embeddingsOptions.Enabled) | ||
| { | ||
| return NotFound(); | ||
| } | ||
|
|
||
| if (endpointOptions is null || !endpointOptions.Enabled) | ||
| { | ||
| return NotFound(); | ||
| } | ||
|
|
||
| // Check if the full request path matches the configured endpoint path. | ||
| // Use Request.Path for comparison since the route prefix "embed" is already | ||
| // consumed by the route template and not included in the route parameter. | ||
| string expectedPath = endpointOptions.EffectivePath; | ||
| if (!expectedPath.StartsWith('/')) | ||
| { | ||
| expectedPath = "/" + expectedPath; | ||
| } | ||
|
|
||
| if (!string.Equals(Request.Path.Value, expectedPath, StringComparison.OrdinalIgnoreCase)) | ||
| { | ||
| return NotFound(); | ||
| } |
There was a problem hiding this comment.
The controller uses a catch-all route pattern [Route("embed/{*route}")] but then manually validates the full request path matches the configured endpoint path. This approach has a potential issue:
If the configured path is NOT /embed (e.g., /vectorize), the route won't match at all because the route template is hardcoded to embed/{*route}. The controller will never be invoked for paths like /vectorize.
The comment on line 26 mentions using "embed" prefix to avoid conflicts, but this creates a mismatch between the route template and the configurable endpoint path. Either:
- Make the route template dynamic based on configuration, or
- Document that the endpoint path MUST start with
/embedregardless of configuration, or - Use a different routing approach that can handle arbitrary paths
This is a fundamental design issue that prevents the endpoint.path configuration from working as intended.
| using StreamReader reader = new(Request.Body); | ||
| text = await reader.ReadToEndAsync(); |
There was a problem hiding this comment.
The controller reads the entire request body into memory without any size limit using ReadToEndAsync(). This creates a potential denial-of-service (DoS) vulnerability where an attacker could send extremely large text payloads to consume server memory.
Add a maximum request body size limit check before reading the body. Consider using Request.ContentLength to validate the size, or use a streaming approach with a limited buffer size. A reasonable limit might be a few MB, depending on the expected use case for embedding requests.
| RuntimeCacheOptions? Cache = null, | ||
| PaginationOptions? Pagination = null, | ||
| RuntimeHealthCheckConfig? Health = null, | ||
| EmbeddingsOptions? Embeddings = null) |
There was a problem hiding this comment.
Syntax error: Missing comma at the end of line 35 after the Embeddings parameter. This will cause a compilation error. The Embeddings parameter should end with a comma before the Compression parameter.
| EmbeddingsOptions? Embeddings = null) | |
| EmbeddingsOptions? Embeddings = null, |
JerryNixon
left a comment
JerryNixon
left a comment
There was a problem hiding this comment.
A lot of good work here!
| /// <param name="route">The route path after the "embed" prefix.</param> | ||
| /// <returns>Plain text embedding vector or error response.</returns> | ||
| [HttpPost] | ||
| [Route("embed/{*route}")] |
There was a problem hiding this comment.
The route embed/{*route} tells ASP.NET “send any request that starts with /embed/ here, and capture everything after it,” but the code then does a strict check that Request.Path must exactly equal endpointOptions.EffectivePath, which means most /embed/... requests will be rejected unless they match one exact configured path; this makes the {*route} catch-all effectively useless and makes configuration easy to get wrong because the real routing rules are split between the attribute and a manual runtime check, pick one approach: either make routing match the configured path using endpoint routing (and remove the manual path equality check), or keep a fixed /embed/... route and use the {*route} value consistently instead of comparing full paths.
…dding controller tests, switching the dab schema for the embedding system to default to false
Why make this change?
Internal DAB system for text embedding/vectorization to support future parameter substitution and Redis semantic search features.
What is this change?
Configuration (runtime.embeddings)
enabled: Master toggle (default: true)
provider: azure-openai | openai
base-url, api-key, model: Provider connection (supports @env())
api-version, dimensions, timeout-ms: Optional tuning
endpoint.enabled/path/roles: Optional REST endpoint at configured path (default: /embed)
health.enabled/threshold-ms/test-text/expected-dimensions: Health check config
Core Components
IEmbeddingService with TryEmbedAsync() pattern - returns result objects, no exceptions
EmbeddingService - HTTP client with FusionCache L1 (24h TTL, SHA256 hash keys with provider/model included)
EmbeddingsOptionsConverterFactory - Custom JSON deserializer with env var replacement
EmbeddingTelemetryHelper - OpenTelemetry metrics/spans for latency, cache hits, dimensions
EmbeddingController - REST endpoint for /embed with role-based authorization
Health Check Implementation ✅
HealthCheckHelper.UpdateEmbeddingsHealthCheckResultsAsync() - Executes test embedding with threshold validation
Validates response time against health.threshold-ms
Validates dimensions against health.expected-dimensions if specified
Reports Healthy/Unhealthy status in comprehensive health check report
ConfigurationDetails includes Embeddings and EmbeddingsEndpoint status
REST Endpoint Implementation ✅
EmbeddingController serves POST requests at configured path (default: /embed)
Accepts plain text input and returns comma-separated float values (plain text)
Role-based authorization using X-MS-API-ROLE header
In development mode, defaults to anonymous access
In production mode, requires explicit role configuration
Validation & Safety
Constructor validates Azure OpenAI requires model/deployment name
Constructor validates required fields (BaseUrl, ApiKey)
Cache keys include provider and model to prevent collisions
Validates non-empty embedding arrays from API responses
Telemetry Integration
TryEmbedAsync and TryEmbedBatchAsync instrumented with activity spans and metrics
Cache hit/miss tracking in batch operations
API call duration and error tracking
Integration Points
Health check report includes embeddings status in comprehensive checks
Hot reload via EMBEDDINGS_CONFIG_CHANGED event
Startup logging when embeddings configured
CLI: dab configure --runtime.embeddings.*
Code Organization
Azure.DataApiBuilder.Config.ObjectModel.Embeddings - Config models
Azure.DataApiBuilder.Core.Services.Embeddings - Service, telemetry, interface
Azure.DataApiBuilder.Service.Controllers - EmbeddingController
How was this tested?
Sample Request(s)
{
"runtime": {
"embeddings": {
"enabled": true,
"provider": "azure-openai",
"base-url": "@env('EMBEDDINGS_ENDPOINT')",
"api-key": "@env('EMBEDDINGS_API_KEY')",
"model": "text-embedding-ada-002",
"endpoint": {
"enabled": true,
"path": "/embed",
"roles": ["authenticated"]
},
"health": {
"enabled": true,
"threshold-ms": 5000,
"test-text": "health check"
}
}
}
}
Embed Endpoint:
curl -X POST http://localhost:5000/embed
-H "Content-Type: text/plain"
-H "X-MS-API-ROLE: authenticated"
-d "Hello, world!"
Response:
0.123456,0.234567,0.345678,...