Skip to content

Commit

Permalink
Merge branch 'main' into api-log-diagnostics
Browse files Browse the repository at this point in the history
  • Loading branch information
CodeBlanch authored Dec 8, 2023
2 parents 54b5bdc + 93fd7d3 commit 749ee6e
Show file tree
Hide file tree
Showing 10 changed files with 121 additions and 91 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/stale.yml
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ jobs:
stale:
runs-on: ubuntu-latest
steps:
- uses: actions/stale@v8
- uses: actions/stale@v9
with:
stale-pr-message: 'This PR was marked stale due to lack of activity and will be closed in 7 days. Commenting or Pushing will instruct the bot to automatically remove the label. This bot runs once per day.'
close-pr-message: 'Closed as inactive. Feel free to reopen if this PR is still being worked on.'
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ internal interface IMeterProviderBuilder : IDeferredMeterProviderBuilder

/// <summary>
/// Register a callback action to configure the <see
/// cref="IServiceCollection"/> where metric services are configured.
/// cref="IServiceCollection"/> where metrics services are configured.
/// </summary>
/// <remarks>
/// Note: Metric services are only available during the application
/// Note: Metrics services are only available during the application
/// configuration phase. This method should throw a <see
/// cref="NotSupportedException"/> if services are configured after the
/// application <see cref="IServiceProvider"/> has been created.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -112,10 +112,10 @@ public static MeterProviderBuilder AddInstrumentation<T>(

/// <summary>
/// Register a callback action to configure the <see
/// cref="IServiceCollection"/> where tracing services are configured.
/// cref="IServiceCollection"/> where metrics services are configured.
/// </summary>
/// <remarks>
/// Note: Tracing services are only available during the application
/// Note: Metrics services are only available during the application
/// configuration phase.
/// </remarks>
/// <param name="meterProviderBuilder"><see cref="MeterProviderBuilder"/>.</param>
Expand Down
10 changes: 8 additions & 2 deletions src/OpenTelemetry.Api/Trace/ActivityExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -57,20 +57,26 @@ public static Status GetStatus(this Activity activity)
}

/// <summary>
/// Adds an activity event containing information from the specified exception.
/// Adds an <see cref="ActivityEvent"/> containing information from the specified exception.
/// </summary>
/// <param name="activity">Activity instance.</param>
/// <param name="ex">Exception to be recorded.</param>
/// <remarks> The exception is recorded as per <a href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/exceptions.md">specification</a>.
/// "exception.stacktrace" is represented using the value of <a href="https://learn.microsoft.com/dotnet/api/system.exception.tostring">Exception.ToString</a>.
/// </remarks>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static void RecordException(this Activity activity, Exception? ex)
=> RecordException(activity, ex, default);

