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

Feature: Prometheus Metrics Framework #554

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

Conversation

celestialorb
Copy link
Contributor

Changes proposed in this PR:

This is a revamp of an old PR (#479) of mine. I'm getting back into KSP and would like a server that can export metrics in a way that Prometheus can scrape and capture them, allowing me to easily view trends over time in something like Grafana.

I decided to create a new branch and a new PR to avoid cleaning up the old one and to refresh my own memory as to how this works.

This initial PR just establishes a basic endpoint for Prometheus metrics and only exposes two metrics: one for total online player count as well as one for online status of individual players (if detailed player metrics are enabled).

If this is accepted I will create PRs to add individual groups of metrics to the server.
Looking at what I've done in the past for an old server of mine this would/could include:

  • Space Program Metrics
    • Funds
    • Science
    • Reputation
  • Vessel Metrics
    • Position
    • Orbital Elements
    • Staging
    • Resource Levels
  • Facility Metrics
    • Levels
    • maybe damage?
  • Contract Metrics
    • State of available contracts (i.e. rewards, accepted/offered)
  • Technology Metrics
    • Science cost
    • Number of parts
  • Subspace Metrics
    • Time difference of each subspace from server
  • Kerbal Metrics
    • Attributes (bravery, dumb, etc.)
    • Flight Counts
    • Deaths
    • Recoveries

@celestialorb
Copy link
Contributor Author

Tagging @gavazquez and @DasSkelett as they were involved in the review of the original PR.

@celestialorb
Copy link
Contributor Author

Looks like the Appveyor jobs are failing. I guess the secret needs to be updated?

Copy link
Contributor

@DasSkelett DasSkelett left a comment

Choose a reason for hiding this comment

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

Sorry for the long wait. Thanks for the work, looks very good, two small comments:

Server/Server/Metrics/Player.cs Outdated Show resolved Hide resolved
Server/Web/WebServer.cs Outdated Show resolved Hide resolved
@celestialorb
Copy link
Contributor Author

celestialorb commented Aug 12, 2023

Added the changes recommended @DasSkelett! Performed some basic smoke tests with the changes and everything still works.

@celestialorb
Copy link
Contributor Author

@DasSkelett Shall this be merged in, or would you like to fix the AppVeyor build first?

@celestialorb
Copy link
Contributor Author

@DasSkelett Just a friendly poke on this PR.

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.

2 participants