Fix hackeny.POOL.queue_count metric name

[GregMefford]

It looks like the code was actually populating metrics under the queue_count name, but declaring the histogram as queue_counter. I didn't actually test that this fixes it, but wanted to get a PR opened to see whether this is the name to use or not. It seems like this would be more consistent with the other metric names.

[benoitc]

main difficulty with it is that it's introducing a breaking change for those who're using them , so it will have to be in the next major release I guess. Otherwise I agree with the change. Let's keep it that PR around

[GregMefford]

Actually I think the way this PR is currently written, it should be a bugfix instead of a breaking change, right? The code currently uses the queue_count name when it updates the histogram:

• hackney/src/hackney_pool.erl

  Line 274 in 74159da

  Engine, [hackney_pool, PoolName, queue_count], dict:size(Pending2)

• hackney/src/hackney_pool.erl

  Line 382 in 74159da

  State#state.metrics, [hackney_pool, State#state.name, queue_count], dict:size(Pending2)

It just never declares that metric because it declares it as queue_counter. I think this means that the metric already won't show up in metrics systems where you need to pre-declare your metrics before using them. So the only breaking change is that it won't declare the queue_counter metric, which is never actually populated with data anyway.

[nkmanolovsumup]

@GregMefford @benoitc Wouldn't changing the update statement to queue_counter be more appropriate rather than renaming the metric itself? In that case it won't be a breaking change, the queue_counter histogram will actually work and you can change it to queue_count in the next major release if you find the name more appropriate.

[GregMefford]

I might be misunderstanding something, but it looks like in the code I linked above, it already is sending the metric updates as queue_count, it just incorrectly declares the metric as queue_counter.

[nkmanolovsumup]

@GregMefford yeah, sorry, for some reason I thought the opposite was done. I also think you are right and don't see how this could be a breaking change. Surely no one is relying on an empty histogram, right?

[benoitc]

Closing: The new pool implementation correctly uses queue_count for the metric name. The queue_counter/queue_count mismatch has been fixed. Additionally, the new architecture no longer has a queue (queue_count is always 0).