Add General properties to Application Name Telemetry - #3826
Open
aaronburtle wants to merge 3 commits into
Open
aaronburtle wants to merge 3 commits into
aaronburtle wants to merge 3 commits into
Conversation
Populate the reserved general section with OS, container, hosting, data-source count, and per-source managed identity flags. Preserve positional compatibility and conservative detection with explicit hosting overrides. Bound SQL application names, respect provider credential parsing and OBO ambiguity, and keep recursive CLI inspection offline. Add regression coverage and update the existing telemetry design. Validated full Debug solution build, 3453 non-database service tests, 348 focused telemetry/OBO tests, 12 CLI appname tests, and changed-file formatting.
Model PostgreSQL UTF-8 clipping, ASCII byte escaping, and final statistics clipping. Assert exact surviving telemetry fields with a fixed version and cover Unicode, control bytes, and multibyte boundaries. Clarify the existing documentation without changing production behavior. Validated 18 corrected unit cases, 18 matching live PostgreSQL 16.15 fixtures, 3468 non-database service tests, and changed-file formatting.
Contributor
There was a problem hiding this comment.
Copilot review overview
🔵 Needs a closer look
The broad telemetry, configuration, authentication, and CLI changes warrant final human review.
Review effort: Lite
Findings: None
What changed in this PR
Adds six host and data-source properties to application-name telemetry while preserving token compatibility and enabling offline CLI inspection.
Changes:
- Adds environment, hosting, source-count, and managed-identity detection.
- Preserves SQL application-name limits and connection overrides.
- Adds recursive offline config loading, regression tests, and documentation.
| File | Description |
|---|---|
src/Service.Tests/UnitTests/ApplicationNameTelemetryTests.cs |
Updates telemetry compatibility tests. |
src/Service.Tests/UnitTests/ApplicationNameGeneralTelemetryTests.cs |
Adds General telemetry coverage. |
src/Service.Tests/Configuration/RuntimeConfigLoaderTests.cs |
Verifies multi-source telemetry. |
src/Config/Telemetry/ApplicationNameTelemetryEnvironment.cs |
Detects host environment signals. |
src/Config/Telemetry/ApplicationNameTelemetry.cs |
Encodes and decodes General fields. |
src/Config/RuntimeConfigLoader.cs |
Handles effective connections and SQL limits. |
src/Config/Azure.DataApiBuilder.Config.csproj |
Adds MySQL parsing support. |
src/Cli/Commands/AppNameOptions.cs |
Uses offline configuration loading. |
src/Cli/AppNameConfigLoader.cs |
Loads configurations recursively offline. |
src/Cli.Tests/EndToEndTests.cs |
Tests offline CLI behavior. |
docs/design/application-name-telemetry.md |
Documents the expanded telemetry format. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Suggested title: Add general settings to application-name telemetry
Why make this change?
Closes #3792.
The application-name telemetry format reserved a General section but was initially empty. We populate it with the agreed host and data-source characteristics to improve aggregate usage visibility without collecting identifiers or secrets.
What is this change?
Adds six positional fields using the existing shared encoder/decoder design:
The format remains:
<marker><version>+<context>|<general>|<runtime>|<entity>+DAB_HOSTING_ENVIRONMENTandDAB_AZURE_HOSTING_SERVICEprovide explicit overrides. Detection performs no metadata requests or credential acquisition.M;DefaultAzureCredentialavailability is not treated as proof of MI use.How was this tested?
Sample Request(s)
Inspect the telemetry for a configuration:
dab appname --config dab-config.jsonDecode an illustrative token:
dab appname --decode "dab_oss_1.2.3+XXSX|L1AC01|MMMM00MMM00MMMMMMMMM|MMM?MMMMMMMMM?+"The General section,
L1AC01, represents Linux, running in a container, Azure Container Apps, one data source, and explicitly configured managed identity.