/// <summary>
/// Adds an activity event containing information from the specified exception and additional tags.
/// Adds an <see cref="ActivityEvent"/> containing information from the specified exception and additional tags.
/// </summary>
/// <param name="activity">Activity instance.</param>
/// <param name="ex">Exception to be recorded.</param>
/// <param name="tags">Additional tags to record on the event.</param>
/// <remarks> The exception is recorded as per <a href="https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/trace/exceptions.md">specification</a>.
/// "exception.stacktrace" is represented using the value of <a href="https://learn.microsoft.com/dotnet/api/system.exception.tostring">Exception.ToString</a>.
/// </remarks>
[MethodImpl(MethodImplOptions.AggressiveInlining)]
public static void RecordException(this Activity activity, Exception? ex, in TagList tags)
{
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,8 @@

using System.Diagnostics;
using Microsoft.AspNetCore.Http;
using Microsoft.Extensions.Configuration;
using OpenTelemetry.Internal;

namespace OpenTelemetry.Instrumentation.AspNetCore;

Expand All @@ -11,6 +13,24 @@ namespace OpenTelemetry.Instrumentation.AspNetCore;
/// </summary>
public class AspNetCoreTraceInstrumentationOptions
{
/// <summary>
/// Initializes a new instance of the <see cref="AspNetCoreTraceInstrumentationOptions"/> class.
/// </summary>
public AspNetCoreTraceInstrumentationOptions()
: this(new ConfigurationBuilder().AddEnvironmentVariables().Build())
{
}

internal AspNetCoreTraceInstrumentationOptions(IConfiguration configuration)
{
Debug.Assert(configuration != null, "configuration was null");

if (configuration.TryGetBoolValue("OTEL_DOTNET_EXPERIMENTAL_ASPNETCORE_ENABLE_GRPC_INSTRUMENTATION", out var enableGrpcInstrumentation))
{
this.EnableGrpcAspNetCoreSupport = enableGrpcInstrumentation;
}
}

/// <summary>
/// Gets or sets a filter function that determines whether or not to
/// collect telemetry on a per request basis.
Expand Down Expand Up @@ -64,17 +84,11 @@ public class AspNetCoreTraceInstrumentationOptions
/// </remarks>
public bool RecordException { get; set; }

/*
* Removing for stable release of http instrumentation.
* grpc semantic conventions are not yet stable so this option will not be part of stable package.
#if NET6_0_OR_GREATER
/// <summary>
/// Gets or sets a value indicating whether RPC attributes are added to an Activity when using Grpc.AspNetCore. Default is true.
/// </summary>
/// <remarks>
/// https://github.com/open-telemetry/semantic-conventions/blob/main/docs/rpc/rpc-spans.md.
/// </remarks>
public bool EnableGrpcAspNetCoreSupport { get; set; } = true;
#endif
*/
/// <summary>
/// Gets or sets a value indicating whether RPC attributes are added to an Activity when using Grpc.AspNetCore.
/// </summary>
/// <remarks>
/// https://github.com/open-telemetry/semantic-conventions/blob/main/docs/rpc/rpc-spans.md.
/// </remarks>
internal bool EnableGrpcAspNetCoreSupport { get; set; }
}
14 changes: 14 additions & 0 deletions src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,20 @@

## Unreleased

* Re-introduced support for gRPC instrumentation as an opt-in experimental
feature. From now onwards, gRPC can be enabled by setting
`OTEL_DOTNET_EXPERIMENTAL_ASPNETCORE_ENABLE_GRPC_INSTRUMENTATION` flag to
`True`. `OTEL_DOTNET_EXPERIMENTAL_ASPNETCORE_ENABLE_GRPC_INSTRUMENTATION` can
be set as an environment variable or via IConfiguration. The change is
introduced in order to support stable release of `http` instrumentation.
Semantic conventions for RPC is still
[experimental](https://github.com/open-telemetry/semantic-conventions/tree/main/docs/rpc)
and hence the package will only support it as an opt-in experimental feature.
Note that the support was removed in `1.6.0-rc.1` version of the package and
versions released before `1.6.0-rc.1` had gRPC instrumentation enabled by
default.
([#5130](https://github.com/open-telemetry/opentelemetry-dotnet/pull/5130))

## 1.6.0-rc.1

Released 2023-Dec-01
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,17 +6,13 @@
using System.Diagnostics.CodeAnalysis;
#endif
using System.Reflection;
#if !NETSTANDARD2_0
using System.Runtime.CompilerServices;
#endif
using Microsoft.AspNetCore.Http;
#if !NETSTANDARD
using Microsoft.AspNetCore.Routing;
#endif
using OpenTelemetry.Context.Propagation;
#if !NETSTANDARD2_0
using OpenTelemetry.Instrumentation.GrpcNetClient;
#endif
using OpenTelemetry.Internal;
using OpenTelemetry.Trace;

Expand Down Expand Up @@ -170,7 +166,7 @@ public void OnStartActivity(Activity activity, object payload)
#endif

var path = (request.PathBase.HasValue || request.Path.HasValue) ? (request.PathBase + request.Path).ToString() : "/";
activity.DisplayName = this.GetDisplayName(request.Method);
activity.DisplayName = GetDisplayName(request.Method);

// see the spec https://github.com/open-telemetry/semantic-conventions/blob/v1.23.0/docs/http/http-spans.md

Expand Down Expand Up @@ -233,20 +229,18 @@ public void OnStopActivity(Activity activity, object payload)
var routePattern = (context.GetEndpoint() as RouteEndpoint)?.RoutePattern.RawText;
if (!string.IsNullOrEmpty(routePattern))
{
activity.DisplayName = this.GetDisplayName(context.Request.Method, routePattern);
activity.DisplayName = GetDisplayName(context.Request.Method, routePattern);
activity.SetTag(SemanticConventions.AttributeHttpRoute, routePattern);
}
#endif

activity.SetTag(SemanticConventions.AttributeHttpResponseStatusCode, TelemetryHelper.GetBoxedStatusCode(response.StatusCode));
/*
#if !NETSTANDARD2_0
if (this.options.EnableGrpcAspNetCoreSupport && TryGetGrpcMethod(activity, out var grpcMethod))
{
this.AddGrpcAttributes(activity, grpcMethod, context);
}
*/

if (this.options.EnableGrpcAspNetCoreSupport && TryGetGrpcMethod(activity, out var grpcMethod))
{
AddGrpcAttributes(activity, grpcMethod, context);
}

if (activity.Status == ActivityStatusCode.Unset)
{
activity.SetStatus(SpanHelper.ResolveSpanStatusForHttpStatusCode(activity.Kind, response.StatusCode));
Expand Down Expand Up @@ -327,27 +321,15 @@ static bool TryFetchException(object payload, out Exception exc)
=> ExceptionPropertyFetcher.TryFetch(payload, out exc) && exc != null;
}

#if !NETSTANDARD2_0
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static bool TryGetGrpcMethod(Activity activity, out string grpcMethod)
{
grpcMethod = GrpcTagHelper.GetGrpcMethodFromActivity(activity);
return !string.IsNullOrEmpty(grpcMethod);
}
#endif

private string GetDisplayName(string httpMethod, string httpRoute = null)
{
var normalizedMethod = RequestMethodHelper.GetNormalizedHttpMethod(httpMethod);

return string.IsNullOrEmpty(httpRoute)
? normalizedMethod
: $"{normalizedMethod} {httpRoute}";
}

#if !NETSTANDARD2_0
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private void AddGrpcAttributes(Activity activity, string grpcMethod, HttpContext context)
private static void AddGrpcAttributes(Activity activity, string grpcMethod, HttpContext context)
{
// The RPC semantic conventions indicate the span name
// should not have a leading forward slash.
Expand Down Expand Up @@ -389,5 +371,13 @@ private void AddGrpcAttributes(Activity activity, string grpcMethod, HttpContext
}
}
}
#endif

private static string GetDisplayName(string httpMethod, string httpRoute = null)
{
var normalizedMethod = RequestMethodHelper.GetNormalizedHttpMethod(httpMethod);

return string.IsNullOrEmpty(httpRoute)
? normalizedMethod
: $"{normalizedMethod} {httpRoute}";
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,13 @@
</PropertyGroup>

<ItemGroup>
<Compile Include="$(RepoRoot)\src\Shared\EnvironmentVariables\*.cs" Link="Includes\EnvironmentVariables\%(Filename).cs" Condition="'$(TargetFrameworkIdentifier)' != '.NETCoreApp'" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Instrumentation.GrpcNetClient\GrpcTagHelper.cs" Link="Includes\GrpcTagHelper.cs" />
<Compile Include="$(RepoRoot)\src\OpenTelemetry.Instrumentation.GrpcNetClient\StatusCanonicalCode.cs" Link="Includes\StatusCanonicalCode.cs" />
<Compile Include="$(RepoRoot)\src\Shared\Guard.cs" Link="Includes\Guard.cs" />
<Compile Include="$(RepoRoot)\src\Shared\RequestMethodHelper.cs" Link="Includes\RequestMethodHelper.cs" />
<Compile Include="$(RepoRoot)\src\Shared\Shims\NullableAttributes.cs" Link="Includes\Shims\NullableAttributes.cs" />
<Compile Include="$(RepoRoot)\src\Shared\Options\*.cs" Link="Includes\Options\%(Filename).cs" />
</ItemGroup>

<ItemGroup Condition="'$(RunningDotNetPack)' != 'true'">
Expand All @@ -28,10 +30,11 @@
</ItemGroup>

<ItemGroup Condition="'$(TargetFrameworkIdentifier)' != '.NETCoreApp'">
<PackageReference Include="System.Text.Encodings.Web" />
<PackageReference Include="Microsoft.AspNetCore.Http.Abstractions" />
<PackageReference Include="Microsoft.AspNetCore.Http.Features" />
<PackageReference Include="Microsoft.Extensions.Configuration" />
<PackageReference Include="Microsoft.Extensions.Options" />
<PackageReference Include="System.Text.Encodings.Web" />
</ItemGroup>

<ItemGroup Condition="'$(TargetFrameworkIdentifier)' == '.NETCoreApp'">
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,8 @@ public static TracerProviderBuilder AddAspNetCoreInstrumentation(
{
services.Configure(name, configureAspNetCoreTraceInstrumentationOptions);
}

services.RegisterOptionsFactory(configuration => new AspNetCoreTraceInstrumentationOptions(configuration));
});

if (builder is IDeferredTracerProviderBuilder deferredTracerProviderBuilder)
Expand Down
Loading

0 comments on commit 749ee6e

Please sign in to comment.