Add configuring-opentelemetry-dotnet skill (+8.1% eval, near-miss) - #91
Add configuring-opentelemetry-dotnet skill (+8.1% eval, near-miss)#91mrsharm wants to merge 2 commits into
Conversation
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.
There was a problem hiding this comment.
Pull request overview
Adds a new .NET skill aimed at guiding users through configuring OpenTelemetry (tracing, metrics, logging) in ASP.NET Core, along with an evaluation scenario file to validate activation and non-activation behavior.
Changes:
- Introduces
configuring-opentelemetry-dotnetskill documentation and workflow guidance. - Adds eval scenarios to validate correct OpenTelemetry setup guidance and prevent activation on a Serilog-only question.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated 3 comments.
| File | Description |
|---|---|
| src/dotnet/skills/configuring-opentelemetry-dotnet/SKILL.md | New skill content describing package selection, Program.cs configuration, custom spans/metrics, exporters, and propagation. |
| src/dotnet/tests/configuring-opentelemetry-dotnet/eval.yaml | New evaluation scenarios/assertions for the skill’s activation and a negative (non-activation) case. |
Comments suppressed due to low confidence (5)
src/dotnet/skills/configuring-opentelemetry-dotnet/SKILL.md:241
- The context propagation snippet isn’t self-contained: it references
ActivitySourcewithout defining it in scope, and it omits required usings/types (e.g.,OpenTelemetry.Context.PropagationforPropagators/PropagationContext,System.DiagnosticsforActivity/Baggage). Consider making this a complete example (declare the ActivitySource being used and include the needed using directives).
```csharp
// Manual context propagation (e.g., across message queues)
// On the SENDING side:
var propagator = Propagators.DefaultTextMapPropagator;
var context = new PropagationContext(Activity.Current!.Context, Baggage.Current);
var carrier = new Dictionary<string, string>();
propagator.Inject(context, carrier, (dict, key, value) => dict[key] = value);
// Send carrier dictionary as message headers
// On the RECEIVING side:
var parentContext = propagator.Extract(default, carrier,
(dict, key) => dict.TryGetValue(key, out var value) ? new[] { value } : Array.Empty<string>());
Baggage.Current = parentContext.Baggage;
using var activity = ActivitySource.StartActivity("ProcessMessage",
ActivityKind.Consumer,
parentContext.ActivityContext); // Links to parent trace!
src/dotnet/tests/configuring-opentelemetry-dotnet/eval.yaml:36
- The negative scenario forbids the string "OpenTelemetry" entirely, which will fail even if the assistant correctly says “you don’t need OpenTelemetry for this Serilog question.” Consider removing this assertion or narrowing it to OpenTelemetry-specific APIs (e.g., AddOpenTelemetry/OTLP/ActivitySource) so the test focuses on “not suggesting” rather than “not mentioning”.
- type: "output_not_contains"
value: "OpenTelemetry"
- type: "output_not_matches"
pattern: "(ActivitySource|StartActivity|AddOpenTelemetry|OTLP)"
src/dotnet/tests/configuring-opentelemetry-dotnet/eval.yaml:28
expect_tools: ["bash"]makes the scenario fail unless a bash tool call occurs, but this prompt can be answered correctly without running any commands. Unless tool usage is essential to the behavior being tested, dropexpect_toolshere to avoid false failures.
- "Emphasized that ActivitySource/Meter names must match AddSource/AddMeter registrations exactly"
expect_tools: ["bash"]
timeout: 120
src/dotnet/skills/configuring-opentelemetry-dotnet/SKILL.md:49
- Step 2 uses
.AddRuntimeInstrumentation(), but Step 1 doesn’t include the required NuGet package (OpenTelemetry.Instrumentation.Runtime). Add it to the install list or remove the runtime instrumentation call to keep the guidance buildable.
# Optional: additional auto-instrumentation
dotnet add package OpenTelemetry.Instrumentation.SqlClient # SQL Server
dotnet add package OpenTelemetry.Instrumentation.EntityFrameworkCore # EF Core
dotnet add package OpenTelemetry.Instrumentation.GrpcNetClient # gRPC
**src/dotnet/skills/configuring-opentelemetry-dotnet/SKILL.md:105**
* The OTLP exporter endpoint is configured for tracing, but metrics uses `.AddOtlpExporter()` without the same endpoint/protocol settings. That can lead to traces and metrics going to different collectors if the user changes the tracing exporter options. Consider showing a shared OTLP exporter configuration for both tracing and metrics.
.AddOtlpExporter(options =>
{
options.Endpoint = new Uri("http://localhost:4317"); // gRPC endpoint
// For HTTP: options.Protocol = OtlpExportProtocol.HttpProtobuf;
// options.Endpoint = new Uri("http://localhost:4318/v1/traces");
}))
.WithMetrics(metrics => metrics
.AddAspNetCoreInstrumentation()
.AddHttpClientInstrumentation()
.AddRuntimeInstrumentation() // GC, thread pool metrics
// Custom meters
.AddMeter("MyOrderService.Metrics")
.AddOtlpExporter());
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| activity?.SetStatus(ActivityStatusCode.Ok); | ||
|
|
||
| return order; | ||
| } | ||
| catch (Exception ex) | ||
| { | ||
| // Record the exception on the span | ||
| activity?.SetStatus(ActivityStatusCode.Error, ex.Message); | ||
| activity?.RecordException(ex); |
There was a problem hiding this comment.
In the spans example, SetStatus(...) and RecordException(...) are OpenTelemetry extension methods on Activity (namespace OpenTelemetry.Trace). The snippet only includes using System.Diagnostics;, so it won’t compile as-is. Add using OpenTelemetry.Trace; (or qualify the extension methods) in that example.
| activity?.SetStatus(ActivityStatusCode.Ok); | |
| return order; | |
| } | |
| catch (Exception ex) | |
| { | |
| // Record the exception on the span | |
| activity?.SetStatus(ActivityStatusCode.Error, ex.Message); | |
| activity?.RecordException(ex); | |
| if (activity != null) | |
| { | |
| OpenTelemetry.Trace.ActivityExtensions.SetStatus(activity, ActivityStatusCode.Ok); | |
| } | |
| return order; | |
| } | |
| catch (Exception ex) | |
| { | |
| // Record the exception on the span | |
| if (activity != null) | |
| { | |
| OpenTelemetry.Trace.ActivityExtensions.SetStatus(activity, ActivityStatusCode.Error, ex.Message); | |
| OpenTelemetry.Trace.ActivityExtensions.RecordException(activity, ex); | |
| } |
| ```skill | ||
| --- | ||
| name: configuring-opentelemetry-dotnet | ||
| description: Configure OpenTelemetry distributed tracing, metrics, and logging in ASP.NET Core using the .NET OpenTelemetry SDK. Use when adding observability, setting up OTLP exporters, creating custom metrics/spans, or troubleshooting distributed trace correlation. | ||
| --- | ||
|
|
There was a problem hiding this comment.
SKILL.md is wrapped in a fenced ```skill code block, but the skill validator only parses YAML frontmatter when the file starts with ---. As written, metadata (especially `description`) won’t be detected and the whole document will be treated as code fencing. Remove the outer code fence and use normal top-of-file YAML frontmatter like the other skills.
| activity?.SetTag("order.status", "completed"); | ||
| activity?.SetStatus(ActivityStatusCode.Ok); | ||
|
|
||
| return order; | ||
| } |
There was a problem hiding this comment.
In the OrderService example, return order; references an order variable that isn’t defined in the snippet, so the sample won’t compile. Either construct the order object in the example or return an existing variable (and keep the focus on tracing).
| }) | ||
| .AddSqlClientInstrumentation(options => | ||
| { | ||
| options.SetDbStatementForText = true; // Capture SQL text |
There was a problem hiding this comment.
does SetDbStatementForText exist in the latest https://github.com/open-telemetry/opentelemetry-dotnet-contrib/blob/main/src/OpenTelemetry.Instrumentation.SqlClient/SqlClientTraceInstrumentationOptions.cs?
| .WithMetrics(metrics => metrics | ||
| .AddAspNetCoreInstrumentation() | ||
| .AddHttpClientInstrumentation() | ||
| .AddRuntimeInstrumentation() // GC, thread pool metrics |
There was a problem hiding this comment.
does it need to reference OpenTelemetry.Instrumentation.Runtime package?
| |---------|----------| | ||
| | `ActivitySource.StartActivity` returns null | Source name doesn't match any `AddSource()` — names must match exactly | | ||
| | Traces not appearing in exporter | Check OTLP endpoint: gRPC uses port 4317, HTTP uses 4318 | | ||
| | Missing HTTP client spans | `AddHttpClientInstrumentation()` only works with `IHttpClientFactory`-created clients | |
There was a problem hiding this comment.
Is this accurate? wouldn't the instrumentation still work with the spans created from clients not created from IHttpClientFactory?
@rajkumar-rangaraj may help confirming or correcting.
| .AddRuntimeInstrumentation() // GC, thread pool metrics | ||
| // Custom meters | ||
| .AddMeter("MyOrderService.Metrics") | ||
| .AddOtlpExporter()); |
There was a problem hiding this comment.
these doesn't need to configure the endpoint as you did with metrics?
| Trace context propagation is automatic for HTTP calls when using `AddHttpClientInstrumentation()`. For non-HTTP scenarios: | ||
|
|
||
| ```csharp | ||
| // Manual context propagation (e.g., across message queues) |
There was a problem hiding this comment.
do we need using directives here?
| // ObservableGauge — use for point-in-time values (queue depth, etc.) | ||
| // Note: registered once, callback invoked on each collection | ||
| private static readonly ObservableGauge<int> QueueDepth = | ||
| Meter.CreateObservableGauge("orders.queue_depth", () => GetQueueDepth()); |
There was a problem hiding this comment.
I assume GetQueueDepth just demonstrating the idea and doesn't have to exist in the code.
Skill Validation Results — configuring-opentelemetry-dotnet
Overall improvement: +15.3% (3 runs, not statistically significant) Model: claude-opus-4.6 | Judge: claude-opus-4.6 |
…ching rubric emphasis
| | Input | Required | Description | | ||
| |-------|----------|-------------| | ||
| | ASP.NET Core project | Yes | The application to instrument | | ||
| | Observability backend | No | Where to export: Jaeger, Prometheus, OTLP collector, Aspire | |
There was a problem hiding this comment.
| | Observability backend | No | Where to export: Jaeger, Prometheus, OTLP collector, Aspire | | |
| | Observability backend | No | Where to export: Jaeger, Prometheus, OTLP collector, Aspire Dashboard | |
| - Troubleshooting distributed trace context propagation across services | ||
|
|
||
| ## When Not to Use | ||
|
|
There was a problem hiding this comment.
| - The user's application doesn't use ASP.NET |
| options.SetDbStatementForText = true; // Capture SQL text | ||
| options.RecordException = true; | ||
| }) | ||
| // Custom activity sources (for your own spans) |
There was a problem hiding this comment.
Many apps probably have no need to do this. You might want to segregate this into another step that is clearly optional.
|
|
||
| **This correlates logs with traces automatically** — each log entry gets the current TraceId and SpanId. | ||
|
|
||
| ### Step 4: Create custom spans (Activities) for business operations |
There was a problem hiding this comment.
I'd recommend marking this optional. I suspect many customers don't need this.
|
|
||
| **Critical: `ActivitySource` name must match `AddSource("...")` in configuration.** Unmatched sources are silently ignored — this is the #1 debugging issue. | ||
|
|
||
| ### Step 5: Create custom metrics |
There was a problem hiding this comment.
This should also be optional, though probably more apps would benefit from it than customer tracing spans.
| public class OrderMetrics | ||
| { | ||
| // Meter name must match AddMeter("...") in configuration | ||
| private static readonly Meter Meter = new("MyOrderService.Metrics"); |
There was a problem hiding this comment.
The official guidance is for apps using DI to use IMeterFactory rather than creating a static Meter:
https://learn.microsoft.com/en-us/aspnet/core/log-mon/metrics/metrics?view=aspnetcore-10.0#creating-metrics-in-aspnet-core-apps-with-imeterfactory
|
|
||
| ## Validation | ||
|
|
||
| - [ ] Traces appear in the observability backend (Jaeger, Aspire dashboard, etc.) |
| | Missing HTTP client spans | `AddHttpClientInstrumentation()` only works with `IHttpClientFactory`-created clients | | ||
| | High cardinality tags | Don't use user IDs, request IDs, or UUIDs as metric tags — explodes storage | | ||
| | OTLP gRPC vs HTTP mismatch | Default is gRPC (port 4317); if collector only accepts HTTP, set `OtlpExportProtocol.HttpProtobuf` | | ||
| | `Meter` and `ActivitySource` not static | Must be static — creating per-request wastes memory and may lose data | |
| scenarios: | ||
| - name: "Add OpenTelemetry tracing and metrics to an ASP.NET Core API" | ||
| prompt: | | ||
| I need to add distributed tracing and custom metrics to my ASP.NET Core 8 API. We use an OTLP-compatible collector (Jaeger). I want to: |
There was a problem hiding this comment.
I'd recommend either adding more eval prompts, or if we will only have a limited number of prompts focus on prompts that are simpler and more generalized. I'd guess prompts like these are going to be more common:
"Please enable telemetry for my app"
"Help set up OpenTelemetry"
"I want to record some metrics, how do I do that?"
Regardless of whether those imprecise prompts are best-practice, its what I would expect real users will frequently do.
|
Just curious, have you compared any results between writing all this guidance inline vs. having the skill reference pre-existing docs such as: The skill is certainly more compact, but its not clear to me how the tradeoff between compactness vs. depth/breadth affects the results. (I'm also not sure we have enough test cases to draw much conclusion from the automated results alone) |
External docs inform; inline skills steer. Pointing to docs put more faith in the LLM figuring it out. Doing it inline allows you a much more direct way to influence. |
|
I recently updated the OpenTelemetry .NET documentation to improve how OpenTelemetry configuration is explained and structured. As part of that work, we intentionally separated the builder-based configuration docs so they can be consumed not only by application developers, but also by agents and automation workflows that need to reason about setup in a more modular and programmatic way. You can see that structure here: It might be worth borrowing a similar approach for skills documentation by:
Given that skills in this repo are also expected to be consumable by agents as well as developers, explicitly separating configuration intent from implementation wiring could help. For example:
Just sharing this as a pattern that has worked well in a similar context and may help future readers and agents navigate and reuse this content more predictably. |
|
Adding OpenTelemetry .NET maintainers to get their perspective too. @alanwest @Kielek @martincostello |
|
Closing: replaced by new PR from mrsharm/skills with plugins/ directory structure. |
… - Workforce stays on xUnit
Summary
Adds the configuring-opentelemetry-dotnet skill for setting up OpenTelemetry distributed tracing, metrics, and logging in ASP.NET Core.
Eval Results
What the Skill Teaches
Files