Skip to content
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

[examples] Add manual activities and custom metrics to ASP.NET Core example #4133

Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 13 additions & 2 deletions examples/AspNetCore/Controllers/WeatherForecastController.cs
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

namespace Examples.AspNetCore.Controllers;

using Examples.AspNetCore;
using Microsoft.AspNetCore.Mvc;

[ApiController]
Expand All @@ -24,16 +25,18 @@ public class WeatherForecastController : ControllerBase
{
private static readonly string[] Summaries = new[]
{
"Freezing", "Bracing", "Chilly", "Cool", "Mild", "Warm", "Balmy", "Hot", "Sweltering", "Scorching",
"Freezing", "Bracing", "Chilly", "Cool", "Mild", "Warm", "Balmy", "Hot", "Sweltering", "Scorching",
};

private static readonly HttpClient HttpClient = new();

private readonly ILogger<WeatherForecastController> logger;
private readonly Instrumentation instrumentation;

public WeatherForecastController(ILogger<WeatherForecastController> logger)
public WeatherForecastController(ILogger<WeatherForecastController> logger, Instrumentation instrumentation)
{
this.logger = logger ?? throw new ArgumentNullException(nameof(logger));
this.instrumentation = instrumentation ?? throw new ArgumentNullException(nameof(instrumentation));
}

[HttpGet]
Expand All @@ -45,6 +48,11 @@ public IEnumerable<WeatherForecast> Get()
// how dependency calls will be captured and treated
// automatically as child of incoming request.
var res = HttpClient.GetStringAsync("http://google.com").Result;

// Manually create an activity. This will become a child of
// the incoming request.
using var activity = this.instrumentation.ActivitySource.StartActivity("calculate forecast");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could we store the activitysource, and counter instead? so that this.Activitysource.StartActivity, this.FreezingDaysCounter style code can be used.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a preference on the null checks in the constructor?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am just checking for Instrumentation

Copy link

@noahfalk noahfalk Feb 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oh I see I just made the opposite advice ;p @cijothomas what do you like about the multiple fields?


var rng = new Random();
var forecast = Enumerable.Range(1, 5).Select(index => new WeatherForecast
{
Expand All @@ -54,6 +62,9 @@ public IEnumerable<WeatherForecast> Get()
})
.ToArray();

// Count the freezing days
this.instrumentation.FreezingDaysCounter.Add(forecast.Count(f => f.TemperatureC < 0));

this.logger.LogInformation(
"WeatherForecasts generated {count}: {forecasts}",
forecast.Length,
Expand Down
44 changes: 44 additions & 0 deletions examples/AspNetCore/Instrumentation.cs
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
// <copyright file="Instrumentation.cs" company="OpenTelemetry Authors">
// Copyright The OpenTelemetry Authors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.
// </copyright>

namespace Examples.AspNetCore;

using System.Diagnostics;
using System.Diagnostics.Metrics;

public class Instrumentation : IDisposable
{
internal const string ScopeName = "Examples.AspNetCore";
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Exposing this since the ActivitySource and Meter registration is coupled to their names.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'd suggest to rename "ScopeName" to something else.
Scope is used in many context, so it could confuse new users.
https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/api.md#get-a-tracer

Copy link
Contributor Author

@danelson danelson Feb 7, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you have a suggestion? In my opinion the instrumentation scope is the entity to which this data is associated. It also matches the underlying proto (granted most users probably won't know that)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

okay to use ActivitySourceName, MeterName.
AddSource, AddMeter usage is shown later, which looks easier to follow with AddMeter(instrumentaion.MeterNamer), AddActivitySource(instrument.ActivitySourceName)

private readonly Meter meter;

public Instrumentation()
{
string? version = typeof(Instrumentation).Assembly.GetName().Version?.ToString();
this.ActivitySource = new ActivitySource(ScopeName, version);
this.meter = new Meter(ScopeName, version);
this.FreezingDaysCounter = this.meter.CreateCounter<int>("weather.days.freezing", "The number of days where the temperature is below freezing");
}

public ActivitySource ActivitySource { get; }

public Counter<int> FreezingDaysCounter { get; }

public void Dispose()
{
this.ActivitySource.Dispose();
this.meter.Dispose();
}
}
12 changes: 10 additions & 2 deletions examples/AspNetCore/Program.cs
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
// limitations under the License.
// </copyright>

using System.Reflection;
using Examples.AspNetCore;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"using" or "namespace"?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think using is correct. This allows me to reference Instrumentation here. As far as I know you cannot define a namespace with these top level statements

using OpenTelemetry;
using OpenTelemetry.Exporter;
using OpenTelemetry.Instrumentation.AspNetCore;
Expand All @@ -37,9 +37,13 @@
// Build a resource configuration action to set service information.
Action<ResourceBuilder> configureResource = r => r.AddService(
serviceName: appBuilder.Configuration.GetValue<string>("ServiceName"),
serviceVersion: Assembly.GetExecutingAssembly().GetName().Version?.ToString() ?? "unknown",
serviceVersion: typeof(Program).Assembly.GetName().Version?.ToString() ?? "unknown",
serviceInstanceId: Environment.MachineName);

// Create a service to expose ActivitySource, Meter, and Instruments
// for manual instrumentation
appBuilder.Services.AddSingleton<Instrumentation>();

// Configure OpenTelemetry tracing & metrics with auto-start using the
// StartWithHost extension from OpenTelemetry.Extensions.Hosting.
appBuilder.Services.AddOpenTelemetry()
Expand All @@ -48,7 +52,9 @@
{
// Tracing

// Ensure the TracerProvider subscribes to any custom ActivitySources.
builder
.AddSource(Instrumentation.ScopeName)
.SetSampler(new AlwaysOnSampler())
.AddHttpClientInstrumentation()
.AddAspNetCoreInstrumentation();
Expand Down Expand Up @@ -98,7 +104,9 @@
{
// Metrics

// Ensure the MeterProvider subscribes to any custom Meters.
builder
.AddMeter(Instrumentation.ScopeName)
.AddRuntimeInstrumentation()
.AddHttpClientInstrumentation()
.AddAspNetCoreInstrumentation();
Expand Down