[syahra712]

Description

docs/reference/telemetry.md documents --telemetry-params collection-stats-sample-interval:60 for the always-on collection stats device. However, _create_solr_telemetry_devices built SolrCollectionStats without a sample_interval_s, so the device always polled every 30 s.

This PR reads collection-stats-sample-interval from the telemetry params, with a default of 30, and passes it to SolrCollectionStats. Like ShardStats with shard-stats-sample-interval, SolrCollectionStats now raises SystemSetupError for values <= 0. Otherwise a zero interval would make _poll_loop poll Solr in a tight loop.

Issues Resolved

Fixes #136 (found while looking into #5)

Testing

• New functionality includes testing

• tests/worker_coordinator/worker_coordinator_test.py::WorkerCoordinatorTests::test_collection_stats_uses_sample_interval_param sets collection-stats-sample-interval: 7 and checks the created device's interval. On main it fails with 7 != 30.0.

• tests/unit/solr/test_telemetry.py::TestSolrCollectionStats::test_rejects_non_positive_sample_interval checks that 0 and -5 are rejected.

• The full tests/ suite passes locally on Python 3.12 (1176 passed, 5 skipped), and ruff check . passes. I haven't run the integration suite (it/).

This touches the same list in _create_solr_telemetry_devices as #135, but different lines, so either can merge first.

---

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

[janhoy]

Is this solution informed by how Opensearch Benchmarks wires things or is it a novel fix? We might want to consult “upstream” to find out where the solr-port went wrong.

[syahra712]

Good question. It's a gap in the port rather than a new design. SolrCollectionStats (like the other Solr*Stats devices) was written for Solr in the initial port (57387ed) and has no OpenSearch Benchmark counterpart. So there's no upstream wiring to copy for this device. The closest upstream pattern is OSB's NodeStats/ShardStats: they receive telemetry_params, read <device>-sample-interval themselves, and raise SystemSetupError for values <= 0. In solr-orbit, the ported ShardStats still works that way. The always-on Solr devices are built in _create_solr_telemetry_devices without the params, which is why the documented collection-stats-sample-interval was never read.

This PR copies the upstream validation but passes the value at the call site. If you'd rather match OSB exactly, I can pass telemetry_params into SolrCollectionStats.__init__ and read it there, as ShardStats does. Just say which you prefer.

I also merged main to fix the conflict with the ClusterEnvironmentInfo test (both tests kept). The full tests/ suite passes: 1177 passed, 5 skipped.

[janhoy]

Yes, let's standardize on passing telemetry_params for consistency and smoother to add params in the future without method signature changes.

[syahra712]

Done in 1015b71. SolrCollectionStats now takes telemetry_params and reads collection-stats-sample-interval itself, like ShardStats. It still defaults to 30 s and rejects values <= 0. The coordinator just passes telemetry_params through. I added a unit test for the default and for the param. The full suite passes (1178 passed, 5 skipped).