Cosmos: Add diagnostic events with statistics #25431
Conversation
| ExecutingSqlQuery = CoreEventId.ProviderBaseId + 100, | ||
| ExecutingReadItem | ||
| ExecutingReadItem, |
There was a problem hiding this comment.
A bit weird that SqlQuery has only an Executing event, ReadItem has both Executing and Executed, and the others only Executed. Should have a Executing and Executed for all of them, or is there a good reason it's this way?
There was a problem hiding this comment.
SqlQuery has both Executing and Executed. The difference is that ExecutedReadNext can be called multiple times per query if it was split up (paging).
ExecutingReadItem is there because it would be breaking to remove it, I don't think there's value in adding Executing for any of the other ones.
| } | ||
| } | ||
|
|
||
| private static string ExecutedReadNext(EventDefinitionBase definition, EventData payload) |
There was a problem hiding this comment.
Can move this inside ExecutedReadNext above as a local method (other methods too)
There was a problem hiding this comment.
Yes, but I'm sticking to the pattern. If we want to refactor this we should do it everywhere.
AndriySvyryd
force-pushed
the
Issue17298
branch
from
c74f3c1
to
524dcf1
Compare
Fixes #17298