dsmiley commented on code in PR #3509:
URL: https://github.com/apache/solr/pull/3509#discussion_r2364671625
##########
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##########
@@ -1000,6 +1002,26 @@ private void loadInternal() {
// initialize gauges for reporting the number of cores and disk total/free
+ Attributes containerAttrs =
+ Attributes.builder().put(CATEGORY_ATTR,
SolrInfoBean.Category.CONTAINER.toString()).build();
+
+ solrMetricsContext.observableLongGauge(
+ "solr_cores_loaded",
Review Comment:
I forget... are we namespacing all metrics under major areas (as we are
doing for system props / env) lately? If so, a name like "solr_node_cores_
..." would be better. If not... ok.
##########
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##########
@@ -1018,8 +1040,52 @@ private void loadInternal() {
"unloaded",
SolrInfoBean.Category.CONTAINER.toString(),
"cores");
+
Path dataHome =
cfg.getSolrDataHome() != null ? cfg.getSolrDataHome() :
cfg.getCoreRootDirectory();
+
+ solrMetricsContext.observableLongGauge(
+ "solr_cores_filesystem_disk_space",
+ "Disk metrics for Solr's data home directory",
Review Comment:
The description should reflect what we're using. Maybe it could also have
the path?
An aside... I kind of question wether Solr needs a "data home"
differentiated from a "core root directory" but should a debate is out of scope.
##########
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##########
@@ -1000,6 +1002,26 @@ private void loadInternal() {
// initialize gauges for reporting the number of cores and disk total/free
+ Attributes containerAttrs =
Review Comment:
Can you please move metric initialization of SolrCores out of CoreContainer
and into SolrCores.initializeMetrics where it belongs?
##########
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##########
@@ -1018,8 +1040,52 @@ private void loadInternal() {
"unloaded",
SolrInfoBean.Category.CONTAINER.toString(),
"cores");
+
Path dataHome =
cfg.getSolrDataHome() != null ? cfg.getSolrDataHome() :
cfg.getCoreRootDirectory();
+
+ solrMetricsContext.observableLongGauge(
+ "solr_cores_filesystem_disk_space",
Review Comment:
I suggest calling this "solr_disk_space", (maybe "node" namespaced too).
Practically speaking, I think this is the only disk space metric that
matters.
##########
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##########
@@ -1018,8 +1040,52 @@ private void loadInternal() {
"unloaded",
SolrInfoBean.Category.CONTAINER.toString(),
"cores");
+
Path dataHome =
cfg.getSolrDataHome() != null ? cfg.getSolrDataHome() :
cfg.getCoreRootDirectory();
+
+ solrMetricsContext.observableLongGauge(
+ "solr_cores_filesystem_disk_space",
+ "Disk metrics for Solr's data home directory",
+ measurement -> {
+ try {
+ measurement.record(
+ Files.getFileStore(dataHome).getTotalSpace(),
Review Comment:
fileStore can be extracted so its used once
##########
solr/core/src/java/org/apache/solr/core/CoreContainer.java:
##########
@@ -1018,8 +1040,52 @@ private void loadInternal() {
"unloaded",
SolrInfoBean.Category.CONTAINER.toString(),
"cores");
+
Path dataHome =
cfg.getSolrDataHome() != null ? cfg.getSolrDataHome() :
cfg.getCoreRootDirectory();
+
+ solrMetricsContext.observableLongGauge(
+ "solr_cores_filesystem_disk_space",
+ "Disk metrics for Solr's data home directory",
+ measurement -> {
+ try {
+ measurement.record(
+ Files.getFileStore(dataHome).getTotalSpace(),
+ containerAttrs.toBuilder().put(TYPE_ATTR,
"total_space").build());
+ measurement.record(
+ Files.getFileStore(dataHome).getUsableSpace(),
+ containerAttrs.toBuilder().put(TYPE_ATTR,
"usable_space").build());
+ } catch (IOException e) {
+ throw new SolrException(
+ ErrorCode.SERVER_ERROR,
+ "Error retrieving disk space information for data home
directory" + dataHome,
+ e);
+ }
+ },
+ OtelUnit.BYTES);
+ solrMetricsContext.observableLongGauge(
+ "solr_cores_root_disk_space",
Review Comment:
I suggest dropping this as redundant with "data". If someone defined "data"
(which is maybe rare and perhaps should be unsupported), then this different
directory would be insignificant, only holding metadata.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
