🐛 dotnet-counters not working with Orleans

[tomachristian]

Using dotnet-counters with a process hosting an Orleans Silo (v8.2.0) does not work.

We get a blank screen with no statistics:

> dotnet-counters monitor -p PID

I noticed that commenting out (almost all of) our Program.cs file brought those back. Reenabling things one by one I got to this line of code in Orleans' EnvironmentStatisticsProvider:

Then I remembered seeing that multiple listeners of "counters" must be using the same interval (unfortunately I no longer have a source for this), so I changed the interval (via Debugging) in the Olreans' EnvironmentStatisticsProvider to be "1". That made dotnet-counters work again.

The other way around with setting dotnet-counters --refresh-interval to 0.5 does not work.. probably a limitation of the CLI as it only supports integers.

In my opinnion we should provide advice for this in the Orleans documentation, or change the scan interval in the Orleans code base to be configurable or back to the default of "1" sec. Actually I think it HAS to be changed in Orleans to "1" because event dotnet-monitor has a minimum of https://github.com/dotnet/dotnet-monitor/blob/51a814d9d55738e5a9232a51088b973bd368bdde/src/Microsoft.Diagnostics.Monitoring.Options/GlobalCounterOptions.cs#L15, but I am not very familiar with these things... so I think someone that knows what they are doing should have a look at this.

The change in Orleans was introduced with #8820 (cc @ledjon-behluli)

[ntovas]

Nice finding @tomachristian,
The obvious solution is to change the value in EventCounterListener to 1 or make it configurable, but in my opinion using EventListeners and messing with these metrics is a bit too much for a framework like this and for a functionality that cannot be disabled.
Possibly changing it to use PerformanceCounter, expose some settings to the user, or even give the user the ability to implement the IEnvironmentStatisticsProvider the way they like seems like a better idea to me.

What is your opinion @ReubenBond?

[ReubenBond]

Let's change it to 1. I am out on paternity leave, but if you can open a PR, I'll merge it.

[ledjon-behluli]

@ntovas context: some high-impact fatures are depended on the environment stats, and it was also quite a bit of effort to make these cross-plat to switch to PCs or make it optional. Not only the new features but also generally people expecting silo-stats gossip. Changing it to 1 should be the way to go. Even making it configurable may have side effects i.e. users setting these to high(er) values.

[ntovas]

Hi @ledjon-behluli,

As far as I looked the code, it seems to me that the user can actually replace the EnvironmentStatisticsProvider with their own implementation without much trouble since the IEnvironmentStatisticsProvider is public and can be easily implemented and replaced in the DI. For me this combined with the change to 1 sec is more than enough.