-
Notifications
You must be signed in to change notification settings - Fork 769
Add import telemetry #13682
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
base: main
Are you sure you want to change the base?
Add import telemetry #13682
Conversation
|
🚀 Dogfood this PR with:
curl -fsSL https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.sh | bash -s -- 13682Or
iex "& { $(irm https://raw.githubusercontent.com/dotnet/aspire/main/eng/scripts/get-aspire-cli-pr.ps1) } 13682" |
|
I think we need to a config to control what features are enabled here for a dashboard. I'm guessing hosted dashboards wouldn't want to allow people to import external 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.
Pull request overview
This PR adds telemetry import functionality to the Aspire Dashboard, allowing users to import telemetry data (logs, traces, and metrics) from JSON and ZIP files through a new import button in the settings dialog.
Key Changes
- Import Service: New
TelemetryImportServicefor handling file imports with support for JSON and ZIP formats - Unified Data Model: Consolidated separate OTLP JSON data types (
OtlpLogsDataJson,OtlpTracesDataJson,OtlpMetricsDataJson) into a singleOtlpTelemetryDataJsonclass - UI Enhancement: Added import button with file upload capability to the Settings dialog
Reviewed changes
Copilot reviewed 25 out of 26 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
src/Aspire.Dashboard/Model/TelemetryImportService.cs |
New service implementing telemetry import from JSON/ZIP files with validation and error handling |
tests/Aspire.Dashboard.Tests/Model/TelemetryImportServiceTests.cs |
Comprehensive test suite covering import scenarios including edge cases and round-trip validation |
src/Aspire.Dashboard/Otlp/Model/Serialization/OtlpCommonJson.cs |
Added unified OtlpTelemetryDataJson class supporting logs, traces, and metrics in a single structure |
src/Aspire.Dashboard/Otlp/Model/Serialization/OtlpLogsJson.cs |
Removed redundant OtlpLogsDataJson class (replaced by unified model) |
src/Aspire.Dashboard/Otlp/Model/Serialization/OtlpTraceJson.cs |
Removed redundant OtlpTracesDataJson class (replaced by unified model) |
src/Aspire.Dashboard/Otlp/Model/Serialization/OtlpMetricsJson.cs |
Removed redundant OtlpMetricsDataJson class (replaced by unified model) |
src/Aspire.Dashboard/Otlp/Model/Serialization/OtlpJsonSerializerContext.cs |
Updated serializer context to reference new unified data type |
src/Aspire.Dashboard/Model/TelemetryExportService.cs |
Updated to return OtlpTelemetryDataJson instead of separate type-specific classes |
src/Aspire.Dashboard/Components/Dialogs/SettingsDialog.razor |
Added FluentInputFile component and import button to UI |
src/Aspire.Dashboard/Components/Dialogs/SettingsDialog.razor.cs |
Implemented file upload handlers with progress and completion callbacks |
src/Aspire.Dashboard/DashboardWebApplication.cs |
Registered TelemetryImportService as a scoped service |
src/Aspire.Dashboard/Resources/Dialogs.resx |
Added "SettingsImportButtonText" localization key |
src/Aspire.Dashboard/Resources/Dialogs.Designer.cs |
Generated designer code for new localization resource |
src/Aspire.Dashboard/Resources/xlf/* |
Added localization entries for import button text across all supported languages |
Files not reviewed (1)
- src/Aspire.Dashboard/Resources/Dialogs.Designer.cs: Language not supported
src/Aspire.Dashboard/Components/Dialogs/SettingsDialog.razor.cs
Outdated
Show resolved
Hide resolved
src/Aspire.Dashboard/Components/Dialogs/SettingsDialog.razor.cs
Outdated
Show resolved
Hide resolved
|
I think we need to start thinking about the dashboard as an API as well. We want to make sure we can collect data on the CI after a run. We need to think about what it means for the dashboard to be disabled in this context (but still available as an API). |
| /// <returns>A task representing the async operation.</returns> | ||
| public async Task ImportAsync(string fileName, Stream stream, CancellationToken cancellationToken) | ||
| { | ||
| await ImportCoreAsync(fileName, stream, allowZipFile: true, cancellationToken).ConfigureAwait(false); |
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.
Is this externally configurable? Seems like you've allow for turning it off but I can't see where you set it other than this hardcoded value.
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.
allowZipFile? It's true here and false in ImportZipAsync. I didn't want to allow recursive zip files.
60599d5 to
a778b76
Compare
|
@eerhardt The AppService integration can automatically add the dashboard. Should imports be disable? It would just be a matter of adding an env var: |
Description
Adds import button:
Checklist
<remarks />and<code />elements on your triple slash comments?doc-ideatemplatebreaking-changetemplatediagnostictemplate