Add configuring-opentelemetry-dotnet skill - #268
Conversation
Migration NoteThis PR replaces #91 which was opened from
All prior review feedback from #91 still applies — please see that PR for the full discussion history. |
There was a problem hiding this comment.
Pull request overview
Adds a new .NET skill, configuring-opentelemetry-dotnet, intended to guide agents through setting up OpenTelemetry (tracing/metrics/logging) in ASP.NET Core, with accompanying evaluation scenarios and CODEOWNERS entries.
Changes:
- Added
configuring-opentelemetry-dotnetskill documentation underplugins/dotnet/skills/. - Added evaluation scenarios under
tests/dotnet/configuring-opentelemetry-dotnet/. - Registered code ownership for the new skill and test directory.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| plugins/dotnet/skills/configuring-opentelemetry-dotnet/SKILL.md | Introduces the new OpenTelemetry setup skill content and code snippets. |
| tests/dotnet/configuring-opentelemetry-dotnet/eval.yaml | Adds eval scenarios to validate skill activation and avoid false positives. |
| .github/CODEOWNERS | Assigns ownership for the new skill and its evals. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
Feedback carried over from #91PR #91 — Add configuring-opentelemetry-dotnet skill (open)18 review threads — Reviewers: copilot-pull-request-reviewer, tarekgh, noahfalk
|
d2d71e4 to
1edd695
Compare
|
Now we have new plugin factoring, moved the skill from |
|
@BrennanConroy @adityamandaleeka - please review |
|
@mrsharm this seems ASP.NET specific? If so should probably go in the dotnet-aspnet plugin not dotnet-diag ? |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 3 out of 3 changed files in this pull request and generated 4 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
/evaluate |
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 4 out of 4 changed files in this pull request and generated 6 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Skill Validation Results
[1] (Plugin) Quality unchanged but weighted score is -1.3% due to: time (10.0s → 14.1s)
Model: claude-opus-4.6 | Judge: claude-opus-4.6
🔍 Full results — includes quality and agent details
|
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 2 comments.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
There was a problem hiding this comment.
Pull request overview
Copilot reviewed 6 out of 6 changed files in this pull request and generated 1 comment.
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
/evaluate |
Skill Validation Results
[1] Model: claude-opus-4.6 | Judge: claude-opus-4.6 🔍 Full Results - additional metrics and failure investigation steps |
tarekgh
left a comment
There was a problem hiding this comment.
Added some questions/suggestions. Also, CC'ed some more people who will be good if can take a look too. LGTM otherwise!
|
/evaluate |
Skill Validation Results
[1] Model: claude-opus-4.6 | Judge: claude-opus-4.6 🔍 Full Results - additional metrics and failure investigation steps ▶ Sessions Visualisation -- interactive replay of all evaluation sessions |
|
/evaluate |
|
@ViktorHofer - if things good look to you, should be good to merge -- approved by the area expert. |
Skill Validation Results
[1] Model: claude-opus-4.6 | Judge: claude-opus-4.6 🔍 Full Results - additional metrics and failure investigation steps ▶ Sessions Visualisation -- interactive replay of all evaluation sessions |
* Add configuring-opentelemetry-dotnet skill (+8.1% eval, near-miss) Teaches OpenTelemetry SDK configuration in ASP.NET Core: package selection, AddOpenTelemetry with tracing/metrics/logging, custom ActivitySource and Meter creation, OTLP exporter setup, and context propagation. Eval results: +8.1% improvement (threshold: 10%, near-miss) Includes eval.yaml with OTel setup scenario + negative test. * Sharpen eval.yaml: add custom-spans-invisible pain-point and name-matching rubric emphasis * Migrate configuring-opentelemetry-dotnet to plugins/ directory structure * Move OpenTelemetry skill to new dotnet-aspnet plugin, address review feedback - Move skill from plugins/dotnet-diag to new plugins/dotnet-aspnet plugin - Create dotnet-aspnet plugin.json - Fix SKILL.md frontmatter: remove code fence wrapper so YAML metadata parses - Fix undefined 'order' variable in ProcessOrderAsync example - Move SqlClient/Runtime instrumentation to clearly marked optional sections - Add missing using directives (System.Diagnostics, OpenTelemetry.Context.Propagation) - Handle null Activity.Current in context propagation snippet - Use IMeterFactory via DI instead of static Meter per official guidance - Add OpenTelemetry.Instrumentation.Http to eval rubric package list - Simplify eval prompt to be more natural/generalized - Update CODEOWNERS: add dotnet-aspnet plugin entries, remove old dotnet-diag entries Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Configure metrics OTLP exporter endpoint explicitly to match tracing config Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Address round 2 review: logging endpoint, marketplace, RecordException using, ActivitySource in propagation, HttpClient pitfall - Configure logging OTLP exporter endpoint explicitly for consistency - Add dotnet-aspnet to both marketplace.json files - Add 'using OpenTelemetry.Trace' for RecordException extension method - Declare ActivitySource in context propagation snippet - Fix HttpClient instrumentation pitfall (works with new HttpClient() too) Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Clarify logging package source and add ActivitySource registration reminder Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add @dotnet/aspnet team to skill CODEOWNERS entries Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add dotnet-aspnet to README table, make propagation ActivitySource static Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add expect_activation:false, IMeterFactory using, clarify OTLP exporter scope Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Add OtlpExporter using directive, clarify OTLP package covers logging Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Remove Prometheus from OTLP claim, add OTLP exporter assertion to eval Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Clarify Jaeger is traces-only, document logging package provenance Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Fix eval: increase timeout, reject tools, rewrite prompt as explanation-only - Scenario 1: rewrite prompt to ask for code explanation (not file modification), add reject_tools to prevent bash/edit overhead, increase timeout 120s->360s, update rubric wording - Scenario 2: add reject_tools to eliminate tool-overhead noise Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> * Improve OTel skill evals and address review feedback - Remove redundant per-skill CODEOWNERS lines (ViktorHofer feedback) - Add SetResourceBuilder + ParseStateValues to SKILL.md Step 3 (logging provider gap) - Rewrite eval scenarios: 2 targeted positive scenarios testing real skill gaps - Remove noisy negative activation scenario (high variance, -21.5% drag) - Eval results: +14.2% improvement (threshold: 10%), overfitting 0.10 (low) Scenario 1: 3.0/5 -> 5.0/5, Scenario 2: 3.8/5 -> 5.0/5 * Add propagate-trace-context scenario to OTel evals (+25.6%) Third scenario tests trace context propagation across message queues, where the base model scores 3.2/5 and skill-enhanced scores 5.0/5. Removed metrics and debug scenarios that baseline already aced (5.0/5) which only added token cost without measurable improvement. Final: 3 scenarios, +25.6% improvement, overfitting 0.09 (low) * Remove duplicate dotnet-aspnet CODEOWNERS entries (L69-70 shadowed by L86-87) * Use @dotnet/aspnet team alias in CODEOWNERS for wider reach * Addressed feedback --------- Co-authored-by: Dan Moseley <danmose@microsoft.com> Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com> Co-authored-by: Viktor Hofer <7412651+ViktorHofer@users.noreply.github.com>
Adds the configuring-opentelemetry-dotnet skill for setting up OpenTelemetry in ASP.NET Core.
What the Skill Teaches
The base model gets several OpenTelemetry patterns wrong or incomplete. This skill addresses five key gaps:
OpenTelemetry.Instrumentation.Httpor installs the bareOpenTelemetrypackage instead ofOpenTelemetry.Extensions.Hosting. The skill lists the exact packages needed for each signal.ActivitySourceandMeternames must matchAddSource()/AddMeter()exactly or telemetry is silently dropped. The base model rarely warns about this, which is the Initial documentation and validation workflow #1 debugging issue.new Meter(). The skill teaches the modernIMeterFactorypattern via dependency injection for proper lifetime management.ConfigureResource()on the tracing/metrics pipeline does NOT propagate to the logging provider. The skill teaches settingSetResourceBuilderexplicitly on the logging provider so logs carry the same service name.TextMapPropagator.Inject/Extract.Eval Results
Overall: +25.6% | Model: claude-opus-4.6 | Judge: claude-opus-4.6