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

Update Resource utilization health checks doc page #42626

Conversation

evgenyfedorov2
Copy link
Contributor

@evgenyfedorov2 evgenyfedorov2 commented Sep 16, 2024

Resource utilization health checks does not require registering Resource Monitoring any more. The issue was dotnet/extensions#4560 and resolved by dotnet/extensions#5413. Now, updating the docs accordingly.


Internal previews

📄 File 🔗 Preview link
docs/core/diagnostics/diagnostic-health-checks.md docs/core/diagnostics/diagnostic-health-checks

@evgenyfedorov2
Copy link
Contributor Author

@gewarren please review the PR.
OPS status check is failing but I don't think it is related to my changes

@RussKie
Copy link
Member

RussKie commented Sep 16, 2024

@evgenyfedorov2 I see the deleted parts of the docs but no additions? Should the docs at least talk about the erroneous scenario as you described in dotnet/extensions#5413?

@evgenyfedorov2
Copy link
Contributor Author

@evgenyfedorov2 I see the deleted parts of the docs but no additions? Should the docs at least talk about the erroneous scenario as you described in dotnet/extensions#5413?

That's because I found that the registration of custom IResourceMonitor implementations is not documented at all, that's why there is no place where to add the erroneous scenario.

@IEvangelist are you aware of any specific reason why it is not documented how to add custom implementations of IResourceMonitor, e.g. .AddSingleton<IResourceMonitor, MyResourceMonitor>()? Adding such a custom implementation would be another workaround preventing the issue dotnet/extensions#4560 from happening, but even if there was not that issue, it might be worth documenting it in general. I mean, default stuff registered by AddResourceMonitoring() might not be enough for some customers and they might want to register their own, but it is not documented at all. What do you think?

@gewarren gewarren closed this Oct 4, 2024
@gewarren gewarren reopened this Oct 4, 2024
@gewarren gewarren requested a review from IEvangelist October 4, 2024 17:24
@evgenyfedorov2
Copy link
Contributor Author

To unblock myself, I created a new issue here #42853

@evgenyfedorov2 I see the deleted parts of the docs but no additions? Should the docs at least talk about the erroneous scenario as you described in dotnet/extensions#5413?

That's because I found that the registration of custom IResourceMonitor implementations is not documented at all, that's why there is no place where to add the erroneous scenario.

@IEvangelist are you aware of any specific reason why it is not documented how to add custom implementations of IResourceMonitor, e.g. .AddSingleton<IResourceMonitor, MyResourceMonitor>()? Adding such a custom implementation would be another workaround preventing the issue dotnet/extensions#4560 from happening, but even if there was not that issue, it might be worth documenting it in general. I mean, default stuff registered by AddResourceMonitoring() might not be enough for some customers and they might want to register their own, but it is not documented at all. What do you think?

@evgenyfedorov2 evgenyfedorov2 merged commit a60aa02 into dotnet:main Oct 5, 2024
16 of 17 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants