dsmiley commented on code in PR #3745:
URL: https://github.com/apache/solr/pull/3745#discussion_r2421983495
##########
solr/core/src/java/org/apache/solr/util/stats/MetricUtils.java:
##########
@@ -833,17 +190,16 @@ public static ExecutorService instrumentedExecutorService(
* @param obj an instance of MXBean
* @param interfaces interfaces that it may implement. Each interface will
be tried in turn, and
* only if it exists and if it contains unique properties then they will
be added as metrics.
- * @param prefix optional prefix for metric names
* @param consumer consumer for created names and metrics
* @param <T> formal type
*/
public static <T extends PlatformManagedObject> void addMXBeanMetrics(
Review Comment:
And likewise the name and javadocs are no longer accurate
##########
solr/core/src/java/org/apache/solr/util/stats/MetricUtils.java:
##########
@@ -754,7 +125,7 @@ static void convertCounter(
* @param <T> formal type
*/
public static <T extends PlatformManagedObject> void addMXBeanMetrics(
Review Comment:
The javadocs of this method are no longer true. Maybe even the name needs
adjustment... perhaps foreachGetterValue?
It's not even metrics related at all; maybe it should move to its only
caller in SystemInfoHandler
##########
solr/core/src/java/org/apache/solr/util/stats/MetricUtils.java:
##########
@@ -499,249 +96,23 @@ static double nsToMs(boolean convert, double value) {
}
}
- // some snapshots represent time in ns, other snapshots represent raw values
(eg. chunk size)
- static void addSnapshot(
- MapWriter.EntryWriter ew,
- Snapshot snapshot,
- Predicate<CharSequence> propertyFilter,
- boolean ms) {
- BiConsumer<String, Object> filter =
- (k, v) -> {
- if (propertyFilter.test(k)) {
- ew.putNoEx(k, v);
- }
- };
- filter.accept((ms ? MIN_MS : MIN), nsToMs(ms, (double) snapshot.getMin()));
- filter.accept((ms ? MAX_MS : MAX), nsToMs(ms, (double) snapshot.getMax()));
- filter.accept((ms ? MEAN_MS : MEAN), nsToMs(ms, snapshot.getMean()));
- filter.accept((ms ? MEDIAN_MS : MEDIAN), nsToMs(ms, snapshot.getMedian()));
- filter.accept((ms ? STDDEV_MS : STDDEV), nsToMs(ms, snapshot.getStdDev()));
- filter.accept((ms ? P75_MS : P75), nsToMs(ms,
snapshot.get75thPercentile()));
- filter.accept((ms ? P95_MS : P95), nsToMs(ms,
snapshot.get95thPercentile()));
- filter.accept((ms ? P99_MS : P99), nsToMs(ms,
snapshot.get99thPercentile()));
- filter.accept((ms ? P999_MS : P999), nsToMs(ms,
snapshot.get999thPercentile()));
- }
-
- /**
- * Convert a {@link Timer} to a map.
- *
- * @param name metric name
- * @param timer timer instance
- * @param propertyFilter limit what properties of a metric are returned
- * @param skipHistograms if true then discard the histogram part of the
timer.
- * @param simple use simplified representation for complex metrics - instead
of a (name, map) only
- * the selected (name "." key, value) pairs will be produced.
- * @param consumer consumer that accepts produced objects
- */
- public static void convertTimer(
- String name,
- Timer timer,
- Predicate<CharSequence> propertyFilter,
- boolean skipHistograms,
- boolean simple,
- String separator,
- BiConsumer<String, Object> consumer) {
- if (simple) {
- String prop = "meanRate";
- if (propertyFilter.test(prop)) {
- consumer.accept(name + separator + prop, timer.getMeanRate());
- }
- } else {
- MapWriter writer =
- ew -> {
- BiConsumer<String, Object> filter =
- (k, v) -> {
- if (propertyFilter.test(k)) {
- ew.putNoEx(k, v);
- }
- };
- filter.accept("count", timer.getCount());
- filter.accept("meanRate", timer.getMeanRate());
- filter.accept("1minRate", timer.getOneMinuteRate());
- filter.accept("5minRate", timer.getFiveMinuteRate());
- filter.accept("15minRate", timer.getFifteenMinuteRate());
- if (!skipHistograms) {
- // time-based values in nanoseconds
- addSnapshot(ew, timer.getSnapshot(), propertyFilter, true);
- }
- };
- if (writer._size() > 0) {
- consumer.accept(name, writer);
- }
- }
- }
-
- /**
- * Convert a {@link Meter} to a map.
- *
- * @param name metric name
- * @param meter meter instance
- * @param propertyFilter limit what properties of a metric are returned
- * @param simple use simplified representation for complex metrics - instead
of a (name, map) only
- * the selected (name "." key, value) pairs will be produced.
- * @param consumer consumer that accepts produced objects
- */
- static void convertMeter(
- String name,
- Meter meter,
- Predicate<CharSequence> propertyFilter,
- boolean simple,
- String separator,
- BiConsumer<String, Object> consumer) {
- if (simple) {
- if (propertyFilter.test("count")) {
- consumer.accept(name + separator + "count", meter.getCount());
- }
- } else {
- MapWriter writer =
- ew -> {
- BiConsumer<String, Object> filter =
- (k, v) -> {
- if (propertyFilter.test(k)) {
- ew.putNoEx(k, v);
- }
- };
- filter.accept("count", meter.getCount());
- filter.accept("meanRate", meter.getMeanRate());
- filter.accept("1minRate", meter.getOneMinuteRate());
- filter.accept("5minRate", meter.getFiveMinuteRate());
- filter.accept("15minRate", meter.getFifteenMinuteRate());
- };
- if (writer._size() > 0) {
- consumer.accept(name, writer);
- }
- }
- }
-
- static void convertMapWriter(
- String name,
- MapWriter metric,
- Predicate<CharSequence> propertyFilter,
- boolean simple,
- boolean compact,
- String separator,
- BiConsumer<String, Object> consumer) {
- ConditionalKeyMapWriter filteredMetric = new
ConditionalKeyMapWriter(metric, propertyFilter);
- if (compact || simple) {
- if (simple) {
- filteredMetric._forEachEntry((k, v) -> consumer.accept(name +
separator + k, v));
- } else {
- if (filteredMetric._size() > 0) {
- consumer.accept(name, filteredMetric);
- }
- }
- } else {
- if (filteredMetric._size() > 0) {
- consumer.accept(name, (MapWriter) ew -> ew.putNoEx("value",
filteredMetric));
- }
- }
- }
-
/**
- * Convert a {@link Gauge}.
- *
- * @param name metric name
- * @param gauge gauge instance
- * @param propertyFilter limit what properties of a metric are returned
- * @param simple use simplified representation for complex metrics - instead
of a (name, map) only
- * the selected (name "." key, value) pairs will be produced.
- * @param compact if true then only return {@link Gauge#getValue()}. If
false then return a map
- * with a "value" field.
- * @param consumer consumer that accepts produced objects
- */
- static void convertGauge(
- String name,
- Gauge<?> gauge,
- Predicate<CharSequence> propertyFilter,
- boolean simple,
- boolean compact,
- String separator,
- BiConsumer<String, Object> consumer) {
- if (compact || simple) {
- Object o = gauge.getValue();
- if (o instanceof Map) {
- if (simple) {
- for (Map.Entry<?, ?> entry : ((Map<?, ?>) o).entrySet()) {
- String prop = entry.getKey().toString();
- if (propertyFilter.test(prop)) {
- consumer.accept(name + separator + prop, entry.getValue());
- }
- }
- } else {
- boolean notEmpty =
- ((Map<?, ?>) o)
- .entrySet().stream()
- .anyMatch(entry ->
propertyFilter.test(entry.getKey().toString()));
- MapWriter writer =
- ew -> {
- for (Map.Entry<?, ?> entry : ((Map<?, ?>) o).entrySet()) {
- String prop = entry.getKey().toString();
- if (propertyFilter.test(prop)) {
- ew.putNoEx(prop, entry.getValue());
- }
- }
- };
- if (notEmpty) {
- consumer.accept(name, writer);
- }
- }
- } else {
- consumer.accept(name, o);
- }
- } else {
- Object o = gauge.getValue();
- if (o instanceof Map) {
- boolean notEmpty =
- ((Map<?, ?>) o)
- .entrySet().stream()
- .anyMatch(entry ->
propertyFilter.test(entry.getKey().toString()));
- if (notEmpty) {
- consumer.accept(
- name,
- (MapWriter)
- ew -> {
- ew.putNoEx(
- "value",
- (MapWriter)
- ew1 -> {
- for (Map.Entry<?, ?> entry : ((Map<?, ?>)
o).entrySet()) {
- String prop = entry.getKey().toString();
- if (propertyFilter.test(prop)) {
- ew1.put(prop, entry.getValue());
- }
- }
- });
- });
- }
- } else {
- if (propertyFilter.test("value")) {
- consumer.accept(name, (MapWriter) ew -> ew.putNoEx("value", o));
- }
- }
- }
- }
-
- /**
- * Convert a {@link Counter}
+ * Adds metrics from a Timer to a NamedList, using well-known back-compat
names.
*
- * @param counter counter instance
- * @param propertyFilter limit what properties of a metric are returned
- * @param compact if true then only return {@link Counter#getCount()}. If
false then return a map
- * with a "count" field.
+ * @param lst The NamedList to add the metrics data to
+ * @param timer The Timer to extract the metrics from
*/
- static void convertCounter(
- String name,
- Counter counter,
- Predicate<CharSequence> propertyFilter,
- boolean compact,
- BiConsumer<String, Object> consumer) {
- if (compact) {
- consumer.accept(name, counter.getCount());
- } else {
- if (propertyFilter.test("count")) {
- consumer.accept(name, (MapWriter) ew -> ew.putNoEx("count",
counter.getCount()));
- }
- }
+ public static void addMetrics(NamedList<Object> lst, Timer timer) {
Review Comment:
You moved this from its former position. Can you put it back unless there's
a reason for the move?
##########
gradle/libs.versions.toml:
##########
@@ -320,11 +320,7 @@ cybozulabs-langdetect = { module =
"com.cybozu.labs:langdetect", version.ref = "
decompose-decompose = { module = "com.arkivanov.decompose:decompose",
version.ref = "decompose" }
decompose-extensions-compose = { module =
"com.arkivanov.decompose:extensions-compose", version.ref = "decompose" }
dropwizard-metrics-core = { module = "io.dropwizard.metrics:metrics-core",
version.ref = "dropwizard-metrics" }
Review Comment:
Okay; looks like another JIRA I guess. At least doesn't seem like a release
blocker.
--
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]
