Remove/reduce usage of MetricBuildItem

[ebullient]

Remove/reduce usage of MetricBuildItem to allow support for different metrics systems.
This will resolve #8272

@Sanne -- this is the next step of our conversation. This is similar / comparable to what the micrometer HibernateMeterBinder does, and is a basic re-frame of what the MetricBuildItem did, but is more direct.

@kenfinnigan / @jmartisk : FYI & review, too

For Hibernate-ORM metrics produced look like this w/ the changes in this PR:

# HELP vendor_hibernate_orm_collections_fetched_total Global number of collections fetched
# TYPE vendor_hibernate_orm_collections_fetched_total counter
vendor_hibernate_orm_collections_fetched_total{entityManagerFactory="default"} 0.0
# HELP vendor_hibernate_orm_collections_loaded_total Global number of collections loaded
# TYPE vendor_hibernate_orm_collections_loaded_total counter
vendor_hibernate_orm_collections_loaded_total{entityManagerFactory="default"} 0.0
# HELP vendor_hibernate_orm_collections_recreated_total Global number of collections recreated
# TYPE vendor_hibernate_orm_collections_recreated_total counter
vendor_hibernate_orm_collections_recreated_total{entityManagerFactory="default"} 5.0
# HELP vendor_hibernate_orm_collections_removed_total Global number of collections removed
# TYPE vendor_hibernate_orm_collections_removed_total counter
vendor_hibernate_orm_collections_removed_total{entityManagerFactory="default"} 0.0
# HELP vendor_hibernate_orm_collections_updated_total Global number of collections updated
# TYPE vendor_hibernate_orm_collections_updated_total counter
vendor_hibernate_orm_collections_updated_total{entityManagerFactory="default"} 0.0
# HELP vendor_hibernate_orm_connections_obtained_total Get the global number of connections asked by the sessions (the actual number of connections used may be much smaller depending whether you use a connection pool or not)
# TYPE vendor_hibernate_orm_connections_obtained_total counter
vendor_hibernate_orm_connections_obtained_total{entityManagerFactory="default"} 54.0
# HELP vendor_hibernate_orm_entities_deleted_total Global number of entity deletes
# TYPE vendor_hibernate_orm_entities_deleted_total counter
vendor_hibernate_orm_entities_deleted_total{entityManagerFactory="default"} 1.0
# HELP vendor_hibernate_orm_entities_fetched_total Global number of entity fetches
# TYPE vendor_hibernate_orm_entities_fetched_total counter
vendor_hibernate_orm_entities_fetched_total{entityManagerFactory="default"} 0.0
# HELP vendor_hibernate_orm_entities_inserted_total Global number of entity inserts
# TYPE vendor_hibernate_orm_entities_inserted_total counter
vendor_hibernate_orm_entities_inserted_total{entityManagerFactory="default"} 15.0
# HELP vendor_hibernate_orm_entities_loaded_total Global number of entity loads
# TYPE vendor_hibernate_orm_entities_loaded_total counter
vendor_hibernate_orm_entities_loaded_total{entityManagerFactory="default"} 3.0
# HELP vendor_hibernate_orm_entities_updated_total Global number of entity updates
# TYPE vendor_hibernate_orm_entities_updated_total counter
vendor_hibernate_orm_entities_updated_total{entityManagerFactory="default"} 2.0
# HELP vendor_hibernate_orm_flushes_total Global number of flush operations executed (either manual or automatic).
# TYPE vendor_hibernate_orm_flushes_total counter
vendor_hibernate_orm_flushes_total{entityManagerFactory="default"} 26.0
# HELP vendor_hibernate_orm_natural_id_cache_hits_total Global number of cached natural id lookups successfully retrieved from cache
# TYPE vendor_hibernate_orm_natural_id_cache_hits_total counter
vendor_hibernate_orm_natural_id_cache_hits_total{entityManagerFactory="default"} 0.0
# HELP vendor_hibernate_orm_natural_id_cache_misses_total Global number of cached natural id lookups *not* found in cache
# TYPE vendor_hibernate_orm_natural_id_cache_misses_total counter
vendor_hibernate_orm_natural_id_cache_misses_total{entityManagerFactory="default"} 0.0
# HELP vendor_hibernate_orm_natural_id_cache_puts_total Global number of cacheable natural id lookups put in cache
# TYPE vendor_hibernate_orm_natural_id_cache_puts_total counter
vendor_hibernate_orm_natural_id_cache_puts_total{entityManagerFactory="default"} 0.0
# HELP vendor_hibernate_orm_natural_id_queries_executions_total Global number of natural id queries executed against the database
# TYPE vendor_hibernate_orm_natural_id_queries_executions_total counter
vendor_hibernate_orm_natural_id_queries_executions_total{entityManagerFactory="default"} 0.0
# HELP vendor_hibernate_orm_optimistic_lock_failures_total The number of Hibernate StaleObjectStateExceptions or JPA OptimisticLockExceptions that occurred.
# TYPE vendor_hibernate_orm_optimistic_lock_failures_total counter
vendor_hibernate_orm_optimistic_lock_failures_total{entityManagerFactory="default"} 0.0
# HELP vendor_hibernate_orm_queries_executed_total Global number of executed queries
# TYPE vendor_hibernate_orm_queries_executed_total counter
vendor_hibernate_orm_queries_executed_total{entityManagerFactory="default"} 21.0
# HELP vendor_hibernate_orm_query_cache_hits_total Global number of cached queries successfully retrieved from cache
# TYPE vendor_hibernate_orm_query_cache_hits_total counter
vendor_hibernate_orm_query_cache_hits_total{entityManagerFactory="default"} 0.0
# HELP vendor_hibernate_orm_query_cache_misses_total Global number of cached queries *not* found in cache
# TYPE vendor_hibernate_orm_query_cache_misses_total counter
vendor_hibernate_orm_query_cache_misses_total{entityManagerFactory="default"} 0.0
# HELP vendor_hibernate_orm_query_cache_puts_total Global number of cacheable queries put in cache
# TYPE vendor_hibernate_orm_query_cache_puts_total counter
vendor_hibernate_orm_query_cache_puts_total{entityManagerFactory="default"} 0.0
# HELP vendor_hibernate_orm_second_level_cache_hits_total Global number of cacheable entities/collections successfully retrieved from the cache
# TYPE vendor_hibernate_orm_second_level_cache_hits_total counter
vendor_hibernate_orm_second_level_cache_hits_total{entityManagerFactory="default"} 0.0
# HELP vendor_hibernate_orm_second_level_cache_misses_total Global number of cacheable entities/collections not found in the cache and loaded from the database.
# TYPE vendor_hibernate_orm_second_level_cache_misses_total counter
vendor_hibernate_orm_second_level_cache_misses_total{entityManagerFactory="default"} 0.0
# HELP vendor_hibernate_orm_second_level_cache_puts_total Global number of cacheable entities/collections put in the cache
# TYPE vendor_hibernate_orm_second_level_cache_puts_total counter
vendor_hibernate_orm_second_level_cache_puts_total{entityManagerFactory="default"} 0.0
# HELP vendor_hibernate_orm_sessions_closed_total Global number of sessions closed
# TYPE vendor_hibernate_orm_sessions_closed_total counter
vendor_hibernate_orm_sessions_closed_total{entityManagerFactory="default"} 16.0
# HELP vendor_hibernate_orm_sessions_open_total Global number of sessions opened
# TYPE vendor_hibernate_orm_sessions_open_total counter
vendor_hibernate_orm_sessions_open_total{entityManagerFactory="default"} 16.0
# HELP vendor_hibernate_orm_statements_closed_total The number of prepared statements that were released
# TYPE vendor_hibernate_orm_statements_closed_total counter
vendor_hibernate_orm_statements_closed_total{entityManagerFactory="default"} 0.0
# HELP vendor_hibernate_orm_statements_prepared_total The number of prepared statements that were acquired
# TYPE vendor_hibernate_orm_statements_prepared_total counter
vendor_hibernate_orm_statements_prepared_total{entityManagerFactory="default"} 62.0
# HELP vendor_hibernate_orm_timestamps_cache_hits_total Global number of timestamps successfully retrieved from cache
# TYPE vendor_hibernate_orm_timestamps_cache_hits_total counter
vendor_hibernate_orm_timestamps_cache_hits_total{entityManagerFactory="default"} 0.0
# HELP vendor_hibernate_orm_timestamps_cache_misses_total Global number of timestamp requests that were not found in the cache
# TYPE vendor_hibernate_orm_timestamps_cache_misses_total counter
vendor_hibernate_orm_timestamps_cache_misses_total{entityManagerFactory="default"} 0.0
# HELP vendor_hibernate_orm_timestamps_cache_puts_total Global number of timestamps put in cache
# TYPE vendor_hibernate_orm_timestamps_cache_puts_total counter
vendor_hibernate_orm_timestamps_cache_puts_total{entityManagerFactory="default"} 33.0
# HELP vendor_hibernate_orm_transactions_total The number of transactions we know to have completed
# TYPE vendor_hibernate_orm_transactions_total counter
vendor_hibernate_orm_transactions_total{entityManagerFactory="default"} 15.0
# HELP vendor_hibernate_orm_transactions_successful_total The number of transactions we know to have been successful
# TYPE vendor_hibernate_orm_transactions_successful_total counter
vendor_hibernate_orm_transactions_successful_total{entityManagerFactory="default"} 15.0

