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
embeddings endpoint is now permanently fixed to /embed with no user-configurable path option. This removes unnecessary configuration surface since the feature has not been released yet, eliminating the need for backward compatibility. Changes: - Remove path property from dab.draft.schema.json - Remove Path, UserProvidedPath, and EffectivePath from EmbeddingsEndpointOptions - Remove EffectiveEndpointPath from EmbeddingsOptions - Remove path deserialization from EmbeddingsOptionsConverterFactory - Remove --runtime.embeddings.endpoint.path CLI option - Remove path configuration logic from ConfigGenerator - Remove endpoint path validation from RuntimeConfigValidator - Update Startup.cs logging to use DEFAULT_PATH constant - Update all tests to remove path references
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,...