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

Make dependencyName in SQL dependency tracking more unique #419

Closed
stijnmoreels opened this issue Jun 27, 2022 · 3 comments · Fixed by #460
Closed

Make dependencyName in SQL dependency tracking more unique #419

stijnmoreels opened this issue Jun 27, 2022 · 3 comments · Fixed by #460
Assignees
Labels
application-insights All issues related to Azure Application Insights dependencies All issues related to dependencies enhancement New feature or request good first issue Good for newcomers
Projects
Milestone

Comments

@stijnmoreels
Copy link
Member

Is your feature request related to a problem? Please describe.
In a previous release, we made sure that we could pass-in a pseudo SQL command because it is quite common to interact with multiple SQL tables in a realistic application (JOIN, multiple SELECT...).
Although, this change cased a more general dependencyName for SQL dependency tracking, which messed up the performance overview as all dependency tracking is now under the same databaseName.

Describe the solution you'd like
We should make sure that we again make the SQL dependency tracking more unique. We could do this by adding (for example) the pseudo SQL command to the dependencyName.

<databaseName>/<sqlCommand
Orders/SELECT OrderId FROM Orders

Discovered by @pim-simons .

@stijnmoreels stijnmoreels added enhancement New feature or request good first issue Good for newcomers application-insights All issues related to Azure Application Insights dependencies All issues related to dependencies labels Jun 27, 2022
@stijnmoreels stijnmoreels added this to the v3.0.0 milestone Jun 27, 2022
@stijnmoreels stijnmoreels added this to To do in Roadmap via automation Jun 27, 2022
@pim-simons
Copy link
Contributor

Some more context, pre version 2.5.0 the performance logging looked like this (where red is the databasename and blue the tablename):
image

With version 2.5.0 I noticed this:
image
And changed the logging to the new way of working where we do not specify a table name but include a pseudo SQL command.

Now the performance logging looks like this (where red is the databasename and green the servername):
image

Instead of having a performance overview per table (or per pseudo SQL command) I now have all SQL logging as a single line in the performance logging and we lose some information on the performance of the specific commands.

@fgheysels
Copy link
Member

fgheysels commented Jun 27, 2022

My 2 cents: can we change this so that the pseudo SQL Command is written as a custom dimension on the dependency instead ?
We should then make sure that the dependency name is the databasename + a functional description of the operation.

@stijnmoreels
Copy link
Member Author

My 2 cents: can we change this so that the pseudo SQL Command is written as a custom dimension on the dependency instead ? The dependency name should then be the databasename + a functional description of the operation ?

Yeah, we previously had an operationName but we removed that due to the pseudo SQL command.

@stijnmoreels stijnmoreels modified the milestones: v3.0.0, v2.6.0 Jul 14, 2022
@stijnmoreels stijnmoreels self-assigned this Aug 29, 2022
Roadmap automation moved this from To do to Done Sep 1, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
application-insights All issues related to Azure Application Insights dependencies All issues related to dependencies enhancement New feature or request good first issue Good for newcomers
Projects
Roadmap
  
Done
Development

Successfully merging a pull request may close this issue.

3 participants