-
Notifications
You must be signed in to change notification settings - Fork 730
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
Instrumentation Adapters to support Activity API #701
Changes from 1 commit
831d712
737f9b6
9b840f0
036a9bb
5b836a4
382d85f
f1e5108
170ad87
0832e71
3b4cbb7
19d138f
121e4b5
823fee6
c759d8a
6df93de
0d07263
692b0dc
7185426
37071c5
b889a8c
8630782
c4834ae
581cd1e
7341c59
d9c9fff
514d89c
f939361
a220d9e
99b91bf
63dc16f
11a175a
efdb1d3
3646fa1
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -22,6 +22,7 @@ | |
using Microsoft.AspNetCore.Http.Features; | ||
using OpenTelemetry.Context.Propagation; | ||
using OpenTelemetry.Trace; | ||
using OpenTelemetry.Trace.Samplers; | ||
|
||
namespace OpenTelemetry.Instrumentation.AspNetCore.Implementation | ||
{ | ||
|
@@ -35,6 +36,7 @@ internal class HttpInListener : ListenerHandler | |
private readonly PropertyFetcher beforeActionTemplateFetcher = new PropertyFetcher("Template"); | ||
private readonly bool hostingSupportsW3C = false; | ||
private readonly AspNetCoreInstrumentationOptions options; | ||
private readonly ActivitySampler sampler = new AlwaysOnActivitySampler(); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. hardcoded sampler for just prototyping. |
||
|
||
public HttpInListener(string name, Tracer tracer, AspNetCoreInstrumentationOptions options) | ||
: base(name, tracer) | ||
|
@@ -60,10 +62,21 @@ public override void OnStartActivity(Activity activity, object payload) | |
return; | ||
} | ||
|
||
// TODO: the line below once .NET ships new Activity | ||
// Or do reflection now. | ||
// activity.ActivityKind = ActivityKind.Server | ||
|
||
var request = context.Request; | ||
|
||
// see the spec https://github.com/open-telemetry/opentelemetry-specification/blob/master/specification/data-semantic-conventions.md | ||
var path = (request.PathBase.HasValue || request.Path.HasValue) ? (request.PathBase + request.Path).ToString() : "/"; | ||
if (activity != null) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
is it possible the Activity can be null? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. No. I didnt need the activity null check. |
||
{ | ||
activity.DisplayName = path; | ||
} | ||
|
||
var samplingDecision = this.sampler.ShouldSample(activity.Context, activity.TraceId, default, path, activity.Kind, activity.Tags, activity.Links); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This is the part which makes me not 100% convinved this approach is right. All instrumentationadatpers now need to know about Samplers (which is SDK concept). Instead of simply checking if activity.IsAllDataRequested, its now first setting IsAllDataRequested by running sampling algorithm |
||
activity.IsAllDataRequested = samplingDecision.IsSampled; | ||
|
||
TelemetrySpan span; | ||
if (this.hostingSupportsW3C && this.options.TextFormat is TraceContextFormat) | ||
|
@@ -90,6 +103,25 @@ public override void OnStartActivity(Activity activity, object payload) | |
span.PutHttpUserAgentAttribute(userAgent); | ||
span.PutHttpRawUrlAttribute(GetUri(request)); | ||
} | ||
|
||
if (activity.IsAllDataRequested) | ||
{ | ||
if (request.Host.Port == 80 || request.Host.Port == 443) | ||
{ | ||
activity.AddTag("http.host", request.Host.Host); | ||
} | ||
else | ||
{ | ||
activity.AddTag("http.host", request.Host.Host + ":" + request.Host.Port); | ||
} | ||
|
||
activity.AddTag("http.method", request.Method); | ||
activity.AddTag("http.path", path); | ||
|
||
var userAgent = request.Headers["User-Agent"].FirstOrDefault(); | ||
activity.AddTag("http.user_agent", userAgent); | ||
activity.AddTag("http.url", GetUri(request)); | ||
} | ||
} | ||
|
||
public override void OnStopActivity(Activity activity, object payload) | ||
|
@@ -114,6 +146,11 @@ public override void OnStopActivity(Activity activity, object payload) | |
var response = context.Response; | ||
|
||
span.PutHttpStatusCode(response.StatusCode, response.HttpContext.Features.Get<IHttpResponseFeature>().ReasonPhrase); | ||
|
||
if (activity.IsAllDataRequested) | ||
{ | ||
activity.AddTag("http.status_code", response.StatusCode.ToString()); | ||
} | ||
} | ||
|
||
span.End(); | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I will create the Activity exporter for Zipkin this week or the next.