Skip to content

Commit

Permalink
Fixing enrich order (#1548)
Browse files Browse the repository at this point in the history
  • Loading branch information
@eddynaka
eddynaka authored Nov 16, 2020
1 parent 51806b7 commit a944d94
Show file tree
Hide file tree
Showing 5 changed files with 105 additions and 91 deletions.
50 changes: 25 additions & 25 deletions src/OpenTelemetry.Instrumentation.AspNetCore/Implementation/HttpInListener.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -112,15 +112,6 @@ public override void OnStartActivity(Activity activity, object payload)

if (activity.IsAllDataRequested)
{
try
{
this.options.Enrich?.Invoke(activity, "OnStartActivity", request);
}
catch (Exception ex)
{
AspNetCoreInstrumentationEventSource.Log.EnrichmentException(ex);
}

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

Expand All @@ -144,6 +135,15 @@ public override void OnStartActivity(Activity activity, object payload)
{
activity.SetTag(SemanticConventions.AttributeHttpUserAgent, userAgent);
}

try
{
this.options.Enrich?.Invoke(activity, "OnStartActivity", request);
}
catch (Exception ex)
{
AspNetCoreInstrumentationEventSource.Log.EnrichmentException(ex);
}
}
}

Expand All @@ -160,15 +160,6 @@ public override void OnStopActivity(Activity activity, object payload)

var response = context.Response;

try
{
this.options.Enrich?.Invoke(activity, "OnStopActivity", response);
}
catch (Exception ex)
{
AspNetCoreInstrumentationEventSource.Log.EnrichmentException(ex);
}

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

#if NETSTANDARD2_1
Expand All @@ -183,6 +174,15 @@ public override void OnStopActivity(Activity activity, object payload)
#else
SetStatusFromHttpStatusCode(activity, response.StatusCode);
#endif

try
{
this.options.Enrich?.Invoke(activity, "OnStopActivity", response);
}
catch (Exception ex)
{
AspNetCoreInstrumentationEventSource.Log.EnrichmentException(ex);
}
}

if (activity.OperationName.Equals(ActivityNameByHttpInListener, StringComparison.Ordinal))
Expand Down Expand Up @@ -245,6 +245,13 @@ public override void OnException(Activity activity, object payload)
return;
}

if (this.options.RecordException)
{
activity.RecordException(exc);
}

activity.SetStatus(Status.Error.WithDescription(exc.Message));

try
{
this.options.Enrich?.Invoke(activity, "OnException", exc);
Expand All @@ -253,13 +260,6 @@ public override void OnException(Activity activity, object payload)
{
AspNetCoreInstrumentationEventSource.Log.EnrichmentException(ex);
}

if (this.options.RecordException)
{
activity.RecordException(exc);
}

activity.SetStatus(Status.Error.WithDescription(exc.Message));
}
}

Expand Down
31 changes: 22 additions & 9 deletions ...penTelemetry.Instrumentation.GrpcNetClient/Implementation/GrpcClientDiagnosticListener.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ internal class GrpcClientDiagnosticListener : ListenerHandler
private readonly GrpcClientInstrumentationOptions options;
private readonly ActivitySourceAdapter activitySource;
private readonly PropertyFetcher<HttpRequestMessage> startRequestFetcher = new PropertyFetcher<HttpRequestMessage>("Request");
private readonly PropertyFetcher<HttpResponseMessage> stopRequestFetcher = new PropertyFetcher<HttpResponseMessage>("Response");

public GrpcClientDiagnosticListener(ActivitySourceAdapter activitySource, GrpcClientInstrumentationOptions options)
: base("Grpc.Net.Client")
Expand Down Expand Up @@ -92,15 +93,6 @@ public override void OnStartActivity(Activity activity, object payload)

if (activity.IsAllDataRequested)
{
try
{
this.options.Enrich?.Invoke(activity, "OnStartActivity", request);
}
catch (Exception ex)
{
GrpcInstrumentationEventSource.Log.EnrichmentException(ex);
}

activity.SetTag(SemanticConventions.AttributeRpcSystem, GrpcTagHelper.RpcSystemGrpc);

if (GrpcTagHelper.TryParseRpcServiceAndRpcMethod(grpcMethod, out var rpcService, out var rpcMethod))
Expand All @@ -123,6 +115,15 @@ public override void OnStartActivity(Activity activity, object payload)
}

