SSmetrics - Elasticsearch powered metrics viewing #16549
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
AffectedArc07
commented
Aug 15, 2021
AffectedArc07
commented
Aug 15, 2021
AffectedArc07
commented
Aug 15, 2021
| @@ -32,7 +32,9 @@ GLOBAL_DATUM_INIT(configuration, /datum/server_configuration, new()) | |||
| var/datum/configuration_section/logging_configuration/logging | |||
| /// Holder for the MC configuration datum | |||
| var/datum/configuration_section/mc_configuration/mc | |||
| /// Holder for the MC configuration datum | |||
There was a problem hiding this comment.
This is a typo fix
|
PR is now /mergeable/ per say, just could do with extra input from Fox for specific metrics from each SS. |
|
This PR will be long term TM'd to gather metrics for dashboards early on. |
Kyep
approved these changes
Aug 15, 2021
|
Spoke with fox and implemented metrics on subsystems where getting them would be useful. Also fixed an issue with |
SabreML
reviewed
Aug 16, 2021
| @@ -11,6 +11,11 @@ SUBSYSTEM_DEF(acid) | |||
| /datum/controller/subsystem/acid/stat_entry() | |||
| ..("P:[processing.len]") | |||
|
|
|||
| /datum/controller/subsystem/acid/get_metrics() | |||
| . = ..() | |||
| var/list/cust = list() | |||
There was a problem hiding this comment.
These being cust rather than just custom or something else entirely seems a bit odd. It's pretty obvious that it does mean "custom" of course, but it could probably be a bit clearer.
There was a problem hiding this comment.
It's for easier differentiation between the list index and the entry itself. I also recall something having a custom var and I didn't want these to interfere.
|
Latest commit fixes this
|
|
|
Fox-McCloud
reviewed
Aug 17, 2021
| @@ -2,6 +2,8 @@ | |||
| /datum/controller/subsystem | |||
| // Metadata; you should define these. | |||
| name = "fire codertrain" //name of the subsystem | |||
| /// Subsystem ID. Used for when we need a technical name for the SS | |||
| var/ss_id = "fire_codertrain_again" | |||
Fox-McCloud
reviewed
Aug 17, 2021
Fox-McCloud
reviewed
Aug 17, 2021
| var/list/out = list() | ||
| out["@timestamp"] = time_stamp() // This is required by ElasticSearch, complete with this name. DO NOT REMOVE THIS. | ||
| out["cpu"] = world.cpu | ||
| // out["maptick"] = world.map_cpu // TODO: 514 |
3 tasks
dwasint
added a commit
to Vanderlin-Tales-Of-Wine/Vanderlin
that referenced
this pull request
Feb 15, 2025
<!-- Write **BELOW** The Headers and **ABOVE** The comments else it may not be viewable. --> <!-- Note: PRs including balance changes authored by anyone other than maintainers and official devs will not be considered. --> ## About The Pull Request Lets us upload data to elasticsearch without ripping up log files <!-- Describe The Pull Request. Please be sure every change is documented or this can delay review or prevent the PR from being merged! --> ## Why It's Good For The Game <!-- Please add a short description of why you think these changes would benefit the game. If you can't justify it in words, it might not be worth adding. --> The idea for this came from: ParadiseSS13/Paradise#16549 where instead of metrics it was normalized into just an elastic subsystem which can be used for metrics later ## Pre-Merge Checklist <!-- Don't bother filling these in while creating your Pull Request, just click the checkboxes after the Pull Request is opened and you are redirected to the page. --> - [ ] You tested this on a local server. - [ ] This code did not runtime during testing. - [ ] You documented all of your changes. <!-- Neither the compiler nor workflow checks are perfect at detecting runtimes and errors. It is important to test your code/feature/fix locally. -->
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What Does This PR Do
This PR adds a subsystem in, SSmetrics, which allows ingame metrics to be ingested straight into ElasticSearch. This allows us to easily visualise server performance and trends on pretty graphs, as well as correlate it with things such as player count. This allows us to gain a lot of insight on where the server dies more and gives us historical data.
This will POST the results directly into ElasticSearch, meaning they dont have to be written to a file for them to be read in, it can all happen through a HTTP post.
Todo:
Why It's Good For The Game
Being able to visaulise server metrics over time is incredibly useful and will allow us to further optimise things.
Changelog
🆑 AffectedArc07
add: Added a subsystem which can send server metrics straight into ElasticSearch.
/:cl: