[Shim] docs: Update breaking changes documentation for 3.x migration#3047
Conversation
There was a problem hiding this comment.
Pull request overview
This PR introduces comprehensive breaking changes documentation for the Application Insights SDK migration from version 2.x to 3.x. The migration represents a fundamental architectural shift from a custom telemetry pipeline to an OpenTelemetry-based implementation with Azure Monitor Exporter.
Key Changes:
- Complete breaking changes documentation covering 5 Application Insights packages
- Detailed API removal and signature change documentation
- Migration guidance for moving from custom telemetry infrastructure to OpenTelemetry patterns
- Configuration changes from InstrumentationKey to ConnectionString
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
|
||
| --- | ||
|
|
||
| # 5. Microsoft.ApplicationInsights.Web (Classic ASP.NET) |
There was a problem hiding this comment.
The heading levels are inconsistent across package sections. Lines 28, 151, 259, 324, and 406 all start major package sections but use different heading levels (H1 for lines 28, 151, 259, 324, 406 vs H2 for General Migration Guidance at line 571). Consider using a consistent heading level (preferably H2) for all major package sections to maintain document hierarchy and improve navigation.
| # 5. Microsoft.ApplicationInsights.Web (Classic ASP.NET) | |
| ## 5. Microsoft.ApplicationInsights.Web (Classic ASP.NET) |
| - **OpenTelemetry Builder extensions** via `TelemetryConfiguration.ConfigureOpenTelemetryBuilder()` | ||
|
|
||
| ### Additional Resources | ||
| - [Shim Breaking Changes Documentation](docs/shim_breaking_changes.md) |
There was a problem hiding this comment.
The link reference to "docs/shim_breaking_changes.md" appears to be circular or incorrect. This document itself appears to be the breaking changes documentation. Consider either removing this self-reference or updating it to point to the correct resource. If this is meant to reference the current document, it should be removed to avoid confusion.
| - [Shim Breaking Changes Documentation](docs/shim_breaking_changes.md) |
| - ❌ **`EnableAzureInstanceMetadataTelemetryModule`** - Azure instance metadata module removed | ||
| - ❌ **`EnableHeartbeat`** - Heartbeat configuration removed | ||
| - ❌ **`EnableDiagnosticsTelemetryModule`** - Diagnostics telemetry module removed | ||
|
|
There was a problem hiding this comment.
Inconsistency in EnableHeartbeat property documentation. In the AspNetCore section (line 222), EnableHeartbeat is listed as "Properties Retained" (✅), but in the WorkerService section (line 278), it's listed as "Properties Removed" (❌). These should be consistent across packages, or the difference should be explicitly explained if the property truly behaves differently in these two packages.
| > **Note:** The `EnableHeartbeat` property is retained in `Microsoft.ApplicationInsights.AspNetCore` but has been removed from `Microsoft.ApplicationInsights.WorkerService` in 3.x. This is due to differences in how heartbeat telemetry is supported between ASP.NET Core and Worker Service environments. |
| - ✅ **`EnableDependencyTrackingTelemetryModule`** - Still configurable | ||
| - ✅ **`EnablePerformanceCounterCollectionModule`** - Still configurable | ||
| - ✅ **`EnableQuickPulseMetricStream`** - Maps to `AzureMonitorExporterOptions.EnableLiveMetrics` | ||
| - ✅ **`EnableDebugLogger`** - Still configurable |
There was a problem hiding this comment.
Inconsistency in EnableDebugLogger property documentation. In the AspNetCore section (line 211), EnableDebugLogger is listed as removed (❌), stating "Debug logging handled by OpenTelemetry". However, in the WorkerService section (line 290), it's listed as "Still configurable" (✅). These should be consistent across packages, or the difference should be explicitly explained if the property truly behaves differently in these two packages.
| - ✅ **`EnableDebugLogger`** - Still configurable | |
| - ❌ **`EnableDebugLogger`** - Debug logging handled by OpenTelemetry |
| ### Properties Retained | ||
| - ✅ **`ConnectionString`** - Primary configuration method (maps to `AzureMonitorExporterOptions.ConnectionString`) | ||
| - ✅ **`ApplicationVersion`** - Still configurable | ||
| - ✅ **`DeveloperMode`** - Still configurable |
There was a problem hiding this comment.
Inconsistency in DeveloperMode property documentation. In the AspNetCore section (line 209), DeveloperMode is listed as removed (❌), stating "No longer configurable". However, in the WorkerService section (line 284), it's listed as "Still configurable" (✅). These should be consistent across packages, or the difference should be explicitly explained if the property truly behaves differently in these two packages.
| - ✅ **`DeveloperMode`** - Still configurable | |
| - ✅ **`DeveloperMode`** - Still configurable | |
| > **Note:** `DeveloperMode` is no longer configurable in the AspNetCore package, but remains available in WorkerService. This difference is due to architectural changes in the AspNetCore integration. |
| - ✅ **`EnableAdaptiveSampling`** - Now controls `SamplingRatio` in Azure Monitor Exporter (1.0F when disabled) | ||
| - ✅ **`EnableDependencyTrackingTelemetryModule`** - Still configurable | ||
| - ✅ **`EnablePerformanceCounterCollectionModule`** - Still configurable | ||
| - ✅ **`EnableQuickPulseMetricStream`** - Maps to `AzureMonitorExporterOptions.EnableLiveMetrics` |
There was a problem hiding this comment.
Inconsistency in EnableQuickPulseMetricStream property documentation. In the AspNetCore section (line 208), it's listed as removed (❌), stating "Replaced with OpenTelemetry live metrics". However, in the WorkerService section (line 289), it's listed as retained (✅), stating it "Maps to AzureMonitorExporterOptions.EnableLiveMetrics". These should be consistent across packages, or the difference should be explicitly explained if the property truly behaves differently in these two packages.
| - ✅ **`EnableQuickPulseMetricStream`** - Maps to `AzureMonitorExporterOptions.EnableLiveMetrics` | |
| - ❌ **`EnableQuickPulseMetricStream`** - Replaced with OpenTelemetry live metrics (`AzureMonitorExporterOptions.EnableLiveMetrics`) |
|
|
||
| --- | ||
|
|
||
| # 1. Microsoft.ApplicationInsights (Core SDK) |
There was a problem hiding this comment.
The heading level jumps from H2 (##) at line 15 to H1 (#) at line 28. This creates an inconsistent document structure. Consider using H2 (##) for "1. Microsoft.ApplicationInsights (Core SDK)" to maintain proper hierarchy, or restructure the document so that all major package sections use a consistent heading level.
| # 1. Microsoft.ApplicationInsights (Core SDK) | |
| ## 1. Microsoft.ApplicationInsights (Core SDK) |
No description provided.