Skip to content

Commit

Permalink
Instrumentation raw objects should be sent as custom properties (#1099)
Browse files Browse the repository at this point in the history
* Added raw objects available to instrumentation as Activity custom properties.

* Updated CHANGELOGs.

* Added "OTel" prefix to custom properties. Moved to constants.

Co-authored-by: Cijo Thomas <cithomas@microsoft.com>
  • Loading branch information
CodeBlanch and cijothomas authored Aug 19, 2020
1 parent 25d9a4b commit 8bd8641
Show file tree
Hide file tree
Showing 10 changed files with 64 additions and 35 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -78,8 +78,8 @@ public override void OnStartActivity(Activity activity, object payload)
// Both new activity and old one store the other activity
// inside them. This is required in the Stop step to
// correctly stop and restore Activity.Current.
newOne.SetCustomProperty("ActivityByAspNet", activity);
activity.SetCustomProperty("ActivityByHttpInListener", newOne);
newOne.SetCustomProperty("OTel.ActivityByAspNet", activity);
activity.SetCustomProperty("OTel.ActivityByHttpInListener", newOne);
activity = newOne;
}

Expand Down Expand Up @@ -135,7 +135,7 @@ public override void OnStopActivity(Activity activity, object payload)
// This block is hit if Asp.Net did restore Current to its own activity,
// and we need to retrieve the one created by HttpInListener,
// or an additional activity was never created.
createdActivity = (Activity)activity.GetCustomProperty("ActivityByHttpInListener");
createdActivity = (Activity)activity.GetCustomProperty("OTel.ActivityByHttpInListener");
activityToEnrich = createdActivity ?? activity;
}
}
Expand Down Expand Up @@ -196,7 +196,7 @@ public override void OnStopActivity(Activity activity, object payload)
activity.Stop();

// Restore the original activity as Current.
var activityByAspNet = (Activity)activity.GetCustomProperty("ActivityByAspNet");
var activityByAspNet = (Activity)activity.GetCustomProperty("OTel.ActivityByAspNet");
Activity.Current = activityByAspNet;
}
else if (createdActivity != null)
Expand Down
4 changes: 4 additions & 0 deletions src/OpenTelemetry.Instrumentation.Grpc/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,10 @@

## Unreleased

