bjacobowitz commented on code in PR #3665:
URL: https://github.com/apache/solr/pull/3665#discussion_r2353664521
##########
solr/test-framework/src/java/org/apache/solr/cloud/SolrCloudAuthTestCase.java:
##########
@@ -336,46 +336,6 @@ private static Map<String, DataPointSnapshot>
getMetricValues(
return metrics;
}
- /**
- * Common test method to be able to check audit metrics
- *
- * @param className the class name to be used for composing prefix, e.g.
- * "SECURITY./auditlogging/SolrLogAuditLoggerPlugin"
- */
- protected void assertAuditMetricsMinimums(
- MiniSolrCloudCluster cluster, String className, int count, int errors)
Review Comment:
Do we think there are any users of this function outside of the Solr repo?
If so, we'll be breaking them.
##########
solr/core/src/test/org/apache/solr/security/AuditLoggerIntegrationTest.java:
##########
@@ -542,6 +583,60 @@ private void setupCluster(
myCluster.waitForAllNodes(10);
testHarness.get().setCluster(myCluster);
+
+ CoreContainer coreContainer =
myCluster.getJettySolrRunner(0).getCoreContainer();
+ assertNotNull(coreContainer);
+ metricsReader =
SolrMetricTestUtils.getPrometheusMetricReader(coreContainer, "solr.node");
+ }
+
+ private Labels.Builder getDefaultAuditLoggerMetricsLabelsBuilder() {
+ return Labels.builder()
+ .label("category", "SECURITY")
+ .label("plugin_name", "CallbackAuditLoggerPlugin")
+ .label("handler", "/auditlogging")
+ .label("otel_scope_name", "org.apache.solr");
+ }
+
+ private Labels getDefaultAuditLoggerMetricsLabels() {
+ return getDefaultAuditLoggerMetricsLabelsBuilder().build();
+ }
+
+ private void assertGaugeMetricValue(String metricName, Labels labels, long
expectedValue) {
+ GaugeSnapshot.GaugeDataPointSnapshot dataPointSnapshot =
(GaugeSnapshot.GaugeDataPointSnapshot)
+ SolrMetricTestUtils.getDataPointSnapshot(metricsReader, metricName,
labels);
+ assertNotNull(dataPointSnapshot);
+ assertEquals(expectedValue, dataPointSnapshot.getValue(), 0.0);
+ }
+
+ private void assertAuditMetricsMinimums(int count, int errors) {
+ Labels labels = getDefaultAuditLoggerMetricsLabels();
+ assertCounterMinimumWithRetry("solr_auditlogger_count", labels, count);
+
+ if (errors > 0) {
+ assertCounterMinimumWithRetry("solr_auditlogger_errors", labels, errors);
+ }
+ }
+
+ private void assertCounterMinimumWithRetry(String metricName, Labels labels,
int expectedMinimum) {
+ boolean success = checkCounterMinimum(metricName, labels, expectedMinimum);
+
+ if (!success && expectedMinimum > 0) {
+ log.info("First {} metric check failed, pausing 2s before re-attempt",
metricName);
+ try {
+ Thread.sleep(2000);
+ success = checkCounterMinimum(metricName, labels, expectedMinimum);
+ } catch (InterruptedException e) {
+ Thread.currentThread().interrupt();
+ }
Review Comment:
Nit: I wouldn't bother catching `InterruptedException`, and instead I'd just
add `throws InterruptedException` to the function signature (and whatever other
tests call this).
--
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]