activity.SetTag(SemanticConventions.AttributeNetPeerPort, request.RequestUri.Port);

try
{
this.options.Enrich?.Invoke(activity, "OnStartActivity", request);
}
catch (Exception ex)
{
GrpcInstrumentationEventSource.Log.EnrichmentException(ex);
}
}
}

Expand All @@ -141,6 +142,18 @@ public override void OnStopActivity(Activity activity, object payload)

// Remove the grpc.status_code tag added by the gRPC .NET library
activity.SetTag(GrpcTagHelper.GrpcStatusCodeTagName, null);

if (this.stopRequestFetcher.TryFetch(payload, out HttpResponseMessage response) && response != null)
{
try
{
this.options.Enrich?.Invoke(activity, "OnStopActivity", response);
}
catch (Exception ex)
{
GrpcInstrumentationEventSource.Log.EnrichmentException(ex);
}
}
}

this.activitySource.Stop(activity);
Expand Down
50 changes: 25 additions & 25 deletions src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,15 +88,6 @@ public override void OnStartActivity(Activity activity, object payload)

if (activity.IsAllDataRequested)
{
try
{
this.options.Enrich?.Invoke(activity, "OnStartActivity", request);
}
catch (Exception ex)
{
HttpInstrumentationEventSource.Log.EnrichmentException(ex);
}

activity.SetTag(SemanticConventions.AttributeHttpMethod, HttpTagHelper.GetNameForHttpMethod(request.Method));
activity.SetTag(SemanticConventions.AttributeHttpHost, HttpTagHelper.GetHostTagValueFromRequestUri(request.RequestUri));
activity.SetTag(SemanticConventions.AttributeHttpUrl, request.RequestUri.OriginalString);
Expand All @@ -105,6 +96,15 @@ public override void OnStartActivity(Activity activity, object payload)
{
activity.SetTag(SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.Version));
}

try
{
this.options.Enrich?.Invoke(activity, "OnStartActivity", request);
}
catch (Exception ex)
{
HttpInstrumentationEventSource.Log.EnrichmentException(ex);
}
}

var textMapPropagator = Propagators.DefaultTextMapPropagator;
Expand Down Expand Up @@ -138,6 +138,13 @@ public override void OnStopActivity(Activity activity, object payload)

if (this.stopResponseFetcher.TryFetch(payload, out HttpResponseMessage response) && response != null)
{
activity.SetTag(SemanticConventions.AttributeHttpStatusCode, (int)response.StatusCode);

activity.SetStatus(
SpanHelper
.ResolveSpanStatusForHttpStatusCode((int)response.StatusCode)
.WithDescription(response.ReasonPhrase));

try
{
this.options.Enrich?.Invoke(activity, "OnStopActivity", response);
Expand All @@ -146,13 +153,6 @@ public override void OnStopActivity(Activity activity, object payload)
{
HttpInstrumentationEventSource.Log.EnrichmentException(ex);
}

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

activity.SetStatus(
SpanHelper
.ResolveSpanStatusForHttpStatusCode((int)response.StatusCode)
.WithDescription(response.ReasonPhrase));
}
}

Expand All @@ -169,15 +169,6 @@ public override void OnException(Activity activity, object payload)
return;
}

try
{
this.options.Enrich?.Invoke(activity, "OnException", exc);
}
catch (Exception ex)
{
HttpInstrumentationEventSource.Log.EnrichmentException(ex);
}

if (exc is HttpRequestException)
{
if (exc.InnerException is SocketException exception)
Expand All @@ -195,6 +186,15 @@ public override void OnException(Activity activity, object payload)
activity.SetStatus(Status.Error.WithDescription(exc.Message));
}
}