* Grpc instrumentation will now add the raw Request object to the Activity it
creates
([#1099](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1099))

## 0.4.0-beta.2

Released 2020-07-24
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ namespace OpenTelemetry.Instrumentation.Grpc.Implementation
{
internal class GrpcClientDiagnosticListener : ListenerHandler
{
public const string RequestCustomPropertyName = "OTel.GrpcHandler.Request";

private readonly ActivitySourceAdapter activitySource;
private readonly PropertyFetcher startRequestFetcher = new PropertyFetcher("Request");

Expand All @@ -48,6 +50,7 @@ public override void OnStartActivity(Activity activity, object payload)

activity.SetKind(ActivityKind.Client);
activity.DisplayName = grpcMethod?.Trim('/');
activity.SetCustomProperty(RequestCustomPropertyName, request);

this.activitySource.Start(activity);

Expand Down
3 changes: 3 additions & 0 deletions src/OpenTelemetry.Instrumentation.Http/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@

## Unreleased

* ASP.NET Core instrumentation will now add the raw Request, Response, and/or
Exception objects to the Activity it creates
([#1099](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1099))
* Changed the default propagation to support W3C Baggage
([#1048](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1048))
* The default ITextFormat is now `CompositePropagator(TraceContextFormat,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,10 @@ namespace OpenTelemetry.Instrumentation.Http.Implementation
{
internal class HttpHandlerDiagnosticListener : ListenerHandler
{
public const string RequestCustomPropertyName = "OTel.HttpHandler.Request";
public const string ResponseCustomPropertyName = "OTel.HttpHandler.Response";
public const string ExceptionCustomPropertyName = "OTel.HttpHandler.Exception";

private static readonly Func<HttpRequestMessage, string, IEnumerable<string>> HttpRequestMessageHeaderValuesGetter = (request, name) =>
{
if (request.Headers.TryGetValues(name, out var values))
Expand Down Expand Up @@ -90,6 +94,7 @@ public override void OnStartActivity(Activity activity, object payload)

activity.SetKind(ActivityKind.Client);
activity.DisplayName = HttpTagHelper.GetOperationNameForHttpMethod(request.Method);
activity.SetCustomProperty(RequestCustomPropertyName, request);

this.activitySource.Start(activity);

Expand Down Expand Up @@ -135,7 +140,8 @@ public override void OnStopActivity(Activity activity, object payload)

if (this.stopResponseFetcher.Fetch(payload) is HttpResponseMessage response)
{
// response could be null for DNS issues, timeouts, etc...
activity.SetCustomProperty(ResponseCustomPropertyName, response);

activity.SetTag(SemanticConventions.AttributeHttpStatusCode, (int)response.StatusCode);

activity.SetStatus(
Expand All @@ -158,6 +164,8 @@ public override void OnException(Activity activity, object payload)
return;
}

activity.SetCustomProperty(ExceptionCustomPropertyName, exc);

if (exc is HttpRequestException)
{
if (exc.InnerException is SocketException exception)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,12 @@ namespace OpenTelemetry.Instrumentation.Http.Implementation
/// </remarks>
internal static class HttpWebRequestActivitySource
{
internal const string ActivitySourceName = "OpenTelemetry.HttpWebRequest";
internal const string ActivityName = ActivitySourceName + ".HttpRequestOut";
public const string ActivitySourceName = "OpenTelemetry.HttpWebRequest";
public const string ActivityName = ActivitySourceName + ".HttpRequestOut";

public const string RequestCustomPropertyName = "OTel.HttpWebRequest.Request";
public const string ResponseCustomPropertyName = "OTel.HttpWebRequest.Response";
public const string ExceptionCustomPropertyName = "OTel.HttpWebRequest.Exception";

internal static readonly Func<HttpWebRequest, string, IEnumerable<string>> HttpWebRequestHeaderValuesGetter = (request, name) => request.Headers.GetValues(name);
internal static readonly Action<HttpWebRequest, string, string> HttpWebRequestHeaderValuesSetter = (request, name, value) => request.Headers.Add(name, value);
Expand Down Expand Up @@ -97,7 +101,7 @@ private static void AddRequestTagsAndInstrumentRequest(HttpWebRequest request, A

InstrumentRequest(request, activity);

activity.SetCustomProperty("HttpWebRequest.Request", request);
activity.SetCustomProperty(RequestCustomPropertyName, request);

if (activity.IsAllDataRequested)
{
Expand All @@ -114,7 +118,7 @@ private static void AddRequestTagsAndInstrumentRequest(HttpWebRequest request, A
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static void AddResponseTags(HttpWebResponse response, Activity activity)
{
activity.SetCustomProperty("HttpWebRequest.Response", response);
activity.SetCustomProperty(ResponseCustomPropertyName, response);

if (activity.IsAllDataRequested)
{
Expand All @@ -130,7 +134,7 @@ private static void AddResponseTags(HttpWebResponse response, Activity activity)
[MethodImpl(MethodImplOptions.AggressiveInlining)]
private static void AddExceptionTags(Exception exception, Activity activity)
{
activity.SetCustomProperty("HttpWebRequest.Exception", exception);
activity.SetCustomProperty(ExceptionCustomPropertyName, exception);

if (!activity.IsAllDataRequested)
{
Expand Down
6 changes: 5 additions & 1 deletion src/OpenTelemetry.Instrumentation.SqlClient/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,11 @@

## Unreleased

* Renamed from `AddSqlClientDependencyInstrumentation` to `AddSqlClientInstrumentation`
* .NET Core SqlClient instrumentation will now add the raw Command object to the
Activity it creates
([#1099](https://github.com/open-telemetry/opentelemetry-dotnet/pull/1099))
* Renamed from `AddSqlClientDependencyInstrumentation` to
`AddSqlClientInstrumentation`

## 0.4.0-beta.2

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -22,19 +22,21 @@ namespace OpenTelemetry.Instrumentation.SqlClient.Implementation
{
internal class SqlClientDiagnosticListener : ListenerHandler
{
internal const string ActivitySourceName = "OpenTelemetry.SqlClient";
internal const string ActivityName = ActivitySourceName + ".Execute";
public const string ActivitySourceName = "OpenTelemetry.SqlClient";
public const string ActivityName = ActivitySourceName + ".Execute";

internal const string SqlDataBeforeExecuteCommand = "System.Data.SqlClient.WriteCommandBefore";
internal const string SqlMicrosoftBeforeExecuteCommand = "Microsoft.Data.SqlClient.WriteCommandBefore";
public const string CommandCustomPropertyName = "OTel.SqlHandler.Command";

internal const string SqlDataAfterExecuteCommand = "System.Data.SqlClient.WriteCommandAfter";
internal const string SqlMicrosoftAfterExecuteCommand = "Microsoft.Data.SqlClient.WriteCommandAfter";
public const string SqlDataBeforeExecuteCommand = "System.Data.SqlClient.WriteCommandBefore";
public const string SqlMicrosoftBeforeExecuteCommand = "Microsoft.Data.SqlClient.WriteCommandBefore";

internal const string SqlDataWriteCommandError = "System.Data.SqlClient.WriteCommandError";
internal const string SqlMicrosoftWriteCommandError = "Microsoft.Data.SqlClient.WriteCommandError";
public const string SqlDataAfterExecuteCommand = "System.Data.SqlClient.WriteCommandAfter";
public const string SqlMicrosoftAfterExecuteCommand = "Microsoft.Data.SqlClient.WriteCommandAfter";

internal const string MicrosoftSqlServerDatabaseSystemName = "mssql";
public const string SqlDataWriteCommandError = "System.Data.SqlClient.WriteCommandError";
public const string SqlMicrosoftWriteCommandError = "Microsoft.Data.SqlClient.WriteCommandError";

public const string MicrosoftSqlServerDatabaseSystemName = "mssql";

private static readonly Version Version = typeof(SqlClientDiagnosticListener).Assembly.GetName().Version;
#pragma warning disable SA1202 // Elements should be ordered by access <- In this case, Version MUST come before SqlClientActivitySource otherwise null ref exception is thrown.
Expand Down Expand Up @@ -85,6 +87,7 @@ public override void OnCustom(string name, Activity activity, object payload)
var database = this.databaseFetcher.Fetch(connection);

activity.DisplayName = (string)database;
activity.SetCustomProperty(CommandCustomPropertyName, command);

if (activity.IsAllDataRequested)
{
Expand Down
2 changes: 1 addition & 1 deletion src/OpenTelemetry/Trace/ActivityExtensions.cs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ namespace OpenTelemetry.Trace
/// </summary>
public static class ActivityExtensions
{
internal const string ResourcePropertyName = "otel.resource";
internal const string ResourcePropertyName = "OTel.Resource";

/// <summary>
/// Gets the Resource associated with the Activity.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -185,7 +185,7 @@ public async Task TestBasicReceiveAndResponseEvents(string method, string queryS

Assert.True(eventRecords.Records.TryDequeue(out var stopEvent));
Assert.Equal("Stop", stopEvent.Key);
HttpWebResponse response = (HttpWebResponse)stopEvent.Value.GetCustomProperty("HttpWebRequest.Response");
HttpWebResponse response = (HttpWebResponse)stopEvent.Value.GetCustomProperty(HttpWebRequestActivitySource.ResponseCustomPropertyName);
Assert.NotNull(response);

VerifyActivityStopTags(200, "OK", activity);
Expand Down Expand Up @@ -237,7 +237,7 @@ public async Task TestBasicReceiveAndResponseEventsWitPassThroughSampling(string

Assert.True(eventRecords.Records.TryDequeue(out var stopEvent));
Assert.Equal("Stop", stopEvent.Key);
HttpWebResponse response = (HttpWebResponse)stopEvent.Value.GetCustomProperty("HttpWebRequest.Response");
HttpWebResponse response = (HttpWebResponse)stopEvent.Value.GetCustomProperty(HttpWebRequestActivitySource.ResponseCustomPropertyName);
Assert.NotNull(response);

Assert.Empty(activity.Tags);
Expand Down Expand Up @@ -401,7 +401,7 @@ public async Task TestBasicReceiveAndResponseWebRequestEvents(string method, int

Assert.True(eventRecords.Records.TryDequeue(out var stopEvent));
Assert.Equal("Stop", stopEvent.Key);
HttpWebResponse response = (HttpWebResponse)stopEvent.Value.GetCustomProperty("HttpWebRequest.Response");
HttpWebResponse response = (HttpWebResponse)stopEvent.Value.GetCustomProperty(HttpWebRequestActivitySource.ResponseCustomPropertyName);
Assert.NotNull(response);

VerifyActivityStopTags(200, "OK", activity);
Expand Down Expand Up @@ -518,9 +518,9 @@ public async Task TestResponseWithoutContentEvents(string method)

Assert.True(eventRecords.Records.TryDequeue(out var stopEvent));
Assert.Equal("Stop", stopEvent.Key);
HttpWebRequest stopRequest = (HttpWebRequest)stopEvent.Value.GetCustomProperty("HttpWebRequest.Request");
HttpWebRequest stopRequest = (HttpWebRequest)stopEvent.Value.GetCustomProperty(HttpWebRequestActivitySource.RequestCustomPropertyName);
Assert.Equal(startRequest, stopRequest);
HttpWebResponse stopResponse = (HttpWebResponse)stopEvent.Value.GetCustomProperty("HttpWebRequest.Response");
HttpWebResponse stopResponse = (HttpWebResponse)stopEvent.Value.GetCustomProperty(HttpWebRequestActivitySource.ResponseCustomPropertyName);
Assert.NotNull(stopResponse);

VerifyActivityStopTags(204, "No Content", activity);
Expand Down Expand Up @@ -587,9 +587,9 @@ public async Task TestRequestWithException(string method)

Assert.True(eventRecords.Records.TryDequeue(out KeyValuePair<string, Activity> exceptionEvent));
Assert.Equal("Stop", exceptionEvent.Key);
HttpWebRequest exceptionRequest = (HttpWebRequest)exceptionEvent.Value.GetCustomProperty("HttpWebRequest.Request");
HttpWebRequest exceptionRequest = (HttpWebRequest)exceptionEvent.Value.GetCustomProperty(HttpWebRequestActivitySource.RequestCustomPropertyName);
Assert.Equal(startRequest, exceptionRequest);
Exception exceptionException = (Exception)exceptionEvent.Value.GetCustomProperty("HttpWebRequest.Exception");
Exception exceptionException = (Exception)exceptionEvent.Value.GetCustomProperty(HttpWebRequestActivitySource.ExceptionCustomPropertyName);
Assert.Equal(webException, exceptionException);

Assert.Contains(activity.Tags, i => i.Key == SpanAttributeConstants.StatusCodeKey);
Expand Down Expand Up @@ -631,7 +631,7 @@ public async Task TestCanceledRequest(string method)

Assert.True(eventRecords.Records.TryDequeue(out KeyValuePair<string, Activity> exceptionEvent));
Assert.Equal("Stop", exceptionEvent.Key);
Exception exceptionException = (Exception)exceptionEvent.Value.GetCustomProperty("HttpWebRequest.Exception");
Exception exceptionException = (Exception)exceptionEvent.Value.GetCustomProperty(HttpWebRequestActivitySource.ExceptionCustomPropertyName);

Assert.Contains(exceptionEvent.Value.Tags, i => i.Key == SpanAttributeConstants.StatusCodeKey);
Assert.DoesNotContain(exceptionEvent.Value.Tags, i => i.Key == SpanAttributeConstants.StatusDescriptionKey);
Expand Down Expand Up @@ -672,7 +672,7 @@ public async Task TestSecureTransportFailureRequest(string method)

Assert.True(eventRecords.Records.TryDequeue(out KeyValuePair<string, Activity> exceptionEvent));
Assert.Equal("Stop", exceptionEvent.Key);
Exception exceptionException = (Exception)exceptionEvent.Value.GetCustomProperty("HttpWebRequest.Exception");
Exception exceptionException = (Exception)exceptionEvent.Value.GetCustomProperty(HttpWebRequestActivitySource.ExceptionCustomPropertyName);

Assert.Contains(exceptionEvent.Value.Tags, i => i.Key == SpanAttributeConstants.StatusCodeKey);
Assert.Contains(exceptionEvent.Value.Tags, i => i.Key == SpanAttributeConstants.StatusDescriptionKey);
Expand Down Expand Up @@ -716,7 +716,7 @@ public async Task TestSecureTransportRetryFailureRequest(string method)

Assert.True(eventRecords.Records.TryDequeue(out KeyValuePair<string, Activity> exceptionEvent));
Assert.Equal("Stop", exceptionEvent.Key);
Exception exceptionException = (Exception)exceptionEvent.Value.GetCustomProperty("HttpWebRequest.Exception");
Exception exceptionException = (Exception)exceptionEvent.Value.GetCustomProperty(HttpWebRequestActivitySource.ExceptionCustomPropertyName);

Assert.Contains(exceptionEvent.Value.Tags, i => i.Key == SpanAttributeConstants.StatusCodeKey);
Assert.Contains(exceptionEvent.Value.Tags, i => i.Key == SpanAttributeConstants.StatusDescriptionKey);
Expand All @@ -741,7 +741,7 @@ public async Task TestInvalidBaggage()
Assert.Equal(1, eventRecords.Records.Count(rec => rec.Key == "Start"));
Assert.Equal(1, eventRecords.Records.Count(rec => rec.Key == "Stop"));

WebRequest thisRequest = (WebRequest)eventRecords.Records.First().Value.GetCustomProperty("HttpWebRequest.Request");
WebRequest thisRequest = (WebRequest)eventRecords.Records.First().Value.GetCustomProperty(HttpWebRequestActivitySource.RequestCustomPropertyName);
string[] correlationContext = thisRequest.Headers["baggage"].Split(',');

Assert.Equal(3, correlationContext.Length);
Expand Down Expand Up @@ -809,7 +809,7 @@ public void TestMultipleConcurrentRequests()
pair.Key == "Stop",
"An unexpected event of name " + pair.Key + "was received");

WebRequest request = (WebRequest)activity.GetCustomProperty("HttpWebRequest.Request");
WebRequest request = (WebRequest)activity.GetCustomProperty(HttpWebRequestActivitySource.RequestCustomPropertyName);
Assert.Equal("HttpWebRequest", request.GetType().Name);

if (pair.Key == "Start")
Expand All @@ -831,7 +831,7 @@ public void TestMultipleConcurrentRequests()
else
{
// This must be the response.
WebResponse response = (WebResponse)activity.GetCustomProperty("HttpWebRequest.Response");
WebResponse response = (WebResponse)activity.GetCustomProperty(HttpWebRequestActivitySource.ResponseCustomPropertyName);
Assert.Equal("HttpWebResponse", response.GetType().Name);

// By the time we see the response, the request object may already have been redirected with a different
Expand Down Expand Up @@ -871,7 +871,7 @@ private static (Activity, HttpWebRequest) AssertFirstEventWasStart(ActivitySourc
{
Assert.True(eventRecords.Records.TryDequeue(out KeyValuePair<string, Activity> startEvent));
Assert.Equal("Start", startEvent.Key);
HttpWebRequest startRequest = (HttpWebRequest)startEvent.Value.GetCustomProperty("HttpWebRequest.Request");
HttpWebRequest startRequest = (HttpWebRequest)startEvent.Value.GetCustomProperty(HttpWebRequestActivitySource.RequestCustomPropertyName);
Assert.NotNull(startRequest);
return (startEvent.Value, startRequest);
}
Expand Down

0 comments on commit 8bd8641

Please sign in to comment.