-
Notifications
You must be signed in to change notification settings - Fork 764
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
Remove ActivitySourceAdapter #1836
Changes from 27 commits
febda2d
1c8e418
0f21319
999ca2b
6cd0ced
9291faa
c696f15
808a25b
2a3fac7
6d71638
1f32aa6
0dc0772
1970d6f
7f0b2f8
f24b04d
bc32f82
0869911
04fe5e0
1685599
90df48a
4c5df69
5fe89c0
0e337f7
ce2173c
d499a4d
32824d0
c9deb90
a8509ba
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 |
---|---|---|
|
@@ -29,11 +29,10 @@ internal class AspNetCoreInstrumentation : IDisposable | |
/// <summary> | ||
/// Initializes a new instance of the <see cref="AspNetCoreInstrumentation"/> class. | ||
/// </summary> | ||
/// <param name="activitySource">ActivitySource adapter instance.</param> | ||
/// <param name="options">Configuration options for ASP.NET Core instrumentation.</param> | ||
public AspNetCoreInstrumentation(ActivitySourceAdapter activitySource, AspNetCoreInstrumentationOptions options) | ||
public AspNetCoreInstrumentation(AspNetCoreInstrumentationOptions options) | ||
{ | ||
this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(new HttpInListener("Microsoft.AspNetCore", options, activitySource), null); | ||
this.diagnosticSourceSubscriber = new DiagnosticSourceSubscriber(new HttpInListener("Microsoft.AspNetCore", options), 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. there are lot of special names - lets move them to a common place where it can be commented and probbaly linked to aspnetcore repo source code. |
||
this.diagnosticSourceSubscriber.Subscribe(); | ||
} | ||
|
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,6 +16,7 @@ | |
|
||
using System; | ||
using OpenTelemetry.Instrumentation.GrpcNetClient; | ||
using OpenTelemetry.Instrumentation.GrpcNetClient.Implementation; | ||
|
||
namespace OpenTelemetry.Trace | ||
{ | ||
|
@@ -43,7 +44,10 @@ public static TracerProviderBuilder AddGrpcClientInstrumentation( | |
var grpcOptions = new GrpcClientInstrumentationOptions(); | ||
configure?.Invoke(grpcOptions); | ||
|
||
builder.AddDiagnosticSourceInstrumentation((activitySource) => new GrpcClientInstrumentation(activitySource, grpcOptions)); | ||
builder.AddInstrumentation(() => new GrpcClientInstrumentation(grpcOptions)); | ||
builder.AddSource(GrpcClientDiagnosticListener.ActivitySourceName); | ||
builder.AddLegacyActivity("Grpc.Net.Client.GrpcOut"); | ||
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. nit: create as a const, just like aspnetcore |
||
|
||
return builder; | ||
} | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,9 +16,7 @@ | |
|
||
using System; | ||
using OpenTelemetry.Instrumentation.Http; | ||
#if NETFRAMEWORK | ||
using OpenTelemetry.Instrumentation.Http.Implementation; | ||
#endif | ||
|
||
namespace OpenTelemetry.Trace | ||
{ | ||
|
@@ -60,7 +58,9 @@ public static TracerProviderBuilder AddHttpClientInstrumentation( | |
|
||
configureHttpClientInstrumentationOptions?.Invoke(httpClientOptions); | ||
|
||
builder.AddDiagnosticSourceInstrumentation((activitySource) => new HttpClientInstrumentation(activitySource, httpClientOptions)); | ||
builder.AddInstrumentation(() => new HttpClientInstrumentation(httpClientOptions)); | ||
builder.AddSource(HttpHandlerDiagnosticListener.ActivitySourceName); | ||
builder.AddLegacyActivity("System.Net.Http.HttpRequestOut"); | ||
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. nit: create const |
||
|
||
#if NETFRAMEWORK | ||
builder.AddHttpWebRequestInstrumentation(configureHttpWebRequestInstrumentationOptions); | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
static OpenTelemetry.Trace.TracerProviderExtensions.ForceFlush(this OpenTelemetry.Trace.TracerProvider provider, int timeoutMilliseconds = -1) -> bool | ||
static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddLegacyActivity(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder, string operationName) -> OpenTelemetry.Trace.TracerProviderBuilder | ||
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. Not a big fan of this name. How about 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. I am okay with 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. we have opportunity to rename this until 1.1 stable is out, so i'd prefer to not block this pr. 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. Fine by me, I'll stamp as approved. |
||
static OpenTelemetry.Trace.TracerProviderExtensions.ForceFlush(this OpenTelemetry.Trace.TracerProvider provider, int timeoutMilliseconds = -1) -> bool |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddLegacyActivity(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder, string operationName) -> OpenTelemetry.Trace.TracerProviderBuilder | ||
static OpenTelemetry.Trace.TracerProviderExtensions.ForceFlush(this OpenTelemetry.Trace.TracerProvider provider, int timeoutMilliseconds = -1) -> bool |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddLegacyActivity(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder, string operationName) -> OpenTelemetry.Trace.TracerProviderBuilder | ||
static OpenTelemetry.Trace.TracerProviderExtensions.ForceFlush(this OpenTelemetry.Trace.TracerProvider provider, int timeoutMilliseconds = -1) -> bool |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1 +1,2 @@ | ||
static OpenTelemetry.Trace.TracerProviderBuilderExtensions.AddLegacyActivity(this OpenTelemetry.Trace.TracerProviderBuilder tracerProviderBuilder, string operationName) -> OpenTelemetry.Trace.TracerProviderBuilder | ||
static OpenTelemetry.Trace.TracerProviderExtensions.ForceFlush(this OpenTelemetry.Trace.TracerProvider provider, int timeoutMilliseconds = -1) -> bool |
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 initially thought (inccoreclty) this is a breaking change! The public api analyzer is a gift!
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.
Should we change this to internal?