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

Infrastructure: Trace when query is done. This is needed in order to get SelectRows statistics. #7105

Closed
fitzchak opened this issue Nov 23, 2016 · 10 comments
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Milestone

Comments

@fitzchak
Copy link

In order to get the SelectRows from SqlStatistics we need to add this trace statement, since "System.Data.SqlClient.WriteCommandAfter" tracing is not enough, as this is before the reader is reading the results.
This let us in Entity Framework Profiler report the results for the query.

@fitzchak
Copy link
Author

cc @anpete

@rowanmiller rowanmiller added this to the 1.2.0 milestone Nov 28, 2016
fitzchak pushed a commit to fitzchak/EntityFramework that referenced this issue Nov 29, 2016
…SelectRows statistics. Take dotnet#2: Use the DiagnosticSource on RelationalDataReader.
fitzchak pushed a commit to fitzchak/EntityFramework that referenced this issue Nov 30, 2016
…SelectRows statistics. Take dotnet#2: Use the DiagnosticSource on RelationalDataReader. Take dotnet#3: Fix requested changes.
@fitzchak
Copy link
Author

Can this get into next release of v1?

@ajcvickers
Copy link
Member

Marking for re-triage to consider putting it in patch release.

@ajcvickers ajcvickers reopened this Mar 27, 2017
@ajcvickers ajcvickers removed this from the 2.0.0 milestone Mar 27, 2017
@ajcvickers
Copy link
Member

@fitzchak We discussed this but unfortunately we don't think it meets the bar for a patch release, especially because it contains a binary-compat breaking change. Is there any way you can work around the issue you have without taking the change?

@fitzchak
Copy link
Author

fitzchak commented Apr 2, 2017

@ajcvickers We cannot work around this.
What is the binary compat breaking change? Isn't it just adding more logs?

@ajcvickers
Copy link
Member

@fitzchak The PR changes the signature for the RelationalDataReader constructor. We might be able to make the change in a non-breaking way, but to justify it going into a patch release we will need some details as to exactly what is not working so we can get approval.

@ajcvickers
Copy link
Member

EF Team Triage: Closing this issue as the requested additional details have not been provided and we have been unable to reproduce it.

BTW this is a canned response and may have info or details that do not directly apply to this particular issue. While we'd like to spend the time to uniquely address every incoming issue, we get a lot traffic on the EF projects and that is not practical. To ensure we maximize the time we have to work on fixing bugs, implementing new features, etc. we use canned responses for common triage decisions.

@ajcvickers ajcvickers added this to the 2.0.0 milestone Apr 12, 2017
@ajcvickers ajcvickers added the closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. label Apr 12, 2017
@ajcvickers ajcvickers changed the title Trace when query is done. This is needed in order to get SelectRows statistics. Infrastructure: Trace when query is done. This is needed in order to get SelectRows statistics. May 9, 2017
@fitzchak
Copy link
Author

fitzchak commented May 22, 2017

Hi,
We need this in order to show Row Count in EntityFrameworkProfiler: http://issues.hibernatingrhinos.com/issue/UberProf-365
Can you re-open?

@divega
Copy link
Contributor

divega commented May 22, 2017

@fitzchak to be clear, your PR made it into 2.0.0-preview1. It just did not make into a patch for the 1.x versions (that is what the last few comments are about). Are you getting many customer requests for this functionality in 1.x versions?

@fitzchak
Copy link
Author

@divega Hi, yes. We have many customer requests to use the profiler with the current version of EntityFrameworkCore. The profiler is working well, but do not show the row count and duration for the statements (see attachment image) and we get many requests to fix this. This PR should fix this.

image

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
closed-fixed The issue has been fixed and is/will be included in the release indicated by the issue milestone. type-enhancement
Projects
None yet
Development

No branches or pull requests

5 participants