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

ADO.NET integration #248

Merged
merged 12 commits into from
Jan 17, 2019
Merged

ADO.NET integration #248

merged 12 commits into from
Jan 17, 2019

Conversation

lucaspimentel
Copy link
Member

@lucaspimentel lucaspimentel commented Jan 15, 2019

Add a new integration that should cover all ADO.NET providers.

Tested with:

  • SQL Server, System.Data.SqlClient (replaces current integration)
  • Postgres, Npgsql

Other ADO.NET providers (not tested yet):

  • MySQL
  • Oracle
  • SQLite
  • SQL Server Compact, System.Data.SqlServerCe
  • OLE DB
  • ODBC
  • and more!

@lucaspimentel lucaspimentel added type:new-feature area:automatic-instrumentation Automatic instrumentation managed C# code (Datadog.Trace.ClrProfiler.Managed) labels Jan 15, 2019
@lucaspimentel lucaspimentel added this to the 0.7.0-beta milestone Jan 15, 2019
@lucaspimentel lucaspimentel self-assigned this Jan 15, 2019
@lucaspimentel lucaspimentel added the status:work-in-progress Actively worked on. If this is a PR, no review needed yet. WIP. label Jan 15, 2019
@lucaspimentel lucaspimentel force-pushed the lpimentel/adonet-integration branch from a4127e8 to b382e01 Compare January 15, 2019 23:07
@lucaspimentel lucaspimentel removed the status:work-in-progress Actively worked on. If this is a PR, no review needed yet. WIP. label Jan 15, 2019
@@ -30,8 +30,8 @@ public void SubmitsTraces()
Assert.True(spans.Count > 0, "expected at least one span");
foreach (var span in spans)
{
Assert.Equal(SqlServerIntegration.OperationName, span.Name);
Assert.Equal($"Samples.SqlServer-{SqlServerIntegration.ServiceName}", span.Service);
Assert.Equal("sql-server.query", span.Name);
Copy link
Contributor

Choose a reason for hiding this comment

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

Why do we not also have integration tests for postgres?

Copy link
Member Author

Choose a reason for hiding this comment

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

The short answer is "time." Postgres support is time-sensitive. I tested Postgres manually and the SQL Server tests cover ADO.NET as well. I'm adding more automated integration tests for different ADO.NET providers in a separate PR.

Copy link
Contributor

@bmermet bmermet left a comment

Choose a reason for hiding this comment

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

LGTM 👍

@lucaspimentel lucaspimentel merged commit 426bfca into develop Jan 17, 2019
@lucaspimentel lucaspimentel deleted the lpimentel/adonet-integration branch January 17, 2019 15:32
@hemant-learningforever
Copy link

@lucaspimentel Could you please explain how are you getting this signature for wrapper type?

@lucaspimentel
Copy link
Member Author

lucaspimentel commented May 10, 2019

@hemant-learningforever: The work on this PR is finished and merged. Please ask questions by creating a new issue.

EDIT: never mind, I just saw the issue you already created.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area:automatic-instrumentation Automatic instrumentation managed C# code (Datadog.Trace.ClrProfiler.Managed) type:new-feature
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants