-
Notifications
You must be signed in to change notification settings - Fork 643
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
Fix #1970 by adding a perf log for the external search service #1978
Conversation
@@ -33,7 +33,7 @@ | |||
</li> | |||
<li> | |||
<h2> | |||
<a class="action-menu-link" href="@Url.Action(MVC.Admin.Lucene.Index())"> | |||
<a class="action-menu-link" href="@Url.Action("Index", "Lucene")"> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
👍 I approve of this change!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I can spend an hour or so and actually yank T4MVC out if you'd like :).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Filed #1980 for when you have spare cycles.
FYI this will need to be updated after merging #1969 because that adds a second ExternalSearchService API that needs to be traced. Should be a minor fix. |
Fix #1970 by adding a perf log for the external search service
@@ -39,6 +40,14 @@ public virtual void TraceEvent(TraceEventType type, int id, string message, [Cal | |||
_source.TraceEvent(type, id, FormatMessage(message, member, file, line)); | |||
} | |||
|
|||
public void PerfEvent(string name, TimeSpan time, IEnumerable<KeyValuePair<string,object>> payload) | |||
{ | |||
PerfCounters.AddSample(name, 1000, time.TotalMilliseconds); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is the 1000 here? Why 1000? Why hard-coded?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
1000 is the number of samples that will be held in the ring buffer. It is hardcoded right now. We could make it configurable fairly easily but for now I just wanted the quick solution.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Even just a parameter name and/or comment would suffice.
There is also a simple "performance counter" (not a formal Windows one) for easy monitoring
Fixes #1970