-
Notifications
You must be signed in to change notification settings - Fork 4.8k
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
[AzureMonitorLiveMetrics] POC #40001
Conversation
@@ -14,6 +14,7 @@ | |||
<PackageReference Include="Azure.Core" /> | |||
<PackageReference Include="OpenTelemetry" /> | |||
<PackageReference Include="OpenTelemetry.Exporter.Console" VersionOverride="1.6.0" /> | |||
<PackageReference Include="System.Diagnostics.PerformanceCounter" VersionOverride="7.0.0" /> |
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.
This is needed to read Performance Counters (CPU & Memory) to enable POC. Will work on removing this in the future.
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.
We may need to keep this one forever, there is no good way to get this 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.
I'm fine with this dependency, as System.Diagnostic is a core dependency of OTel.
Application Insights is able to read perf counters without an extra dependency. I need more time to investigate how that works. For short term, this is fine. :)
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.
AI SDK uses System.Diagnostics.PerformanceCounter
- https://github.com/microsoft/ApplicationInsights-dotnet/blob/de66d679ff32f5a74553edbf52b10b9dc57ded70/WEB/Src/PerformanceCollector/PerformanceCollector/Perf.csproj#L32
...re.Monitor.OpenTelemetry.LiveMetrics/src/Customizations/QuickPulseSDKClientAPIsRestClient.cs
Show resolved
Hide resolved
sdk/monitor/Azure.Monitor.OpenTelemetry.LiveMetrics/src/LiveMetricsExporterOptions.cs
Show resolved
Hide resolved
sdk/monitor/Azure.Monitor.OpenTelemetry.LiveMetrics/src/LiveMetricsExtensions.cs
Show resolved
Hide resolved
sdk/monitor/Azure.Monitor.OpenTelemetry.LiveMetrics/src/LiveMetricsExtractionProcessor.cs
Show resolved
Hide resolved
API change check APIView has identified API level changes in this PR and created following API reviews. |
...itor.OpenTelemetry.LiveMetrics/api/Azure.Monitor.OpenTelemetry.LiveMetrics.netstandard2.0.cs
Show resolved
Hide resolved
...itor.OpenTelemetry.LiveMetrics/api/Azure.Monitor.OpenTelemetry.LiveMetrics.netstandard2.0.cs
Show resolved
Hide resolved
sdk/monitor/Azure.Monitor.OpenTelemetry.LiveMetrics/src/LiveMetricsTraceExporter.cs
Show resolved
Hide resolved
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.
Approving as POC. This requires a complete clean up later.
This PR commits a working Proof of Concept. This is the first working POC that gets data into the Azure Portal.
THIS IS NOT FINAL. After committing these changes, I will merge my POC with Raj's and work on additional enhancements.
Changes
QuickPulseSDKClientAPIsRestClient.cs
. This is needed because the Generated RestClient has a deserialization bug.ManagerFactory
,Manager
, andManager.Metrics
.This is modeled off of the Transmitter class in our AzureMonitorExporter. The Factory ensures that we only have one instance of the Manager per ConnectionString. The Manager class has the HTTP traffic (Ping & Post). The Manager.Metrics handles our metrics.
LiveMetricsMetricExporter
to translate OTel Metrics to AzureMonitor MetricsLiveMetricsTraceExporter
as the entry point to my POC and is used to initialize the Manager class.Future Work