-
Notifications
You must be signed in to change notification settings - Fork 142
Add OpenTelemetry instrumentation documentation for Data API Builder integration #965
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
- Updated dab-config.json example to include OTEL configuration - Added comprehensive OTEL documentation to README - Explained how OTEL works with the Aspire integration - Documented advanced OTEL configuration options Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com>
|
@copilot the instrumentation for otel needs to include all the fields: "telemetry": {
"open-telemetry": {
"enabled": true,
"service-name": "@env('OTEL_SERVICE_NAME')",
"endpoint": "@env('OTEL_EXPORTER_OTLP_ENDPOINT')",
"exporter-protocol": "grpc",
"headers": "@env('OTEL_EXPORTER_OTLP_HEADERS')"
}
} |
- Added service-name, endpoint, exporter-protocol, and headers fields - All fields use @env() syntax to reference OTEL environment variables - Updated both example dab-config.json and README documentation Co-authored-by: tommasodotNET <12819039+tommasodotNET@users.noreply.github.com>
... Updated the OTEL configuration to include all fields with environment variable references as requested. The configuration now includes:
Changes made in commit 87fda82. |
| #### Advanced OTEL Configuration | ||
|
|
||
| Data API builder supports additional OTEL configuration options in the `dab-config.json` file: | ||
|
|
||
| ```json | ||
| { | ||
| "runtime": { | ||
| "telemetry": { | ||
| "open-telemetry": { | ||
| "enabled": true, | ||
| "service-name": "my-dab-service", | ||
| "exporter-protocol": "grpc" | ||
| } | ||
| } | ||
| } | ||
| } | ||
| ``` | ||
|
|
||
| Available settings: | ||
| - `enabled`: Enables/disables OTEL telemetry (default: `false`) | ||
| - `service-name`: Logical name for the service in traces (optional, defaults to the resource name) | ||
| - `endpoint`: OTEL collector endpoint URL (optional, automatically set by Aspire) | ||
| - `exporter-protocol`: Choose between `grpc` or `httpprotobuf` (optional, defaults to `grpc`) | ||
| - `headers`: Custom headers for OTEL export (optional) | ||
|
|
||
| > **Note**: The `endpoint` setting is automatically configured by the Aspire integration and typically doesn't need to be set manually. | ||
| For more information about Data API builder telemetry, see the [official documentation](https://learn.microsoft.com/azure/data-api-builder/concept/monitor/open-telemetry). | ||
|
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@copilot this is now redundant. remove this paragraph and add the description of the settings in the above paragraph
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Removed the redundant "Advanced OTEL Configuration" section and moved the setting descriptions into the paragraph following the main configuration example. Each setting now includes an explanation of how it uses the @env() syntax to reference Aspire-provided environment variables.
Changes made in commit 1906f4b.
- Removed duplicate Advanced OTEL Configuration section - Moved setting descriptions to main paragraph after the config example - Improved clarity by explaining the @env() syntax usage - All settings now documented in one place Co-authored-by: tommasodotNET <12819039+tommasodotNET@users.noreply.github.com>
tommasodotNET
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
lgtm
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds comprehensive documentation for OpenTelemetry (OTEL) instrumentation support in the Data API Builder integration for Aspire. Since Data API Builder added native OTEL support, this documentation explains how to configure telemetry to flow into the Aspire dashboard.
Key changes:
- Added detailed OTEL configuration section to the README with examples and field descriptions
- Updated the example
dab-config.jsonwith complete OTEL configuration using@env()syntax
Reviewed Changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
src/CommunityToolkit.Aspire.Hosting.Azure.DataApiBuilder/README.md |
Added new "OpenTelemetry Instrumentation" section documenting how to configure OTEL in dab-config.json and explaining how the integration's .WithOtlpExporter() sets required environment variables |
examples/data-api-builder/CommunityToolkit.Aspire.Hosting.Azure.DataApiBuilder.AppHost/dab-config.json |
Added complete telemetry configuration section with all required OTEL fields using @env() syntax to reference Aspire-provided environment variables |
fboucher
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It looks good. There that failing test but maybe it expected at this time?
|
I don't get why the tests fail - they work on my machine 😭 |
|
@copilot the CI is failing because we don't have the OTEL export headers set, which the config file expects. https://github.com/dotnet/aspire/blob/91481d9b5a5602d3d641fa8ade554c95bcac22b5/src/Aspire.Hosting/OtlpConfigurationExtensions.cs#L81-L84 seems to suggest that Aspire expects an |
- Added static constructor to AppHostTests to set AppHost__OtlpApiKey - This ensures Aspire configures OTEL headers that dab-config.json expects - Fixes CI test failures where OTEL_EXPORTER_OTLP_HEADERS was not set Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com>
Added a static constructor to AppHostTests that sets Changes made in commit afbade2. |
|
@copilot since it looks like there's a problem in how the tests bootstrap the app host, let's remove all changes except the README.md changes. |
- Removed OTEL telemetry configuration from dab-config.json - Removed static constructor from AppHostTests.cs - Keeping only README.md documentation changes - Addresses test bootstrap issues with OTEL configuration Co-authored-by: aaronpowell <434140+aaronpowell@users.noreply.github.com>
Reverted all changes except README.md. Removed the OTEL telemetry configuration from dab-config.json and the static constructor from AppHostTests.cs. The PR now contains only the documentation changes in README.md. Changes made in commit 8122979. |
Minimum allowed line rate is |
Plan: Add OTEL Documentation and Configuration to Data API Builder Integration
Based on the issue and comments, I need to:
dab-config.jsonto include OTEL configuration in theruntime.telemetry.open-telemetrysection.WithOtlpExporter()call in the hosting extensionFix CI test failures by setting AppHost__OtlpApiKey environment variable(reverted due to test bootstrap issues)Summary
Successfully added OpenTelemetry documentation to the Data API Builder integration. The changes are minimal and focused on documentation only:
Files Changed
src/CommunityToolkit.Aspire.Hosting.Azure.DataApiBuilder/README.md- Added comprehensive OTEL documentation with setting descriptionsKey Points
.WithOtlpExporter()which automatically configures OTEL environment variablesTesting & Validation
Security Summary
No security issues detected. Changes are documentation-only.
Original prompt
✨ Let Copilot coding agent set things up for you — coding agent works faster and does higher quality work when set up for your repo.