Skip to content
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

AspNet.TelemetryHttpModule public api + test fixes + renames #2240

Conversation

CodeBlanch
Copy link
Member

@CodeBlanch CodeBlanch commented Aug 7, 2021

Part of #2222.

Changes

  • Added public api files
  • Fixed up unit tests for renames
  • Cleaned up VS IDE suggestions
  • Renamed DiagnosticListener

@CodeBlanch CodeBlanch requested a review from a team August 7, 2021 05:56
@CodeBlanch CodeBlanch changed the title AspNet.TelemetryHttpModule public api + test fixes AspNet.TelemetryHttpModule public api + test fixes + renames Aug 9, 2021
@@ -30,8 +30,7 @@ Telemetry correlation http module enables cross tier telemetry tracking.

public void OnNext(DiagnosticListener listener)
{
if (listener.Name == "Microsoft.AspNet.TelemetryCorrelation" ||
listener.Name == "System.Net.Http" )
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Was this used previously?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think so. My guess is the example snippet came from some code listening to this module and something else instrumenting outgoing requests?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Roger that, thanks!

@CodeBlanch
Copy link
Member Author

@reyang @cijothomas @alanwest Everyone OK renaming the diagnostic source from "Microsoft.AspNet.TelemetryCorrelation" -> "OpenTelemetry.Instrumentation.AspNet.Telemetry"? At the moment, it is backwards compatible and could use the same name. But I'm thinking we'll add more features to it so it might be nice to break with the past and not be tied to the existing contract(s).

@cijothomas cijothomas merged commit a8f5491 into open-telemetry:aspnet-telemetrycorrelation Aug 10, 2021
@CodeBlanch CodeBlanch deleted the aspnet-telemetrycorrelation-publicapi branch August 11, 2021 16:48
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants