Skip to content

Commit

Permalink
Fixing ActivitySource from DiagnosticSource (#1515)
Browse files Browse the repository at this point in the history
* Fixing ActivitySource from DiagnosticSource

* updating changelog

* fixing changelog, updating instrumentation aspnetcore benchmark

* undoing

* updating assembly naming

* updating changelog

Co-authored-by: Cijo Thomas <[email protected]>
  • Loading branch information
eddynaka and cijothomas authored Nov 12, 2020
1 parent d1c174d commit eee0036
Show file tree
Hide file tree
Showing 15 changed files with 86 additions and 34 deletions.
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 @@ -285,7 +285,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 @@ -315,7 +315,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 @@ -338,7 +338,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

0 comments on commit eee0036

Please sign in to comment.