dsmiley commented on code in PR #3427:
URL: https://github.com/apache/solr/pull/3427#discussion_r2217356780
##########
solr/core/src/java/org/apache/solr/metrics/SolrMetricManager.java:
##########
@@ -725,8 +727,9 @@ public SdkMeterProvider meterProvider(String providerName) {
var reader = new PrometheusMetricReader(true, null);
// NOCOMMIT: We need to add a Periodic Metric Reader here if we
want to push with OTLP
// with an exporter
- var provider =
SdkMeterProvider.builder().registerMetricReader(reader).build();
- return new MeterProviderAndReaders(provider, reader);
+ var provider =
SdkMeterProvider.builder().registerMetricReader(reader);
Review Comment:
not a comment on this line but the javadoc of meterProvider() should not be
speaking of MeterProviderAndReaders as that's very internal to the class.
Javadocs are for the callers of the method.
##########
solr/core/src/java/org/apache/solr/response/PrometheusResponseWriter.java:
##########
@@ -55,12 +59,19 @@ public void write(
List<MetricSnapshot> snapshots =
readers.values().stream().flatMap(r -> r.collect().stream()).toList();
- new PrometheusTextFormatWriter(false).write(out,
mergeSnapshots(snapshots));
+ boolean openMetricsFormat =
"openmetrics".equals(request.getParams().get("format"));
Review Comment:
Solr has historically used request parameters to offer clients a means of
requesting different formats. While it's convenient for hacking/playing, it's
not using HTTP standards; the "Accept" header is the way to go. That said,
this doesn't mean we can't have a parameter as well. If we did, wouldn't we
use our existing param -- "wt" ? CC @gerlowski for confirmation
--
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]