[Shim] Cleanup unused usings, address logging todo's#3034
Conversation
There was a problem hiding this comment.
Pull request overview
This PR performs cleanup tasks across the Application Insights SDK codebase, focusing on removing unused using statements and replacing TODO logging comments with proper EventSource implementations. However, the PR contains a critical breaking change that will prevent compilation: the deletion of the IAsyncFlushable interface while ServerTelemetryChannel still implements it.
Key Changes
- Removed unused using statements from example projects and core SDK files
- Replaced logging TODO comments with proper EventSource method calls
- Added new EventSource methods (
ConnectionStringLoadedFromConfig,NoConnectionStringFoundInConfig,MetricDimensionMismatch,TrackMetricTelemetryIsNull) - Changed
SetAzureTokenCredentialvisibility from public to internal - Deleted
IAsyncFlushableinterface (BREAKING - see below) - Removed obsolete commented-out package references
Reviewed changes
Copilot reviewed 38 out of 38 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| examples/WorkerService/Worker.cs | Removed unused Application Insights imports |
| examples/BasicConsoleApp/MetricsExamples.cs | Removed unused DataContracts import |
| examples/AspNetCoreWebApp/Startup.cs | Removed multiple unused System and Application Insights imports |
| examples/AspNetCoreWebApp/Program.cs | Removed unused System and Extensions imports |
| examples/AspNetCoreWebApp/Models/ErrorViewModel.cs | Removed unused System import |
| examples/AspNetCoreWebApp/Controllers/HomeController.cs | Removed unused System.Collections.Generic, System.Linq, System.Threading.Tasks imports |
| WEB/Src/Web/Web/Web.csproj | Removed commented-out TelemetryCorrelation package reference |
| WEB/Src/Web/Web/Properties/AssemblyInfo.cs | Removed unused System.Reflection import |
| WEB/Src/Web/Web/Implementation/WebEventSource.cs | Removed unused System import; added new logging methods |
| WEB/Src/Web/Web/Implementation/HttpRequestExtensions.cs | Removed unused ApplicationInsights.Common import |
| WEB/Src/Web/Web/Implementation/AppMapCorrelationEventSource.cs | Removed unused System and Tracing imports |
| WEB/Src/Web/Web/Extensions/ApplicationInsightsExtensions.cs | Removed unused Azure.Monitor.OpenTelemetry.Exporter import |
| WEB/Src/Web/Web/ApplicationInsightsHttpModule.cs | Replaced TODO comments with actual EventSource logging calls |
| WEB/Src/Common/AppMapCorrelationEventSource.cs | Removed unused System and Tracing imports |
| NETCORE/src/Shared/Extensions/ApplicationInsightsExtensions.cs | Removed multiple unused imports |
| NETCORE/src/Microsoft.ApplicationInsights.WorkerService/Implementation/Tracing/WorkerServiceEventSource.cs | Removed unused System import |
| NETCORE/src/Microsoft.ApplicationInsights.AspNetCore/Microsoft.ApplicationInsights.AspNetCore.csproj | Removed commented-out package reference |
| NETCORE/src/Microsoft.ApplicationInsights.AspNetCore/Extensions/RequestCollectionOptions.cs | Removed unused System import |
| NETCORE/src/Microsoft.ApplicationInsights.AspNetCore/Extensions/ApplicationInsightsExtensions.cs | Removed unused Http, DependencyInjection imports |
| NETCORE/src/Microsoft.ApplicationInsights.AspNetCore/Extensibility/Implementation/Tracing/AspNetCoreEventSource.cs | Removed unused System import |
| BASE/src/Microsoft.ApplicationInsights/TelemetryClient.cs | Removed pragma warnings; replaced TODO with EventSource logging |
| BASE/src/Microsoft.ApplicationInsights/Metric.cs | Replaced System.Diagnostics.Metrics with Tracing import; replaced multiple TODO comments with EventSource logging calls |
| BASE/src/Microsoft.ApplicationInsights/Extensibility/TelemetryConfiguration.cs | Removed unused Implementation import; changed SetAzureTokenCredential to internal |
| BASE/src/Microsoft.ApplicationInsights/Extensibility/Implementation/Utils.cs | Removed unused System, Globalization, Linq imports |
| BASE/src/Microsoft.ApplicationInsights/Extensibility/Implementation/UserContext.cs | Removed unused Collections.Generic import |
| BASE/src/Microsoft.ApplicationInsights/Extensibility/Implementation/Tracing/CoreEventSource.cs | Removed unused System import; added new logging methods |
| BASE/src/Microsoft.ApplicationInsights/Extensibility/Implementation/SessionContext.cs | Removed unused Collections.Generic import |
| BASE/src/Microsoft.ApplicationInsights/Extensibility/Implementation/OperationContext.cs | Removed unused Collections.Generic import |
| BASE/src/Microsoft.ApplicationInsights/Extensibility/Implementation/LocationContext.cs | Removed unused Collections.Generic import |
| BASE/src/Microsoft.ApplicationInsights/Extensibility/Implementation/ComponentContext.cs | Removed unused Collections.Generic import |
| BASE/src/Microsoft.ApplicationInsights/DataContracts/TelemetryContext.cs | Removed unused System, Channel, Extensibility imports |
| BASE/src/Microsoft.ApplicationInsights/DataContracts/RequestTelemetry.cs | Removed unused Collections.Concurrent, Threading imports |
| BASE/src/Microsoft.ApplicationInsights/DataContracts/MetricTelemetry.cs | Removed unused ComponentModel, Globalization, Extensibility imports |
| BASE/src/Microsoft.ApplicationInsights/DataContracts/ExceptionInfo.cs | Removed unused Linq, Implementation imports |
| BASE/src/Microsoft.ApplicationInsights/DataContracts/DependencyTelemetry.cs | Removed unused Collections.Concurrent, ComponentModel, Threading imports |
| BASE/src/Microsoft.ApplicationInsights/Channel/IAsyncFlushable.cs | DELETED - BREAKS BUILD |
| BASE/src/Microsoft.ApplicationInsights/ActivityExtensions.cs | Removed multiple unused System imports |
| .publicApi/Microsoft.ApplicationInsights.dll/Stable/PublicAPI.Shipped.txt | Removed IAsyncFlushable and SetAzureTokenCredential from public API |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
harsimar
left a comment
There was a problem hiding this comment.
Generally looks good, seems like there are a lot of warnings regarding the System.Exception being too broad - if this is intentional maybe we could look into silencing these kinds of warnings.
That message keeps showing up in PRs. We should consider adding pragma disable to remove it. It's not related to this PR, but we can address it separately. |
No description provided.