[jmartisk]

This PR moves metric registration from STATIC_INIT to a runtime @Observes StartupEvent :( can't we invent a way to do it during build time?
It also breaks the unified metric registration mechanism that we have invented and documented for extension developers. This would also need to be well documented (how we expect other extensions to register their metrics).

[ebullient]

This PR moves metric registration from STATIC_INIT to a runtime @Observes StartupEvent :( can't we invent a way to do it during build time?

I am sure we could go back a step and make this happen sooner. In this case, I followed what Ken did to get the micrometer hibernate binder working w/ the extension. In all things, there is eventually an order (these x things must be present to connect the dots). The key in this case is knowing the persistence units defined by the application. What was done previously was done at static init, but it also only looked for the default persistence unit.

It also breaks the unified metric registration mechanism that we have invented and documented for extension developers. This would also need to be well documented (how we expect other extensions to register their metrics).

It does break that approach, and it is something I think we should explore because the MetricBuildItem pattern bakes in hard dependencies between MP Metrics and deployment modules, and doesn't add a lot of benefit in trade.

(From the quarkus micrometer extension, look @ the integration test that uses hibernate.. in theory, that doesn't use SmallRye metrics at all, so SmallRye/MP Metrics shouldn't show up on the classpath as anything other than an optional compile dep... BUT.. because the deployment module uses MetricBuildItem and defines/references some metrics classes in its BuildItems, the Hibernate ORM extension itself has a hard dependency on the MP Metrics API).

[kenfinnigan]

I know there's been other discussions around registering metrics prior to startup, so my only comment is on the class description

[kenfinnigan]

I think this is ready to move out of "Draft" status.

Any objection @jmartisk or @Sanne?

[jmartisk]

I don't think this is ready until the Writing Extensions guide is updated with a concise new strategy how extensions should do this from now on, because this clearly goes against what we currently have documented.

[ebullient]

I don't think this is ready until the Writing Extensions guide is updated with a concise new strategy how extensions should do this from now on, because this clearly goes against what we currently have documented.

It is still in draft, and that's part of why. I have something else in flight now (see the "alternate build item" commit) that would be used to replace that part of the extension guide.

[ebullient]

It is still in draft, and that's part of why. I have something else in flight now (see the "alternate build item" commit) that would be used to replace that part of the extension guide.

• New build item: https://github.com/ebullient/quarkus/blob/metrics/core/deployment/src/main/java/io/quarkus/deployment/metrics/MetricsFactoryConsumerBuildItem.java
• New "MetricsFactory" API: https://github.com/ebullient/quarkus/blob/metrics/core/runtime/src/main/java/io/quarkus/runtime/metrics/MetricsFactory.java
• SmallRye extension implementation of MetricsFactory:
  • deployment:
    • https://github.com/ebullient/quarkus/blob/metrics/extensions/smallrye-metrics/deployment/src/main/java/io/quarkus/smallrye/metrics/deployment/SmallRyeMetricsProcessor.java#L501
    • https://github.com/ebullient/quarkus/blob/metrics/extensions/smallrye-metrics/deployment/src/main/java/io/quarkus/smallrye/metrics/deployment/SmallRyeMetricsProcessor.java#L536
  • runtime: https://github.com/ebullient/quarkus/blob/metrics/extensions/smallrye-metrics/runtime/src/main/java/io/quarkus/smallrye/metrics/runtime/SmallRyeMetricsFactory.java
• Agroal usage/consumption of MetricsFactory API:
  • deployment: https://github.com/ebullient/quarkus/blob/metrics/extensions/agroal/deployment/src/main/java/io/quarkus/agroal/deployment/AgroalMetricsProcessor.java
  • runtime: https://github.com/ebullient/quarkus/blob/metrics/extensions/agroal/runtime/src/main/java/io/quarkus/agroal/runtime/metrics/AgroalMetricsRecorder.java

[ebullient]

Added revisions to the extension guide to use the new build item. It cleans up a lot.

I think we need some additional guidance there (don’t count what can be timed, use overlapping names that take advantage of tags to support aggregation, etc)

[ebullient]

MetricsFactory for micrometer (to compare w/ SmallRye impl): https://github.com/ebullient/quarkus-micrometer-extension/blob/experimental/runtime/src/main/java/dev/ebullient/micrometer/runtime/MicrometerMetricsFactory.java

[jmartisk]

What if an extension needs to create a counter that will be updated via calls to inc() rather than be a supplier or function? I guess we need a counter() function here without parameters that just registers a plain counter

[ebullient]

my intent was to use supplier or ToDoubleFunction always via the shared API. IMO, it is a best practice for reusable libraries anyway.

[jmartisk]

Well, but it would force some libraries to make significant changes, because they work by calling inc() and don't create any state objects for their counters.

[ebullient]

I am going to go find out how common that is. So far, it isn't very. And yes, this is on me to achieve. The goal of this common facade is to make that transition easier... but the model here is what Hibernate, Agroal, and Vert.x use..

Creating any kind of additional Counter interface introduces a lot of wrapper items (which is essentially what the Counter type is.. it just isn't managed by the library.
MP Metrics, FWIW, uses long values with inc() for counters, while Micrometer uses double values with increment(). This is why a simple "counter" interface isn't as simple as it sounds. A MeterBinder pattern works best to cover this case (using metricsSystemSupported method to do something more advanced/native/specific).

In the case of http (for example), I'm doing something native to micrometer for tracking http/eventbus/... metrics within the micrometer extension. I imagine the MP Metrics extension will do the same, and I think (for that case) it is the wiser thing to do. For Micrometer, there are existing MeterBinders that can be reused/pulled in (still @ bytecode recording time), and I prefer to reuse those where we can (this is where metricsSystemSupported can be used for deferred imports to ensure optional/compile-only dependencies.

[kenfinnigan]

Looks great! Thanks!

[kenfinnigan]

@n1hility @jmartisk any remaining concerns before we can merge.

I appreciate we probably want to wait for 1.7 before the merge?

[jmartisk]

I expect the smallrye metrics extension to register its own container filters if it wants to provide REST-container level instrumentation instead

Well, we already do this (except it's in the resteasy extension: https://github.com/quarkusio/quarkus/blob/1.6.1.Final/extensions/resteasy-server-common/deployment/src/main/java/io/quarkus/resteasy/server/common/deployment/ResteasyServerCommonProcessor.java#L444). But this approach only allows the metrics to be registered at runtime (when the REST resources are first called), what I'm talking about is moving the registration to build-time, for which I'd need to use the MetricsFactory somehow. We've had a few confused users recently because REST metrics are currently registered lazily so I was planning to move the registration to build-time soon.

Then I got an idea that we could probably do this by producing (in the RestEasy extension) a Consumer<MetricsFactory> that checks whether the MetricsFactory can be cast to SmallRyeMetricsFactory, and if so, perform the cast and use it to register the required SimpleTimers. Therefore the SmallRyeMetricsFactory would need to have an extra method to register a SimpleTimer.
But this also has one more problem that it will require the RESTEasy extension to get a dependency on the SmallRye Metrics extension (to be able to cast to SmallRyeMetricsFactory), therefore we have a circular dependency problem: https://github.com/quarkusio/quarkus/blob/1.6.1.Final/extensions/smallrye-metrics/deployment/pom.xml#L52 - it's a test-scope dependency only, but a lot of tests use REST, so I'm not very keen on removing that

I'm not sure if we solve the circular dependency by moving this logic from the resteasy extension to the smallrye-metrics extension - because it depends on configuration values of the resteasy extension - can an extension read the config of a different extension? EDIT: yes it should be possible (but I haven't checked it in practice).

Any better ideas how to eagerly register the SimpleTimers described in the MP Metrics spec (https://download.eclipse.org/microprofile/microprofile-metrics-2.3/microprofile-metrics-spec-2.3.html#_optional_rest)?

[kenfinnigan]

One option is to not worry about MP Metrics REST endpoints, and their eager instantiation, and focus on shifting the spec towards the direction it needs to go.

With the proposed direction we're thinking of, I believe metrics would be eager and instrumented at the appropriate level without issue

[jmartisk]

This does not have anything to do with improving the spec for the future (which we definitely will do, but that's a longer term goal), this is fixing an immediate problem that several users have reported lately and I've promised them a resolution.

[kenfinnigan]

But if we can give them the functionality they're requesting by moving things forward away from MP Metrics, doesn't that solve the underlying concern without adding solutions that we don't necessarily like just because of "where things stand now"?

[ebullient]

Taking things one bit at a time: There is a way to declare/use timers in a simple way via the MetricsFactory. In the case of more extensive build or runtime instrumentation for timers, I think that has to move into the metrics extension. E.g. for micrometer, the vert.x binder is enabled or disabled and includes all REST traffic -- it wouldn't do anything with RESTEasy at all. I think enable/disable of metrics can (in the enable/disable case at least) move to the metrics extension.

Detailed adaptation (jaeger, maybe hibernate/agroal) can be in the provider extension, and yes, it would then have a compile-time/optional dependence on the API package only (not the extension, just the API, and preferably only in the runtime module, and not in the deployment module). It could use the MetricsCapability build item to determine at build time which system to initialize.

[kenfinnigan]

@gsmet any concerns from your side in merging this?

[ebullient]

Then I got an idea that we could probably do this by producing (in the RestEasy extension) a Consumer<MetricsFactory> that checks whether the MetricsFactory can be cast to SmallRyeMetricsFactory, and if so, perform the cast and use it to register the required SimpleTimers. Therefore the SmallRyeMetricsFactory would need to have an extra method to register a SimpleTimer.

Declared MetricBuildItems were processed at STATIC_INIT time previously. In the new case, there are a few choices, but all rely on a recorder to keep API dependencies in the runtime module. Usage could still be recorded (to eagerly register metrics) at either STATIC_INIT or RUNTIME_INIT. While the mechanism around declaring the metrics is different (to avoid deployment module entanglement), registration processing time (bytecode recording time) is the same (or can be earlier than currently)

[gsmet]

I don't have a strong opinion about it. @jmartisk is really the guardian of this part so I let him handle that one.

As long as it gets merged before CR1 is packaged (planned for this Thursday), I'm good with it.

[jmartisk]

Declared MetricBuildItems were processed at STATIC_INIT time previously. In the new case, there are a few choices, but all rely on a recorder to keep API dependencies in the runtime module. Usage could still be recorded (to eagerly register metrics) at either STATIC_INIT or RUNTIME_INIT. While the mechanism around declaring the metrics is different (to avoid deployment module entanglement), registration processing time (bytecode recording time) is the same (or can be earlier than currently)

I'm not sure we're on the same page here, my comment wasn't about bytecode recording phases, I was asking how to use the MetricsFactory to register a SimpleTimer, which is something that isn't covered by the new MetricsFactory interface. Extensions that might want to register, say, a Histogram, will have a similar problem.
I don't know if there is any such extension at this time (apart from RESTEasy if we want to implement build-time metric registration there), not in the core repo, but the community might have created some.

If such cases want to continue working (after this PR) without using hacks like what I sketched in #10626 (comment), they will have to stay with the original MetricBuildItem, and therefore stay tied to MP Metrics, not benefitting from this PR.
Many extensions will have to stay MP-Metrics-only anyway, because they have a hard dependency on it (Infinispan, Cassandra, SR Fault Tolerance, SR GraphQL, SR Reactive Messaging,...) so we can't expect them to support multiple metric systems any time soon.

I'm not against merging this PR as I agree with its longer-term usefulness, I'm just pointing out the potential mess that we're creating at the same time.
I can't comment how much this will be in line with the next major revamp of MP Metrics spec, because I don't really know yet what that will look like, or when it will happen.

Given these circumstances, could we perhaps hold this off until 1.8? Or do we really want it in 1.7? I'm not that comfortable including such large changes at the last minute before release, especially if it does not directly benefit users of that release, but rather it changes things for extension developers.

[kenfinnigan]

To head us on the path we want to be on, I believe we do need to merge this sooner rather than later.

Once this is merged, we can look to include a Micrometer based extension.

As this changes developing extensions a bit in the direction we want, it would be good to include this in 1.7.

[gsmet]

@kenfinnigan if you plan to ship the Micrometer extension with Quarkus Core itself (or even the platform), I would say it could wait for 1.8 because there's no chance we will be able to merge it before 1.7.0.Final.

Or is there a secret plan I'm not aware of?

[ebullient]

@kenfinnigan if you plan to ship the Micrometer extension with Quarkus Core itself (or even the platform), I would say it could wait for 1.8 because there's no chance we will be able to merge it before 1.7.0.Final.

Or is there a secret plan I'm not aware of?

micrometer would come in for 1.8, for sure. From 1.7 perspective, this is about shifting the doc and guidance for extensions using metrics.

[kenfinnigan]

Right, the actual Micrometer extension wouldn't be included until post 1.7

[gsmet]

If so, and if we don't expect it being a pain, I would rather merge this at the beginning of the 1.8 cycle.

I see potential risks and no benefits for the user (until a Micrometer extension is introduced) but maybe I'm mistaken? Feel free to correct me if I have an incorrect image of this as I haven't followed this subject closely.

[kenfinnigan]

Yesterday morning I agreed with you @gsmet, but then @ebullient pointed out there are changes to how to integrate with metrics for extension writers that is important to include for the next product release

[ebullient]

I'm not sure we're on the same page here, my comment wasn't about bytecode recording phases, I was asking how to use the MetricsFactory to register a SimpleTimer, which is something that isn't covered by the new MetricsFactory interface. Extensions that might want to register, say, a Histogram, will have a similar problem.

I've updated the SmallRyeMetricsFactory to create SimpleTimer instead (as we discussed). An extension could cast the MetricsFactory in the recorder if they wanted, or they could use the API directly. Metrics can be a hot path, so while the MetricsFactory is designed to cover the 80% case, in the event that something more interesting is warranted, dropping to direct usage of the API will be better (MP Metrics histogram vs. Micrometer distribution summary, e.g.).

I don't know if there is any such extension at this time (apart from RESTEasy if we want to implement build-time metric registration there), not in the core repo, but the community might have created some.

Introducing the new build item doesn't break existing usage of the deprecated build item. We can work with those community projects to help them transition to something that makes the most sense.

If such cases want to continue working (after this PR) without using hacks like what I sketched in #10626 (comment), they will have to stay with the original MetricBuildItem, and therefore stay tied to MP Metrics, not benefitting from this PR.

This is a non-breaking change, so that is expected. You have to start somewhere.

Many extensions will have to stay MP-Metrics-only anyway, because they have a hard dependency on it (Infinispan, Cassandra, SR Fault Tolerance, SR GraphQL, SR Reactive Messaging,...) so we can't expect them to support multiple metric systems any time soon.

Again, I expect this will change over time. What we can do is remove the dependency between deployment modules for the extensions (while the API requirement would remain).

I'm not against merging this PR as I agree with its longer-term usefulness, I'm just pointing out the potential mess that we're creating at the same time.

This is just the first (non-breaking) step. The changes I have included for hibernate, agroal, jaeger, and k8s aren't strictly required, but they do prove that the new build item can be used to satisfy a few different use cases.

I can't comment how much this will be in line with the next major revamp of MP Metrics spec, because I don't really know yet what that will look like, or when it will happen.

We will have more options with this build item, at a minimum.

Given these circumstances, could we perhaps hold this off until 1.8? Or do we really want it in 1.7? I'm not that comfortable including such large changes at the last minute before release, especially if it does not directly benefit users of that release, but rather it changes things for extension developers.

If the "large" bits (hibernate, agroal, jaeger, k8s) are an issue, I can move those commits into a different PR. While they move pieces around, they are not destructive (nor do I consider them risky).

[n1hility]

Lots of great discussion on here but it sounds like most of the remaining points are issues we can look into as part of subsequent work on 1.8. AFAICT everything pertaining to the abstraction has been dealt with. The risk items that @gsmet brings up are excellent concerns this close to 1.7, but it sounds like @ebullient and @kenfinnigan have a high degree of confidence based on testing, and the point @kenfinnigan states above about increased adoption of an API we all know will change very soon is also important. On the balance I find this argument convincing and agree we should merge.

[jmartisk]

Very well, after some contemplation with my skeptical hat on, I finally give this my blessing.
Thanks @n1hility for your insight and @kenfinnigan and @ebullient for your patience with me ;)