Skip to content

Commit

Permalink
Excluding span links based on decisions made in open-telemetry/opente…
Browse files Browse the repository at this point in the history 
…lemetry-specification#2078
  • Loading branch information
@denisivan0v
denisivan0v committed Mar 22, 2022
1 parent 6504aef commit 00b6a84
Show file tree
Hide file tree
Showing 6 changed files with 4 additions and 175 deletions.
2 changes: 0 additions & 2 deletions src/OpenTelemetry.Api/Internal/SpanAttributeConstants.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,8 +23,6 @@ internal static class SpanAttributeConstants
{
public const string StatusCodeKey = "otel.status_code";
public const string StatusDescriptionKey = "otel.status_description";
public const string PreviousTryContextKey = "otel.previous_try_context";
public const string RetryCountKey = "http.retry_count";
public const string DatabaseStatementTypeKey = "db.statement_type";
}
}
37 changes: 0 additions & 37 deletions src/OpenTelemetry.Instrumentation.Http/Implementation/HttpHandlerDiagnosticListener.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,6 @@ internal sealed class HttpHandlerDiagnosticListener : ListenerHandler
internal static readonly Version Version = AssemblyName.Version;
internal static readonly ActivitySource ActivitySource = new(ActivitySourceName, Version.ToString());

private const string RestartedActivityKey = "dotnet.restarted_activity";

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

private readonly PropertyFetcher<HttpRequestMessage> startRequestFetcher = new("Request");
Expand Down Expand Up @@ -90,32 +88,6 @@ public override void OnStartActivity(Activity activity, object payload)
return;
}

var context = Propagators.DefaultTextMapPropagator.Extract(default, request, HttpRequestMessageContextPropagation.HeaderValuesGetter);
if (context != default && request.Properties.TryGetValue(SpanAttributeConstants.PreviousTryContextKey, out var previousContext))
{
// Handling request retry or redirect.
var retryCount = 1;
if (request.Properties.TryGetValue(SpanAttributeConstants.RetryCountKey, out var previousRetryCount))
{
retryCount = (int)previousRetryCount + 1;
}

// Suppressing activity started by HttpClient DiagnosticsHandler.
activity.IsAllDataRequested = false;
activity.Stop();

activity = ActivitySource.StartActivity(activity.Kind, context.ActivityContext, links: new[] { new ActivityLink((ActivityContext)previousContext) });
Activity.Current = activity;

request.Properties[RestartedActivityKey] = activity;

activity.SetTag(SpanAttributeConstants.RetryCountKey, retryCount);
request.Properties[SpanAttributeConstants.RetryCountKey] = retryCount;
}

// Store activity context for the next possible try.
request.Properties[SpanAttributeConstants.PreviousTryContextKey] = activity.Context;

// Propagate context irrespective of sampling decision
var textMapPropagator = Propagators.DefaultTextMapPropagator;
if (!(this.httpClientSupportsW3C && textMapPropagator is TraceContextPropagator))
Expand Down Expand Up @@ -215,15 +187,6 @@ public override void OnStopActivity(Activity activity, object payload)
HttpInstrumentationEventSource.Log.EnrichmentException(ex);
}
}

if (this.startRequestFetcher.TryFetch(payload, out HttpRequestMessage request) && request != null)
{
if (request.Properties.TryGetValue(RestartedActivityKey, out object restartedActivityInstance))
{
var restartedActivity = (Activity)restartedActivityInstance;
restartedActivity.Stop();
}
}
}
}

Expand Down
45 changes: 1 addition & 44 deletions src/OpenTelemetry.Instrumentation.Http/Implementation/HttpWebRequestActivitySource.netfx.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -213,40 +213,8 @@ private static void ProcessRequest(HttpWebRequest request)
return;
}

if (RequestProperties.Instance == null)
{
RequestProperties.Instance = new Dictionary<string, object>();
}

Activity activity;
var context = Propagators.DefaultTextMapPropagator.Extract(default, request, HttpWebRequestHeaderValuesGetter);
if (context != default && RequestProperties.Instance.TryGetValue(SpanAttributeConstants.PreviousTryContextKey, out var previousContext))
{
// This request was instrumented by previous
// ProcessRequest, such is the case with retries or redirect responses where the same request is sent again.

var retryCount = 1;
if (RequestProperties.Instance.TryGetValue(SpanAttributeConstants.RetryCountKey, out var previousRetryCount))
{
retryCount = (int)previousRetryCount + 1;
}

activity = WebRequestActivitySource.StartActivity(ActivityName, ActivityKind.Client, context.ActivityContext, links: new[] { new ActivityLink((ActivityContext)previousContext) });

activity?.SetTag(SpanAttributeConstants.RetryCountKey, retryCount);
RequestProperties.Instance[SpanAttributeConstants.RetryCountKey] = retryCount;
}
else
{
activity = WebRequestActivitySource.StartActivity(ActivityName, ActivityKind.Client);
}

var activity = WebRequestActivitySource.StartActivity(ActivityName, ActivityKind.Client);
var activityContext = Activity.Current?.Context ?? default;
if (activityContext != default)
{
// Store activity context for the next possible try.
RequestProperties.Instance[SpanAttributeConstants.PreviousTryContextKey] = activityContext;
}

// Propagation must still be done in all cases, to allow
// downstream services to continue from parent context, if any.
Expand Down Expand Up @@ -658,17 +626,6 @@ private static Func<object[], T> CreateTypeInstance<T>(ConstructorInfo construct
return (Func<object[], T>)setterMethod.CreateDelegate(typeof(Func<object[], T>));
}

