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
Original file line number Diff line number Diff line change
Expand Up @@ -26,8 +26,11 @@ namespace OpenTelemetry.Instrumentation.AspNet.Implementation
{
internal class HttpInListener : ListenerHandler
{
internal const string ActivitySourceName = "OpenTelemetry.Http";
eddynaka marked this conversation as resolved.
Show resolved Hide resolved
internal const string ActivityNameByHttpInListener = "ActivityCreatedByHttpInListener";
internal const string ActivityOperationName = "Microsoft.AspNet.HttpReqIn";
internal static readonly Version Version = typeof(HttpInListener).Assembly.GetName().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 +109,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
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,9 @@ namespace OpenTelemetry.Instrumentation.AspNetCore.Implementation
{
internal class HttpInListener : ListenerHandler
{
internal const string ActivitySourceName = "OpenTelemetry.AspNetCore";
internal static readonly Version Version = typeof(HttpInListener).Assembly.GetName().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 +107,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
Original file line number Diff line number Diff line change
Expand Up @@ -25,8 +25,11 @@ namespace OpenTelemetry.Instrumentation.GrpcNetClient.Implementation
{
internal class GrpcClientDiagnosticListener : ListenerHandler
{
private readonly GrpcClientInstrumentationOptions options;
internal const string ActivitySourceName = "OpenTelemetry.Grpc";
internal static readonly Version Version = typeof(GrpcClientDiagnosticListener).Assembly.GetName().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 +86,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
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,10 @@ namespace OpenTelemetry.Instrumentation.Http.Implementation
{
internal class HttpHandlerDiagnosticListener : ListenerHandler
{
internal const string ActivitySourceName = "OpenTelemetry.Http";
internal static readonly Version Version = typeof(HttpHandlerDiagnosticListener).Assembly.GetName().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 +83,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
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.Start` requires `ActivitySource` to update new
cijothomas marked this conversation as resolved.
Show resolved Hide resolved
activities.
([#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