New metrics #586
Closed
New metrics #586
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This patch refactors the metrics support within fabio to achieve several goals: * support labels for DogstatD, Prometheus and others * support raw events for statsd and others without aggregation * support multiple backends simultaneously to support migration * make integration of new metrics backends and/or libraries simple * being fully backwards compatible to not break existing setups One of the main challenges is that fabio generates names for the metrics targets through templates and that these templates are configurable. A metrics backend which supports labels needs to generate a different name for the same metric than one that does not support labels. Combining this with the need to support multiple different metrics backends at runtime means that none of the existing metrics libraries like go-kit/metrics and hashicorp/go-metrics is a good fit. However, they can be used as drivers which reduces the need for testing and makes integration of new backends simpler since fabio does not need to rely on a single metrics library. This patch is work in progress with the new metrics code in the metrics4 package and the old metrics code in the 'metrics_old' package where it is kept for reference until it has been migrated or removed. The basic architecture should be good enough to add more providers and functionality so that the community can help. Right now there are two providers "flat" and "label" to demonstrate the concepts. They provide counter and timers in statsd/dogstatd format and can be configured with -metrics.target=flat[,label]. Other providers should be added in the same way. The metrics_old code should be converted to the new provider scheme. The go-kit/metrics library currently supports most of the necessary drivers and is the preferred way of integrating new drivers. The list below is a probably incomplete list of things that need to be done: * The configuration is not used except for metrics.target * The drivers from the metrics_old package need to be migrated * Additional drivers from go-kit need to be added * The old rcrowley/go-metrics code relies on 'registries' for namespacing. After the routing table has changed the registry needs to be pruned of services which no longer exist so that go-metrics stops reporting them. * The RX/TX counters in the TCP and TCP+SNI proxy are probably sub-optimal or broken. * Some counters may not have been initialized properly, e.g. WSConn * WSConn should probably be a gauge which also needs to be set to 0 on startup. * The approach of injecting a noop metrics provider if none has been set has already bitten me once since some metrics are reported while others are not. I need to think about this a bit more. Fixes fabiolb#126 Fixes fabiolb#165 Fixes fabiolb#211 Fixes fabiolb#253 Fixes fabiolb#326 Fixes fabiolb#327 Fixes fabiolb#371 Closes fabiolb#329 Closes fabiolb#331 Closes fabiolb#334 Closes fabiolb#335
|
|
|
Triggering Travis build ... |
|
@maximka777 Thank you for the PR. Would you mind re-basing this on the current master so we can validate the new Travis tests? |
|
Also, please see #476 ... is this duplicative? |
|
@maximka777 I know there is still interest around updating the metrics. Frank has left the project and handed it over to myself and team for maintenance (see #735) for background. If you have the time I'd like to see a rebased and updated version of your work that includes changes frank made since you started this PR (if any) and current changes to master. |
Merged
|
@leprechau I will be trying to do this asap. |
|
@maximka777 do you have any plans of adding this feature? |
|
@vilva42 sorry, I'm not sure that I have enough time for this next few months. |
nathanejohnson
force-pushed
the
metrics4
branch
2 times, most recently
from
3518a54 to
5f99efb
Compare
nathanejohnson
force-pushed
the
metrics4
branch
from
a773cd6 to
b8ec4f4
Compare
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
No description provided.