try
{
this.options.Enrich?.Invoke(activity, "OnException", exc);
}
catch (Exception ex)
{
HttpInstrumentationEventSource.Log.EnrichmentException(ex);
}
}
}
}
Expand Down
48 changes: 24 additions & 24 deletions src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -96,6 +96,14 @@ private static void AddRequestTagsAndInstrumentRequest(HttpWebRequest request, A

if (activity.IsAllDataRequested)
{
activity.SetTag(SemanticConventions.AttributeHttpMethod, request.Method);
activity.SetTag(SemanticConventions.AttributeHttpHost, HttpTagHelper.GetHostTagValueFromRequestUri(request.RequestUri));
activity.SetTag(SemanticConventions.AttributeHttpUrl, request.RequestUri.OriginalString);
if (Options.SetHttpFlavor)
{
activity.SetTag(SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.ProtocolVersion));
}

try
{
Options.Enrich?.Invoke(activity, "OnStartActivity", request);
Expand All @@ -104,14 +112,6 @@ private static void AddRequestTagsAndInstrumentRequest(HttpWebRequest request, A
{
HttpInstrumentationEventSource.Log.EnrichmentException(ex);
}

activity.SetTag(SemanticConventions.AttributeHttpMethod, request.Method);
activity.SetTag(SemanticConventions.AttributeHttpHost, HttpTagHelper.GetHostTagValueFromRequestUri(request.RequestUri));
activity.SetTag(SemanticConventions.AttributeHttpUrl, request.RequestUri.OriginalString);
if (Options.SetHttpFlavor)
{
activity.SetTag(SemanticConventions.AttributeHttpFlavor, HttpTagHelper.GetFlavorTagValueFromProtocolVersion(request.ProtocolVersion));
}
}
}

Expand All @@ -120,6 +120,13 @@ private static void AddResponseTags(HttpWebResponse response, Activity activity)
{
if (activity.IsAllDataRequested)
{
activity.SetTag(SemanticConventions.AttributeHttpStatusCode, (int)response.StatusCode);

activity.SetStatus(
SpanHelper
.ResolveSpanStatusForHttpStatusCode((int)response.StatusCode)
.WithDescription(response.StatusDescription));

try
{
Options.Enrich?.Invoke(activity, "OnStopActivity", response);
Expand All @@ -128,13 +135,6 @@ private static void AddResponseTags(HttpWebResponse response, Activity activity)
{
HttpInstrumentationEventSource.Log.EnrichmentException(ex);
}

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

activity.SetStatus(
SpanHelper
.ResolveSpanStatusForHttpStatusCode((int)response.StatusCode)
.WithDescription(response.StatusDescription));
}
}

Expand All @@ -146,15 +146,6 @@ private static void AddExceptionTags(Exception exception, Activity activity)
return;
}

try
{
Options.Enrich?.Invoke(activity, "OnException", exception);
}
catch (Exception ex)
{
HttpInstrumentationEventSource.Log.EnrichmentException(ex);
}

Status status;
if (exception is WebException wexc)
{
Expand Down Expand Up @@ -192,6 +183,15 @@ private static void AddExceptionTags(Exception exception, Activity activity)
}

activity.SetStatus(status);

try
{
Options.Enrich?.Invoke(activity, "OnException", exception);
}
catch (Exception ex)
{
HttpInstrumentationEventSource.Log.EnrichmentException(ex);
}
}

[MethodImpl(MethodImplOptions.AggressiveInlining)]
Expand Down
17 changes: 9 additions & 8 deletions src/OpenTelemetry.Instrumentation.SqlClient/Implementation/SqlClientDiagnosticListener.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -89,14 +89,6 @@ public override void OnCustom(string name, Activity activity, object payload)
_ = this.databaseFetcher.TryFetch(connection, out var database);

activity.DisplayName = (string)database;
try
{
this.options.Enrich?.Invoke(activity, "OnCustom", command);
}
catch (Exception ex)
{
SqlClientInstrumentationEventSource.Log.EnrichmentException(ex);
}

_ = this.dataSourceFetcher.TryFetch(connection, out var dataSource);
_ = this.commandTextFetcher.TryFetch(command, out var commandText);
Expand Down Expand Up @@ -133,6 +125,15 @@ public override void OnCustom(string name, Activity activity, object payload)
break;
}
}

try
{
this.options.Enrich?.Invoke(activity, "OnCustom", command);
}
catch (Exception ex)
{
SqlClientInstrumentationEventSource.Log.EnrichmentException(ex);
}
}
}

Expand Down

0 comments on commit a944d94

Please sign in to comment.