private static class RequestProperties
{
private static readonly AsyncLocal<Dictionary<string, object>> Properties = new AsyncLocal<Dictionary<string, object>>();

public static Dictionary<string, object> Instance
{
get => Properties.Value;
set => Properties.Value = value;
}
}

private class HashtableWrapper : Hashtable, IEnumerable
{
private readonly Hashtable table;
Expand Down
82 changes: 0 additions & 82 deletions test/OpenTelemetry.Instrumentation.Http.Tests/HttpClientTests.Basic.netcore31.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -303,55 +303,6 @@ public async Task HttpClientInstrumentationWorksIfAlreadyInstrumented()
Assert.NotEqual(parentSpanId, activity.ParentSpanId.ToString());
}

/// <summary>
/// Test request connection retry: reflection hook does not throw.
/// </summary>
[Theory]
[InlineData("GET")]
[InlineData("POST")]
public async Task TestSecureTransportRetryFailureRequest(string method)
{
var processor = new Mock<BaseProcessor<Activity>>();

var uriBuilder = new UriBuilder(this.url) { Scheme = "https" };

using (Sdk.CreateTracerProviderBuilder()
.AddHttpClientInstrumentation()
.AddProcessor(processor.Object)
.Build())
{
using (var client = new HttpClient(new SimpleRetryHandler(new HttpClientHandler(), maxRetryCount: 1)))
{
var ex = await Assert.ThrowsAnyAsync<Exception>(() =>
{
return method == "GET"
? client.GetAsync(uriBuilder.Uri)
: client.PostAsync(uriBuilder.Uri, new StringContent("hello world"));
});
Assert.True(ex is InvalidOperationException);
}
}

Assert.Equal(8, processor.Invocations.Count); // SetParentProvider/OnStart/OnEnd/OnStart/OnStart/OnEnd/OnShutdown/Dispose called.
var firstTry = (Activity)processor.Invocations[2].Arguments[0];
var secondTry = (Activity)processor.Invocations[5].Arguments[0];

Assert.Equal(ActivityKind.Client, firstTry.Kind);
Assert.NotEqual(default, firstTry.Context.SpanId);

Assert.Equal(ActivityKind.Client, secondTry.Kind);
Assert.NotEqual(default, secondTry.Context.SpanId);

ActivityLink activityLink = secondTry.Links.FirstOrDefault();
Assert.NotEqual(default, activityLink);
Assert.Equal(firstTry.Context.TraceId, activityLink.Context.TraceId);
Assert.Equal(firstTry.Context.SpanId, activityLink.Context.SpanId);

var retryCount = secondTry.GetTagItem("http.retry_count");
Assert.NotNull(retryCount);
Assert.Equal(1, (int)retryCount);
}

[Fact]
public async void RequestNotCollectedWhenInstrumentationFilterApplied()
{
Expand Down Expand Up @@ -491,39 +442,6 @@ private static void ActivityEnrichment(Activity activity, string method, object
break;
}
}

private sealed class SimpleRetryHandler : DelegatingHandler
{
private readonly int maxRetryCount;

public SimpleRetryHandler(HttpMessageHandler innerHandler, int maxRetryCount = 3)
: base(innerHandler)
{
this.maxRetryCount = maxRetryCount;
}

protected override async Task<HttpResponseMessage> SendAsync(HttpRequestMessage request, CancellationToken cancellationToken)
{
HttpResponseMessage response = null;
for (int i = 0; i < this.maxRetryCount + 1; i++)
{
try
{
response = await base.SendAsync(request, cancellationToken);
if (response.IsSuccessStatusCode)
{
return response;
}
}
catch
{
// ignored
}
}

return response;
}
}
}
}
#endif
9 changes: 1 addition & 8 deletions test/OpenTelemetry.Instrumentation.Http.Tests/HttpWebRequestTests.Basic.netfx.cs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -218,14 +218,7 @@ public async Task HttpWebRequestInstrumentationWorksIfAlreadyInstrumented()
using var c = new HttpClient();
await c.SendAsync(request);

Assert.Equal(3, activityProcessor.Invocations.Count); // SetParentProvider/Begin/End called
var activity = (Activity)activityProcessor.Invocations[2].Arguments[0];

Assert.Equal(ActivityKind.Client, activity.Kind);
Assert.NotEqual(default, activity.Context.SpanId);
Assert.NotEqual(traceId, activity.TraceId.ToString());
Assert.NotEqual(parentSpanId, activity.SpanId.ToString());
Assert.NotEqual(parentSpanId, activity.ParentSpanId.ToString());
Assert.Equal(1, activityProcessor.Invocations.Count);
}

[Fact]
Expand Down
4 changes: 2 additions & 2 deletions .../OpenTelemetry.Instrumentation.Http.Tests/OpenTelemetry.Instrumentation.Http.Tests.csproj
View file
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
<PropertyGroup>
<Description>Unit test project for OpenTelemetry HTTP instrumentations</Description>
<!-- OmniSharp/VS Code requires TargetFrameworks to be in descending order for IntelliSense and analysis. -->
<TargetFrameworks>net6.0;net5.0;netcoreapp3.1</TargetFrameworks>
<TargetFrameworks Condition="$(OS) == 'Windows_NT'">$(TargetFrameworks);net461</TargetFrameworks>
<TargetFrameworks>net6.0;net5.0;netcoreapp3.1;net461</TargetFrameworks>
<!-- <TargetFrameworks Condition="$(OS) == 'Windows_NT'">$(TargetFrameworks);net461</TargetFrameworks>-->
</PropertyGroup>

<ItemGroup>
Expand Down

0 comments on commit 00b6a84

Please sign in to comment.