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

Fixing ActivitySource from DiagnosticSource #1515

Merged
merged 9 commits into from
Nov 12, 2020
10 changes: 7 additions & 3 deletions src/OpenTelemetry.Instrumentation.AspNet/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,13 +2,17 @@

## Unreleased

* AspNetInstrumentation sets ActivitySource to activities created outside
ActivitySource.
([#1515](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1515/))

## 0.8.0-beta.1

Released 2020-Nov-5

* Renamed TextMapPropagator to TraceContextPropagator, CompositePropapagor
to CompositeTextMapPropagator. IPropagator is renamed to TextMapPropagator
and changed from interface to abstract class.
* Renamed TextMapPropagator to TraceContextPropagator, CompositePropapagor to
CompositeTextMapPropagator. IPropagator is renamed to TextMapPropagator and
changed from interface to abstract class.
([#1427](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1427))
* Propagators.DefaultTextMapPropagator will be used as the default Propagator.
([#1427](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1428))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
using System;
using System.Collections.Generic;
using System.Diagnostics;
using System.Reflection;
using System.Web;
using System.Web.Routing;
using OpenTelemetry.Context.Propagation;
Expand All @@ -28,6 +29,10 @@ internal class HttpInListener : ListenerHandler
{
internal const string ActivityNameByHttpInListener = "ActivityCreatedByHttpInListener";
internal const string ActivityOperationName = "Microsoft.AspNet.HttpReqIn";
internal static readonly AssemblyName AssemblyName = typeof(HttpInListener).Assembly.GetName();
internal static readonly string ActivitySourceName = AssemblyName.Name;
internal static readonly Version Version = AssemblyName.Version;
internal static readonly ActivitySource ActivitySource = new ActivitySource(ActivitySourceName, Version.ToString());
private static readonly Func<HttpRequest, string, IEnumerable<string>> HttpRequestHeaderValuesGetter = (request, name) => request.Headers.GetValues(name);
private readonly PropertyFetcher<object> routeFetcher = new PropertyFetcher<object>("Route");
private readonly PropertyFetcher<string> routeTemplateFetcher = new PropertyFetcher<string>("RouteTemplate");
Expand Down Expand Up @@ -106,7 +111,7 @@ public override void OnStartActivity(Activity activity, object payload)
var path = requestValues.Path;
activity.DisplayName = path;

this.activitySource.Start(activity, ActivityKind.Server);
this.activitySource.Start(activity, ActivityKind.Server, ActivitySource);

if (activity.IsAllDataRequested)
{
Expand Down
14 changes: 9 additions & 5 deletions src/OpenTelemetry.Instrumentation.AspNetCore/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

* AspNetCoreInstrumentation sets ActivitySource to activities created outside
ActivitySource.
([#1515](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1515/))

## 0.8.0-beta.1

Released 2020-Nov-5
Expand All @@ -11,12 +15,12 @@ Released 2020-Nov-5
([#1408](https://github.com/open-telemetry/opentelemetry-dotnet/issues/1408))
* Added configuration option `EnableGrpcAspNetCoreSupport` to enable or disable
support for adding OpenTelemetry RPC attributes when using
[Grpc.AspNetCore](https://www.nuget.org/packages/Grpc.AspNetCore/).
This option is enabled by default.
[Grpc.AspNetCore](https://www.nuget.org/packages/Grpc.AspNetCore/). This
option is enabled by default.
([#1423](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1423))
* Renamed TextMapPropagator to TraceContextPropagator, CompositePropapagor
to CompositeTextMapPropagator. IPropagator is renamed to TextMapPropagator
and changed from interface to abstract class.
* Renamed TextMapPropagator to TraceContextPropagator, CompositePropapagor to
CompositeTextMapPropagator. IPropagator is renamed to TextMapPropagator and
changed from interface to abstract class.
([#1427](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1427))
* Propagators.DefaultTextMapPropagator will be used as the default Propagator
([#1427](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1428))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
using System.Collections.Generic;
using System.Diagnostics;
using System.Linq;
using System.Reflection;
using System.Runtime.CompilerServices;
using System.Text;
using Microsoft.AspNetCore.Http;
Expand All @@ -30,6 +31,10 @@ namespace OpenTelemetry.Instrumentation.AspNetCore.Implementation
{
internal class HttpInListener : ListenerHandler
{
internal static readonly AssemblyName AssemblyName = typeof(HttpInListener).Assembly.GetName();
internal static readonly string ActivitySourceName = AssemblyName.Name;
internal static readonly Version Version = AssemblyName.Version;
internal static readonly ActivitySource ActivitySource = new ActivitySource(ActivitySourceName, Version.ToString());
private const string UnknownHostName = "UNKNOWN-HOST";
private const string ActivityNameByHttpInListener = "ActivityCreatedByHttpInListener";
private static readonly Func<HttpRequest, string, IEnumerable<string>> HttpRequestHeaderValuesGetter = (request, name) => request.Headers[name];
Expand Down Expand Up @@ -104,7 +109,7 @@ public override void OnStartActivity(Activity activity, object payload)
}
}

this.activitySource.Start(activity, ActivityKind.Server);
this.activitySource.Start(activity, ActivityKind.Server, ActivitySource);

if (activity.IsAllDataRequested)
{
Expand Down
6 changes: 4 additions & 2 deletions src/OpenTelemetry.Instrumentation.GrpcNetClient/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,11 @@

## Unreleased

* Add context propagation, when SuppressDownstreamInstrumentation
is enabled.
* Add context propagation, when SuppressDownstreamInstrumentation is enabled.
[#1464](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1464)
* GrpcNetClientInstrumentation sets ActivitySource to activities created outside
ActivitySource.
([#1515](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1515/))

## 0.8.0-beta.1

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
using System;
using System.Diagnostics;
using System.Net.Http;
using System.Reflection;
using OpenTelemetry.Context.Propagation;
using OpenTelemetry.Instrumentation.Http;
using OpenTelemetry.Trace;
Expand All @@ -25,8 +26,12 @@ namespace OpenTelemetry.Instrumentation.GrpcNetClient.Implementation
{
internal class GrpcClientDiagnosticListener : ListenerHandler
{
private readonly GrpcClientInstrumentationOptions options;
internal static readonly AssemblyName AssemblyName = typeof(GrpcClientDiagnosticListener).Assembly.GetName();
internal static readonly string ActivitySourceName = AssemblyName.Name;
internal static readonly Version Version = AssemblyName.Version;
internal static readonly ActivitySource ActivitySource = new ActivitySource(ActivitySourceName, Version.ToString());

private readonly GrpcClientInstrumentationOptions options;
private readonly ActivitySourceAdapter activitySource;
private readonly PropertyFetcher<HttpRequestMessage> startRequestFetcher = new PropertyFetcher<HttpRequestMessage>("Request");

Expand Down Expand Up @@ -83,7 +88,7 @@ public override void OnStartActivity(Activity activity, object payload)

activity.DisplayName = grpcMethod?.Trim('/');

this.activitySource.Start(activity, ActivityKind.Client);
this.activitySource.Start(activity, ActivityKind.Client, ActivitySource);

if (activity.IsAllDataRequested)
{
Expand Down
10 changes: 7 additions & 3 deletions src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

* HttpInstrumentation sets ActivitySource to activities created outside
ActivitySource.
([#1515](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1515/))

## 0.8.0-beta.1

Released 2020-Nov-5
Expand All @@ -10,9 +14,9 @@ Released 2020-Nov-5
`HttpWebRequest` in Activity.CustomProperty. To enrich activity, use the
Enrich action on the instrumentation.
([#1407](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1407))
* Renamed TextMapPropagator to TraceContextPropagator, CompositePropapagor
to CompositeTextMapPropagator. IPropagator is renamed to TextMapPropagator
and changed from interface to abstract class.
* Renamed TextMapPropagator to TraceContextPropagator, CompositePropapagor to
CompositeTextMapPropagator. IPropagator is renamed to TextMapPropagator and
changed from interface to abstract class.
([#1427](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1427))
* Propagators.DefaultTextMapPropagator will be used as the default Propagator
([#1427](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1428))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,11 @@ namespace OpenTelemetry.Instrumentation.Http.Implementation
{
internal class HttpHandlerDiagnosticListener : ListenerHandler
{
internal static readonly AssemblyName AssemblyName = typeof(HttpHandlerDiagnosticListener).Assembly.GetName();
internal static readonly string ActivitySourceName = AssemblyName.Name;
internal static readonly Version Version = AssemblyName.Version;
internal static readonly ActivitySource ActivitySource = new ActivitySource(ActivitySourceName, Version.ToString());

private static readonly Regex CoreAppMajorVersionCheckRegex = new Regex("^\\.NETCoreApp,Version=v(\\d+)\\.", RegexOptions.Compiled | RegexOptions.IgnoreCase);

private readonly ActivitySourceAdapter activitySource;
Expand Down Expand Up @@ -79,7 +84,7 @@ public override void OnStartActivity(Activity activity, object payload)

activity.DisplayName = HttpTagHelper.GetOperationNameForHttpMethod(request.Method);

this.activitySource.Start(activity, ActivityKind.Client);
this.activitySource.Start(activity, ActivityKind.Client, ActivitySource);

if (activity.IsAllDataRequested)
{
Expand Down
4 changes: 4 additions & 0 deletions src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

* SqlInstrumentation sets ActivitySource to activities created outside
ActivitySource.
([#1515](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1515/))

## 0.8.0-beta.1

Released 2020-Nov-5
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
// limitations under the License.
// </copyright>
using System;
using System.Diagnostics;
using OpenTelemetry.Instrumentation.SqlClient.Implementation;

namespace OpenTelemetry.Instrumentation.SqlClient
Expand Down
9 changes: 5 additions & 4 deletions src/OpenTelemetry/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -11,15 +11,16 @@
`CustomProperty`.
([#1463](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1463))
* Remove RentrantExportProcessor as it is not required by spec.
* `ActivitySourceAdapter` supports setting `ActivitySource` for Activities
created without `ActivitySource`.
([#1515](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1515/))

## 0.8.0-beta.1

Released 2020-Nov-5

* TracerProviderBuilder API changes
Renamed AddInstrumentation to AddDiagnosticSourceInstrumentation
and made internal.
Added AddInstrumentation
* TracerProviderBuilder API changes Renamed AddInstrumentation to
AddDiagnosticSourceInstrumentation and made internal. Added AddInstrumentation
([#1454](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1454))

* DiagnosticSource subscription helper classes (DiagnosticSourceSubscriber,
Expand Down
13 changes: 12 additions & 1 deletion src/OpenTelemetry/Trace/ActivitySourceAdapter.cs
Original file line number Diff line number Diff line change
Expand Up @@ -39,6 +39,7 @@ namespace OpenTelemetry.Trace
internal class ActivitySourceAdapter
{
private static readonly Action<Activity, ActivityKind> SetKindProperty = CreateActivityKindSetter();
private static readonly Action<Activity, ActivitySource> SetActivitySourceProperty = CreateActivitySourceSetter();
private readonly Sampler sampler;
private readonly Action<Activity> getRequestedDataAction;
private BaseProcessor<Activity> activityProcessor;
Expand Down Expand Up @@ -71,11 +72,13 @@ private ActivitySourceAdapter()
/// </summary>
/// <param name="activity"><see cref="Activity"/> to be started.</param>
/// <param name="kind">ActivityKind to be set of the activity.</param>
/// <param name="source">ActivitySource to be set of the activity.</param>
[System.Diagnostics.CodeAnalysis.SuppressMessage("Design", "CA1062:Validate arguments of public methods", Justification = "ActivityProcessor is hot path")]
public void Start(Activity activity, ActivityKind kind)
public void Start(Activity activity, ActivityKind kind, ActivitySource source)
{
OpenTelemetrySdkEventSource.Log.ActivityStarted(activity);

SetActivitySourceProperty(activity, source);
SetKindProperty(activity, kind);
this.getRequestedDataAction(activity);
if (activity.IsAllDataRequested)
Expand Down Expand Up @@ -103,6 +106,14 @@ internal void UpdateProcessor(BaseProcessor<Activity> processor)
this.activityProcessor = processor;
}

private static Action<Activity, ActivitySource> CreateActivitySourceSetter()
{
ParameterExpression instance = Expression.Parameter(typeof(Activity), "instance");
ParameterExpression propertyValue = Expression.Parameter(typeof(ActivitySource), "propertyValue");
var body = Expression.Assign(Expression.Property(instance, "Source"), propertyValue);
return Expression.Lambda<Action<Activity, ActivitySource>>(body, instance, propertyValue).Compile();
}

private static Action<Activity, ActivityKind> CreateActivityKindSetter()
{
ParameterExpression instance = Expression.Parameter(typeof(Activity), "instance");
Expand Down
3 changes: 2 additions & 1 deletion test/Benchmarks/Trace/ActivitySourceAdapterBenchmark.cs
Original file line number Diff line number Diff line change
Expand Up @@ -56,6 +56,7 @@ public void ActivitySourceAdapterStartStop()

private class TestInstrumentation
{
internal static ActivitySource ActivitySource = new ActivitySource("test", "1.0.0");
private ActivitySourceAdapter adapter;

public TestInstrumentation(ActivitySourceAdapter adapter)
Expand All @@ -65,7 +66,7 @@ public TestInstrumentation(ActivitySourceAdapter adapter)

public void Start(Activity activity)
{
this.adapter.Start(activity, ActivityKind.Internal);
this.adapter.Start(activity, ActivityKind.Internal, ActivitySource);
}

public void Stop(Activity activity)
Expand Down
14 changes: 7 additions & 7 deletions test/OpenTelemetry.Tests/Trace/ActivitySourceAdapterTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -62,7 +62,7 @@ public void ActivitySourceAdapterSetsKind(ActivityKind kind)
{
var activity = new Activity("test");
activity.Start();
this.activitySourceAdapter.Start(activity, kind);
this.activitySourceAdapter.Start(activity, kind, new ActivitySource("test", "1.0.0"));

Assert.Equal(kind, activity.Kind);
}
Expand Down Expand Up @@ -100,7 +100,7 @@ public void ActivitySourceAdapterCallsStartStopActivityProcessor1(SamplingDecisi

var activity = new Activity("test");
activity.Start();
this.activitySourceAdapter.Start(activity, ActivityKind.Producer);
this.activitySourceAdapter.Start(activity, ActivityKind.Producer, new ActivitySource("test", "1.0.0"));
activity.Stop();
this.activitySourceAdapter.Stop(activity);

Expand Down Expand Up @@ -141,7 +141,7 @@ public void ActivitySourceAdapterCallsStartStopActivityProcessor2(bool isSampled

var activity = new Activity("test");
activity.Start();
this.activitySourceAdapter.Start(activity, ActivityKind.Internal);
this.activitySourceAdapter.Start(activity, ActivityKind.Internal, new ActivitySource("test", "1.0.0"));
activity.Stop();
this.activitySourceAdapter.Stop(activity);

Expand All @@ -164,7 +164,7 @@ public void ActivitySourceAdapterPopulatesSamplingAttributesToActivity(SamplingD

var activity = new Activity("test");
activity.Start();
this.activitySourceAdapter.Start(activity, ActivityKind.Internal);
this.activitySourceAdapter.Start(activity, ActivityKind.Internal, new ActivitySource("test", "1.0.0"));
if (sampling != SamplingDecision.Drop)
{
Assert.Contains(new KeyValuePair<string, object>("tagkeybysampler", "tagvalueaddedbysampler"), activity.TagObjects);
Expand All @@ -186,7 +186,7 @@ public void ActivitySourceAdapterPopulatesSamplingParamsCorrectlyForRootActivity
// and becomes root activity
var activity = new Activity("test");
activity.Start();
this.activitySourceAdapter.Start(activity, ActivityKind.Internal);
this.activitySourceAdapter.Start(activity, ActivityKind.Internal, new ActivitySource("test", "1.0.0"));
activity.Stop();
this.activitySourceAdapter.Stop(activity);
}
Expand Down Expand Up @@ -217,7 +217,7 @@ public void ActivitySourceAdapterPopulatesSamplingParamsCorrectlyForActivityWith
var activity = new Activity("test").SetParentId(remoteParentId);
activity.TraceStateString = tracestate;
activity.Start();
this.activitySourceAdapter.Start(activity, ActivityKind.Internal);
this.activitySourceAdapter.Start(activity, ActivityKind.Internal, new ActivitySource("test", "1.0.0"));
activity.Stop();
this.activitySourceAdapter.Stop(activity);
}
Expand Down Expand Up @@ -250,7 +250,7 @@ public void ActivitySourceAdapterPopulatesSamplingParamsCorrectlyForActivityWith
// i.e of the parent Activity
var activity = new Activity("test");
activity.Start();
this.activitySourceAdapter.Start(activity, ActivityKind.Client);
this.activitySourceAdapter.Start(activity, ActivityKind.Client, new ActivitySource("test", "1.0.0"));
activity.Stop();
this.activitySourceAdapter.Stop(activity);

Expand Down
6 changes: 3 additions & 3 deletions test/OpenTelemetry.Tests/Trace/TracerProviderSdkTest.cs
Original file line number Diff line number Diff line change
Expand Up @@ -283,7 +283,7 @@ public void TracerProvideSdkCreatesActivitySource()
var adapter = testInstrumentation.Adapter;
Activity activity = new Activity("test");
activity.Start();
adapter.Start(activity, ActivityKind.Internal);
adapter.Start(activity, ActivityKind.Internal, new ActivitySource("test", "1.0.0"));
adapter.Stop(activity);
activity.Stop();

Expand Down Expand Up @@ -313,7 +313,7 @@ public void TracerProvideSdkCreatesActivitySource()
tracerProvider.AddProcessor(testActivityProcessorNew);
Activity activityNew = new Activity("test");
activityNew.Start();
adapter.Start(activityNew, ActivityKind.Internal);
adapter.Start(activityNew, ActivityKind.Internal, new ActivitySource("test", "1.0.0"));
adapter.Stop(activityNew);
activityNew.Stop();

Expand All @@ -336,7 +336,7 @@ public void TracerProvideSdkCreatesActivitySourceWhenNoProcessor()
var adapter = testInstrumentation.Adapter;
Activity activity = new Activity("test");
activity.Start();
adapter.Start(activity, ActivityKind.Internal);
adapter.Start(activity, ActivityKind.Internal, new ActivitySource("test", "1.0.0"));
adapter.Stop(activity);
activity.Stop();

Expand Down