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.

Sign up for GitHub

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

Jump to bottom

Convert other integrations to MethodBuilder #494

Merged
merged 17 commits into from Aug 30, 2019
Merged

Convert other integrations to MethodBuilder #494

merged 17 commits into from Aug 30, 2019

Conversation

lucaspimentel
Copy link
Member

@lucaspimentel lucaspimentel commented Aug 27, 2019 •
edited

Changes proposed in this pull request:

  • migrate remaining integrations to use newer MethodBuilder to emit dynamic IL
    • ASP.NET MVC
    • ASP.NET Web API
    • WCF
  • refactor ASP.NET Core MVC integration to use MethodBuilder.WithConcreteType(Type) instead of MethodBuilder.WithConcreteTypeName(string)
  • remove unused MethodBuilder.WithConcreteTypeName(string)
  • remove old uses of dynamic keyword

@lucaspimentel lucaspimentel added area:automatic-instrumentation Automatic instrumentation managed C# code (Datadog.Trace.ClrProfiler.Managed) type:refactor labels Aug 27, 2019
@lucaspimentel lucaspimentel added this to the 1.6.3 milestone Aug 27, 2019
@lucaspimentel lucaspimentel requested a review from a team as a code owner August 27, 2019 18:47
@lucaspimentel lucaspimentel self-assigned this Aug 27, 2019
@lucaspimentel lucaspimentel changed the title migrate integrations to newer MethodBuilder` migrate integrations to newer MethodBuilder API Aug 27, 2019
@lucaspimentel lucaspimentel added the status:work-in-progress Actively worked on. If this is a PR, no review needed yet. WIP. label Aug 27, 2019
@lucaspimentel lucaspimentel changed the title migrate integrations to newer MethodBuilder API [WIP] migrate integrations to newer MethodBuilder API Aug 27, 2019
@lucaspimentel lucaspimentel added type:enhancement Improvement to an existing feature and removed type:refactor labels Aug 28, 2019
@lucaspimentel lucaspimentel changed the title [WIP] migrate integrations to newer MethodBuilder API migrate more integrations to newer MethodBuilder Aug 28, 2019
@lucaspimentel lucaspimentel removed the status:work-in-progress Actively worked on. If this is a PR, no review needed yet. WIP. label Aug 28, 2019
colin-higgins
colin-higgins reviewed Aug 29, 2019
View reviewed changes
src/Datadog.Trace.ClrProfiler.Managed/Integrations/AspNet/AspNetCoreMvc2Integration.cs
@@ -22,13 +22,15 @@ public sealed class AspNetCoreMvc2Integration : IDisposable
/// <summary>
/// Type for unobtrusive hooking into Microsoft.AspNetCore.Mvc.Core pipeline.
/// </summary>
private const string DiagnosticSource = "Microsoft.AspNetCore.Mvc.Internal.MvcCoreDiagnosticSourceExtensions";
private const string DiagnosticSourceTypeName = "Microsoft.AspNetCore.Mvc.Internal.MvcCoreDiagnosticSourceExtensions";
Copy link
Member

Choose a reason for hiding this comment

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

I like the TypeName postfix for clarity, I'm going to adopt that.

@lucaspimentel lucaspimentel merged commit b735a4b into master Aug 30, 2019
@lucaspimentel lucaspimentel deleted the lpimentel/use-method-builder-in-all-integrations branch August 30, 2019 14:03
@lucaspimentel lucaspimentel changed the title migrate more integrations to newer MethodBuilder Convert others integrations to MethodBuilder Sep 3, 2019
@lucaspimentel lucaspimentel changed the title Convert others integrations to MethodBuilder Convert other integrations to MethodBuilder Sep 3, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@colin-higgins colin-higgins colin-higgins approved these changes

@zacharycmontoya zacharycmontoya zacharycmontoya approved these changes

Assignees

@lucaspimentel lucaspimentel

Labels
area:automatic-instrumentation Automatic instrumentation managed C# code (Datadog.Trace.ClrProfiler.Managed) type:enhancement Improvement to an existing feature
Projects
None yet
Milestone
1.7.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@lucaspimentel @colin-higgins @zacharycmontoya