Auto-configure Mongo metrics #23990

onobc · 2020-11-02T06:14:00Z

The current mechanism to enable Micrometer metrics for Mongo is to add the MongoMetricsCommandListener to the MongoClientSettings.Builder. This code proposal simply auto-configures in this command listener when appropriate.

ℹ️ There is also a MongoMetricsConnectionPoolListener that would add more Mongo (connection pool) metrics. However, I left it out because it has a dependency on legacy com.mongodb.MongoClient as well as some Mongo event classes that were removed in Mongo 4 Java driver. I am planning on submitting a fix to Micrometer to bring it up to speed w/ Mongo 4 driver. Once that is available, I will circle back here and auto-configure it in.

...va/org/springframework/boot/actuate/autoconfigure/metrics/MongoMetricsAutoConfiguration.java

onobc · 2020-11-02T06:18:35Z

...va/org/springframework/boot/actuate/autoconfigure/metrics/MongoMetricsAutoConfiguration.java

+	@ConditionalOnClass(MongoMetricsCommandListener.class)
+	@ConditionalOnProperty(name = "management.metrics.mongo.command-listener.enabled", havingValue = "true",
+			matchIfMissing = true)
+	static class MongoMetricsCommandListenerConfiguration {


I split this out into a nested config in anticipation of adding the metrics connection pool listener I alluded to in the initial description. They would be guarded by different classes and properties.

Perhaps this should be called MongoCommandMetricsConfiguration?

...g/springframework/boot/actuate/autoconfigure/metrics/MongoMetricsAutoConfigurationTests.java

spring-boot-project/spring-boot-docs/src/docs/asciidoc/production-ready-features.adoc

wilkinsona

Thanks very much for the PR, @Bono007. On first review, this looks excellent. I've left a few minor comments for your consideration when you have a moment.

Unfortunately, it's too late to add enhancements in 2.4, but we should hopefully be able to merge this for 2.5.

...g/springframework/boot/actuate/autoconfigure/metrics/MongoMetricsAutoConfigurationTests.java

spring-boot-project/spring-boot-docs/src/docs/asciidoc/production-ready-features.adoc

...va/org/springframework/boot/actuate/autoconfigure/metrics/MongoMetricsAutoConfiguration.java

onobc · 2020-11-11T02:04:14Z

Thanks very much for the PR, @Bono007. On first review, this looks excellent. I've left a few minor comments for your consideration when you have a moment.

Thanks @wilkinsona . I am just now getting back to this and will update w/ suggestions shortly.

Unfortunately, it's too late to add enhancements in 2.4, but we should hopefully be able to merge this for 2.5.

No worries.

jonatan-ivanov · 2021-03-25T00:14:47Z

@Bono007 Would you be up to merging this and #25758 together so that both mongo metrics listeners and tag providers will be auto-configured?

onobc · 2021-03-25T02:08:12Z

Perfect timing @jonatan-ivanov. My plan was to add the connection pool metrics listener auto-config this weekend. I will instead happily merge your proposal into this one in the next 12-36hrs.

@wilkinsona are you ok w/ me combining #25758 into here?

wilkinsona · 2021-03-25T07:55:45Z

The code in #25758 in its current form needs to be reworked as the additions have been made in spring-boot-autoconfigure rather than spring-boot-actuator-autoconfigure. I haven't looked at things in detail, but I suspect it may be quicker to redo the tag listener part on top of what's already proposed here rather than trying to merge things together.

onobc · 2021-03-25T12:18:32Z

Yep I saw that as well. Sorry for the lack of clarification @wilkinsona - I did not mean literally "merge" - but rather bring the code into this MR.

Sounds good. I will do this in the 24-36hrs.

onobc · 2021-03-27T13:51:52Z

...va/org/springframework/boot/actuate/autoconfigure/metrics/MongoMetricsAutoConfiguration.java

+	}
+
+	@ConditionalOnClass(MongoMetricsConnectionPoolListener.class)
+	@ConditionalOnProperty(name = "management.metrics.mongo.connectionpoollistener.enabled", havingValue = "true",


@wilkinsona it is a bit hard to not split connectionpoolllistener with hyphens in this case. I resisted the urge though.

Yeah, that's a pretty painful property name. I think we could improve the situation by keeping the fact that there's a listener involved as a lower-level detail and not exposing it in the property name. How about one of the following:

management.metrics.mongo.connectionpool

management.metrics.mongo.connection

management.metrics.mongo.pool

Given the naming used in Micrometer, I'm leaning towards 1.

onobc · 2021-03-27T13:52:58Z

spring-boot-project/spring-boot-docs/src/docs/asciidoc/production-ready-features.adoc

+* `mongodb.driver.pool.checkedout` that reports the count of connections that are currently in use
+* `mongodb.driver.pool.waitqueuesize` that reports the current size of the wait queue for a connection from the pool
+
+NOTE: The `waitqueuesize` metric was https://github.com/mongodb/specifications/blob/master/source/connection-monitoring-and-pooling/connection-monitoring-and-pooling.rst#why-are-waitqueuesize-and-waitqueuemultiple-deprecated[removed] in MongoDB 4.x


@jonatan-ivanov I went ahead and added the info about the waitqueuesize deprecation here as well.

The documentation that's linked to here doesn't seem to be directly related to the metric. The documentation is talking about the removal of a configuration setting rather than a metric for observing the current wait queue size. Given that Micrometer's MongoMetricsConnectionPoolListener provides the metric without any reliance of Mongo Driver APIs, I don't think there's any need to mention the deprecation here.

Fair enough. Ideally those docs would live in Micrometer anyways and at best this would just point to their docs. I will remove.

onobc · 2021-03-27T13:58:44Z

...va/org/springframework/boot/actuate/autoconfigure/metrics/MongoMetricsAutoConfiguration.java

+	@ConditionalOnClass(MongoMetricsConnectionPoolListener.class)
+	@ConditionalOnProperty(name = "management.metrics.mongo.connectionpoollistener.enabled", havingValue = "true",
+			matchIfMissing = true)
+	static class MongoMetricsConnectionPoolListenerConfiguration {


Basically everything in this nested config class was shift/lift from @jonatan-ivanov MR. Jonatan, I did not carry over the customizers concept as adding all listeners to the client settings is a more general Mongo auto-config feature and not needed specifically for the metric listeners. I could see that we may want to add that later though.

Perhaps this should be called MongoConnectionPoolMetricsConfiguration?

…ion-metadata.json

wilkinsona

Thanks again, @Bono007. I've left a few comments for you consideration.

I've also opened micrometer-metrics/micrometer#2541 which may have an effect on this changes. This is just FYI at the moment. If the Micrometer team adopt my suggestion, we can make any updates in Boot at that time.

wilkinsona · 2021-03-31T11:19:10Z

spring-boot-project/spring-boot-docs/src/docs/asciidoc/production-ready-features.adoc

+* `mongodb.driver.pool.checkedout` that reports the count of connections that are currently in use
+* `mongodb.driver.pool.waitqueuesize` that reports the current size of the wait queue for a connection from the pool
+
+NOTE: The `waitqueuesize` metric was https://github.com/mongodb/specifications/blob/master/source/connection-monitoring-and-pooling/connection-monitoring-and-pooling.rst#why-are-waitqueuesize-and-waitqueuemultiple-deprecated[removed] in MongoDB 4.x


The documentation that's linked to here doesn't seem to be directly related to the metric. The documentation is talking about the removal of a configuration setting rather than a metric for observing the current wait queue size. Given that Micrometer's MongoMetricsConnectionPoolListener provides the metric without any reliance of Mongo Driver APIs, I don't think there's any need to mention the deprecation here.

wilkinsona · 2021-03-31T11:22:14Z

...va/org/springframework/boot/actuate/autoconfigure/metrics/MongoMetricsAutoConfiguration.java

+	}
+
+	@ConditionalOnClass(MongoMetricsConnectionPoolListener.class)
+	@ConditionalOnProperty(name = "management.metrics.mongo.connectionpoollistener.enabled", havingValue = "true",


Yeah, that's a pretty painful property name. I think we could improve the situation by keeping the fact that there's a listener involved as a lower-level detail and not exposing it in the property name. How about one of the following:

management.metrics.mongo.connectionpool

management.metrics.mongo.connection

management.metrics.mongo.pool

Given the naming used in Micrometer, I'm leaning towards 1.

wilkinsona · 2021-03-31T11:23:00Z

...va/org/springframework/boot/actuate/autoconfigure/metrics/MongoMetricsAutoConfiguration.java

+public class MongoMetricsAutoConfiguration {
+
+	@ConditionalOnClass(MongoMetricsCommandListener.class)
+	@ConditionalOnProperty(name = "management.metrics.mongo.commandlistener.enabled", havingValue = "true",


I don't think we need listener in the name of this property. How about management.metrics.mongo.command.enabled instead?

[APPLY_THIS_TO_ALL_NAMING_COMMENTS]

@wilkinsona I agree that dropping the "listener" from all the naming things makes it much better and will follow up w/ that sometime today.

wilkinsona · 2021-03-31T11:23:45Z

...va/org/springframework/boot/actuate/autoconfigure/metrics/MongoMetricsAutoConfiguration.java

+	@ConditionalOnClass(MongoMetricsCommandListener.class)
+	@ConditionalOnProperty(name = "management.metrics.mongo.command-listener.enabled", havingValue = "true",
+			matchIfMissing = true)
+	static class MongoMetricsCommandListenerConfiguration {


Perhaps this should be called MongoCommandMetricsConfiguration?

wilkinsona · 2021-03-31T11:33:31Z

...va/org/springframework/boot/actuate/autoconfigure/metrics/MongoMetricsAutoConfiguration.java

+	@ConditionalOnClass(MongoMetricsConnectionPoolListener.class)
+	@ConditionalOnProperty(name = "management.metrics.mongo.connectionpoollistener.enabled", havingValue = "true",
+			matchIfMissing = true)
+	static class MongoMetricsConnectionPoolListenerConfiguration {


Perhaps this should be called MongoConnectionPoolMetricsConfiguration?

onobc · 2021-03-31T12:22:37Z

Thanks again, @Bono007. I've left a few comments for you consideration.

I've also opened micrometer-metrics/micrometer#2541 which may have an effect on this changes. This is just FYI at the moment. If the Micrometer team adopt my suggestion, we can make any updates in Boot at that time.

Oh man, that was me. I created these and while I did see the other tags provider I failed to snap to the naming not containing "Metrics". Uggh, naming. :) I will submit a proposal for these changes in Micrometer.

…ion doc

onobc · 2021-04-01T14:06:05Z

I made the suggested naming changes and yanked the link to the Mongo docs @wilkinsona . I like it much better. Please take a scan when you can. Thanks.

jonatan-ivanov · 2021-04-01T22:13:41Z

...va/org/springframework/boot/actuate/autoconfigure/metrics/MongoMetricsAutoConfiguration.java

@@ -0,0 +1,108 @@
+/*
+ * Copyright 2012-2020 the original author or authors.


Suggested change

* Copyright 2012-2020 the original author or authors.

* Copyright 2020-2021 the original author or authors.

2012-2021is correct here.

jonatan-ivanov · 2021-04-01T22:17:45Z

...va/org/springframework/boot/actuate/autoconfigure/metrics/MongoMetricsAutoConfiguration.java

+@Configuration(proxyBeanMethods = false)
+@AutoConfigureBefore(MongoAutoConfiguration.class)
+@AutoConfigureAfter({ MetricsAutoConfiguration.class, CompositeMeterRegistryAutoConfiguration.class })
+@ConditionalOnClass(MongoClientSettings.class)


Is the MongoClient available here, so that we could do @ConditionalOnClass(MongoClient.class) instead?

Since MongoClientSettings is shared by both the reactive and blocking client this allows the listeners to be shared by both flavors.

...va/org/springframework/boot/actuate/autoconfigure/metrics/MongoMetricsAutoConfiguration.java

jonatan-ivanov · 2021-04-01T22:28:14Z

...g/springframework/boot/actuate/autoconfigure/metrics/MongoMetricsAutoConfigurationTests.java

@@ -0,0 +1,200 @@
+/*
+ * Copyright 2012-2020 the original author or authors.


Suggested change

* Copyright 2012-2020 the original author or authors.

* Copyright 2020-2021 the original author or authors.

2012-2021 is correct here.

jonatan-ivanov · 2021-04-06T16:12:02Z

fyi: the tags providers were renamed, they are available in the snapshot: https://github.com/micrometer-metrics/micrometer/pull/2548/files

See gh-23990

wilkinsona · 2021-04-06T17:17:44Z

Thanks again for the PR, @Bono007. I've merged the proposed changes along with a small polishing commit.

spring-projects-issues added the status: waiting-for-triage An issue we've not yet triaged label Nov 2, 2020

onobc commented Nov 2, 2020

View reviewed changes

...va/org/springframework/boot/actuate/autoconfigure/metrics/MongoMetricsAutoConfiguration.java Outdated Show resolved Hide resolved

onobc commented Nov 2, 2020

View reviewed changes

...g/springframework/boot/actuate/autoconfigure/metrics/MongoMetricsAutoConfigurationTests.java Show resolved Hide resolved

onobc commented Nov 2, 2020

View reviewed changes

spring-boot-project/spring-boot-docs/src/docs/asciidoc/production-ready-features.adoc Show resolved Hide resolved

wilkinsona reviewed Nov 9, 2020

View reviewed changes

wilkinsona added type: enhancement A general enhancement and removed status: waiting-for-triage An issue we've not yet triaged labels Nov 9, 2020

wilkinsona added this to the 2.5.x milestone Nov 9, 2020

wilkinsona changed the title ~~Adds auto-configuration for Mongo metrics~~ Auto-configure Mongo metrics Nov 11, 2020

wilkinsona mentioned this pull request Mar 23, 2021

Autoconfiguring Mongo metrics listeners #25758

Closed

onobc added 3 commits March 26, 2021 22:27

Adds auto-configuration for Mongo metrics

c0eab89

Renames property name and updates readme

c1148e7

Add auto-configuration for MongoMetricsConnectionPoolListener

a6b31a2

onobc commented Mar 27, 2021

View reviewed changes

Adds Mongo metrics enabled properties to additional-spring-configurat…

901d072

…ion-metadata.json

wilkinsona requested changes Mar 31, 2021

View reviewed changes

Dropped "listener" from the naming and removed link to Mongo deprecat…

79cf5ac

…ion doc

onobc requested a review from wilkinsona April 1, 2021 14:06

jonatan-ivanov requested changes Apr 1, 2021

View reviewed changes

Updated copyrights.

48e1151

wilkinsona approved these changes Apr 6, 2021

View reviewed changes

wilkinsona self-assigned this Apr 6, 2021

wilkinsona modified the milestones: 2.5.x, 2.5.0-RC1 Apr 6, 2021

jonatan-ivanov approved these changes Apr 6, 2021

View reviewed changes

wilkinsona pushed a commit that referenced this pull request Apr 6, 2021

Auto-configure Mongo metrics

81c1821

See gh-23990

wilkinsona added a commit that referenced this pull request Apr 6, 2021

Polish "Auto-configure Mongo metrics"

73e1dd8

See gh-23990

wilkinsona closed this in 87dd329 Apr 6, 2021

		@@ -0,0 +1,108 @@
		/*
		* Copyright 2012-2020 the original author or authors.

	* Copyright 2012-2020 the original author or authors.
	* Copyright 2020-2021 the original author or authors.

		@@ -0,0 +1,200 @@
		/*
		* Copyright 2012-2020 the original author or authors.

Auto-configure Mongo metrics #23990

Auto-configure Mongo metrics #23990

Uh oh!

Conversation

onobc commented Nov 2, 2020

Uh oh!

Uh oh!

onobc Nov 2, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

wilkinsona left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

onobc commented Nov 11, 2020

Uh oh!

jonatan-ivanov commented Mar 25, 2021

Uh oh!

onobc commented Mar 25, 2021

Uh oh!

wilkinsona commented Mar 25, 2021

Uh oh!

onobc commented Mar 25, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

wilkinsona left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

onobc commented Mar 31, 2021

Uh oh!

onobc commented Apr 1, 2021

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

jonatan-ivanov commented Apr 6, 2021

Uh oh!

wilkinsona commented Apr 6, 2021

onobc Nov 2, 2020 •

edited

Loading