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

[Backend] Db Server Stats #32

Closed

Conversation

yj7o5
Copy link

@yj7o5 yj7o5 commented Jun 17, 2021

partially closes #10

@yj7o5
Copy link
Author

yj7o5 commented Jun 17, 2021

@AKlaus created as draft at-the-moment since I haven't fully tested it yet. You can go ahead and provide any remarks if you want.

@AKlaus AKlaus changed the base branch from master to feature/#10_DB_stats June 18, 2021 23:01
@AKlaus
Copy link
Collaborator

AKlaus commented Jun 18, 2021

Hi @yj7o5,
Thank you for submitting a PR. I'll write notes on the files and some here when they're applicable to multiple files.

  1. Need automated tests to cover the new functionality. We aren't aiming 100% coverage but the main goal is to ensure that the feature works in principle and all fragile bits (usually, integration points) a covered by tests
  2. All the new DTOs shouldn't be added to Database.Common project (it's a shared project with common classes between Database and Domain, see the diagram here). The best way would be to add another service to Domain project (e.g. call it ServerStatsService). Put all the DTOs into a subfolder of the service folder.
  3. The preferable way to communicate with the server by using the existing IDocumentStore rather than via HTTP. See this link.

{
[ApiController]
[Route("api/[controller]")]
public class AdminController : ControllerBase
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please name the controller Stats.
There aren't going to be any "admin" features.

/// <summary>
/// Get various database stats such as memory, cpu usage, etc.
/// </summary>
[HttpGet("database-stats")]
Copy link
Collaborator

Choose a reason for hiding this comment

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

When the controller is renamed, then the end-point can be called database


public int CountOfIndexes { get; set; }

public long CountOfDocuments { get; set; }
Copy link
Collaborator

Choose a reason for hiding this comment

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

Is there an option to add SizeOnDisk property as well?

3. Import test data from `/documentation/exported_data.ravendbdump` file ([see the docs](https://ravendb.net/docs/article-page/latest/csharp/studio/database/tasks/import-data/import-data-file))
4. Set the address to the _RavenDB_ server and the DB name in `./back-end/WebApi/appsettings.Development.json`.
4. Set the address to the _RavenDB_ server and the DB name in `./back-end/WebApi/appsettings.Development.json`. The database config follows the structure specified in [DatabaseSettings.cs](./back-end/Database/DataBase.Common/Configuration/DatabaseSettings.cs). As an example:
Copy link
Collaborator

Choose a reason for hiding this comment

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

Let's be concise, no one reads long text.
Suggested wording:

Set the address to the RavenDB server and the DB name in ./back-end/WebApi/appsettings.Development.json (properties Database.RavenDbUrls, RavenDbUrls.DbName).

The rest of the config is not applicable here

services.AddHttpClient<IRavenService, RavenService>((serviceProvider, client) =>
{
var dbSettings = serviceProvider.GetService<DatabaseSettings>();

Copy link
Collaborator

Choose a reason for hiding this comment

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

Would it require authentication on the RavenDB server? (I understand that it's not required on the dev instance)
Can we hijack the current connection? (it would be especially handy for tests). Check out this link.

@yj7o5
Copy link
Author

yj7o5 commented Jun 20, 2021

Thanks @AKlaus will go ahead and perform the changes.

@AKlaus
Copy link
Collaborator

AKlaus commented Jul 27, 2022

Closing it, as it appears abandoned by the submitter (idle for more than a year)

@AKlaus AKlaus closed this Jul 27, 2022
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.

Expose DB Server stats
2 participants