[Examples] ASP.NET Core example improvements #6877
Conversation
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## main #6877 +/- ##
==========================================
+ Coverage 87.21% 87.26% +0.04%
==========================================
Files 263 263
Lines 12385 12385
==========================================
+ Hits 10802 10808 +6
+ Misses 1583 1577 -6
Flags with carried forward coverage won't be shown. Click here to find out more. |
- Use async instead of `.Result`. - Use HttpClientFactory. - Remove separate logging class.
Fix traces not rendering.
Add Loki to display OTLP logs in Grafana.
Without `http://` the scheme was being read as the container hostname.
Fix inaccurate comment.
martincostello
force-pushed
the
aspnet-core-example-improvements
branch
from
2c1500e to
36ab567
Compare
There was a problem hiding this comment.
Pull request overview
This pull request improves the ASP.NET Core example application by modernizing the code patterns, fixing infrastructure configuration issues that prevented traces from rendering, and adding support for viewing OTLP logs through Loki in Grafana.
Changes:
- Modernized C# code to use async/await patterns and HttpClientFactory
- Fixed OpenTelemetry Collector configuration to use HTTP protocol for trace export to Tempo
- Added Loki service for log aggregation with OTLP support and Grafana integration
Reviewed changes
Copilot reviewed 10 out of 11 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| examples/AspNetCore/Program.cs | Added HttpClient registration for dependency injection |
| examples/AspNetCore/Examples.AspNetCore.csproj | Suppressed CA2007 warning (ConfigureAwait) for async code |
| examples/AspNetCore/Controllers/WeatherForecastController.cs | Converted to async/await, injected HttpClient, consolidated logging source generator into nested class |
| examples/AspNetCore/Controllers/WeatherForecastControllerLog.cs | Removed separate logging file (consolidated into controller) |
| examples/AspNetCore/tempo.yaml | Added ingester, memberlist, and metrics_generator configuration for faster trace processing |
| examples/AspNetCore/otel-collector.yaml | Changed trace exporter from GRPC to HTTP protocol, added batch processor and Loki log exporter |
| examples/AspNetCore/loki.yaml | New Loki configuration file for log storage |
| examples/AspNetCore/grafana-datasources.yaml | Added Loki datasource configuration |
| examples/AspNetCore/docker-compose.yaml | Added Loki service container |
| examples/AspNetCore/README.md | Updated documentation to include Loki |
| .gitignore | Added loki-data directory to ignore list |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| from metrics to traces) | ||
| - **Tempo** to store traces // TODO: Add a logging store also. | ||
| - **Tempo** to store traces | ||
| - **Loki** to store logs |
There was a problem hiding this comment.
💯 Thank you!
(There were lot of discussions about what tool are allowed vs not-allowed in the past! I really wanted to have this in)
Hoping to test exemplars with the full setup soon.
4 participants
Changes
While working on #6875 I noticed a few improvements that could be made to the ASP.NET Core example application, as well as some issues with the Docker Compose setup to run the OTel Collector and a TODO for adding something to view OTLP Logs.
Specific changes:
asyncinstead of.Result.Merge requirement checklist
Unit tests added/updatedAppropriateCHANGELOG.mdfiles updated for non-trivial changesChanges in public API reviewed (if applicable)