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

WIP - InteractionProfilerGrain #345

Open
wants to merge 11 commits into
base: master
Choose a base branch
from

Conversation

KSemenenko
Copy link

First step for #344

Filter + Grain

namespace OrleansDashboard.Model
{
[Serializable]
public class GrainInteractionInfoEntry
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we start to use records for that?


namespace OrleansDashboard.Implementation
{
public sealed class GrainInteractionFilter : IIncomingGrainCallFilter, IOutgoingGrainCallFilter
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we not extend the current filter for that? Because we have some logic there that we should keep. Like custom method names and attributes to opt out from the profiler.

var info = stack.Pop();

grainInteractionProfiler ??= grainFactory.GetGrain<IInteractionProfiler>(0);
await grainInteractionProfiler.Track(info);
Copy link
Contributor

Choose a reason for hiding this comment

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

In the current profiler we use batching to reduce the grain calls. I think we should use the same approach here.

Copy link
Contributor

@SebastianStehle SebastianStehle left a comment

Choose a reason for hiding this comment

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

Hi, I added a few comments.


private string BuildGraph()
{
var content = string.Join("\n ", interaction.Values.SelectMany(s => s)
Copy link
Member

Choose a reason for hiding this comment

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

I think I would prefer not to concern the back end with front end configuration, such as colours. Could we just return pure data as JSON to the front end?

@@ -26,6 +26,8 @@ public static class ServiceCollectionExtensions
{
builder.ConfigureApplicationParts(parts => parts.AddDashboardParts());
builder.ConfigureServices(services => services.AddDashboard(configurator));
builder.AddIncomingGrainCallFilter<GrainInteractionFilter>();
Copy link
Member

Choose a reason for hiding this comment

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

I think I would like a way to control (switch on/off) this profiling, as it may get slow?

@richorama richorama changed the title InteractionProfilerGrain WIP - InteractionProfilerGrain Feb 21, 2022
@SebastianStehle
Copy link
Contributor

Can we close this?

@KSemenenko
Copy link
Author

I can't figure out how to do it well to store the history of calls

@SebastianStehle
Copy link
Contributor

Why do you want to store the history? The Dashboard is only designed to show either the present or data over the full lifetime of the silo. If you can provide a view that shows the interaction over the full lifetime it would provide value.

For detailed analysis you should use telemetry anyway.

@KSemenenko
Copy link
Author

I'd like to show who's calling whom.
For debugging, so to speak.

I hadn't thought of this as a long term storage.

my case is to look for deadlocks or delays.
Sometimes there are too many grain calls to each other and I would like to see this call tree.

@KSemenenko
Copy link
Author

Here, most likely, by storage, I mean how to collect statistics and pass them to the UI @SebastianStehle

@SebastianStehle
Copy link
Contributor

I think we cannot analyze single calls. This would be responsibility of new relic or application insights. We can only collect general call structure.

So basically the following table:

SourceGrain | SourceMethod (if possible) | TargetGrain | TargetMethod | Count | CountSuccess | CountFailed | TotalTimeNs

So basically the following dictionary

record ProfilerKey (
  string SourceGrain,
   string SourceMethod,
   string TargetGrain,
   string TargetMethod);
   
record ProfileValue (
  long CountSuccess,
  long CountFailed,
   long TotalTimeNs); // Could store 9 years of compute time or 9000 years for ms)
   
Dictionary<ProfilerKey, ProfileValue>

@KSemenenko
Copy link
Author

I found an interesting side effect
when using these filters, if requests were between different silo instances, the request will not be executed due to an error because there is no context

@KSemenenko
Copy link
Author

Seems I found Idea for it

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants