-
Notifications
You must be signed in to change notification settings - Fork 619
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
metrics: refactor metrics to deal only with events #334
Closed
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 package to make integration with aggregation and event based metrics libraries simpler. It drops the Counter and Timer shim objects and exposes only functions for triggering metric events. Namespacing is supported but only for go-metrics based libraries. The api no longer exposes the current rate and 99% percentile.
Closed
magiconair
added a commit
that referenced
this pull request
Mar 24, 2018
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 #126 Fixes #165 Fixes #211 Fixes #253 Fixes #326 Fixes #327 Fixes #371 Closes #329 Closes #331 Closes #334 Closes #335
Merged
Please see #211 (comment) |
nathanejohnson
pushed a commit
that referenced
this pull request
Oct 23, 2020
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 #126 Fixes #165 Fixes #211 Fixes #253 Fixes #326 Fixes #327 Fixes #371 Closes #329 Closes #331 Closes #334 Closes #335 Remove 'Register' methods and create objects directly. Add Gauge and use it for websocket connections Vendoring in github.com/alexcesaro/statsd First stab at integrating raw statsd from #335 Rebase off master 10/23/2020 - NJ
nathanejohnson
pushed a commit
that referenced
this pull request
Oct 23, 2020
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 #126 Fixes #165 Fixes #211 Fixes #253 Fixes #326 Fixes #327 Fixes #371 Closes #329 Closes #331 Closes #334 Closes #335 Remove 'Register' methods and create objects directly. Add Gauge and use it for websocket connections Vendoring in github.com/alexcesaro/statsd First stab at integrating raw statsd from #335 Rebase off master 10/23/2020 - NJ
nathanejohnson
pushed a commit
that referenced
this pull request
Oct 23, 2020
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 #126 Fixes #165 Fixes #211 Fixes #253 Fixes #326 Fixes #327 Fixes #371 Closes #329 Closes #331 Closes #334 Closes #335 Remove 'Register' methods and create objects directly. Add Gauge and use it for websocket connections Vendoring in github.com/alexcesaro/statsd First stab at integrating raw statsd from #335 Rebase off master 10/23/2020 - NJ
nathanejohnson
pushed a commit
that referenced
this pull request
Dec 1, 2020
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 #126 Fixes #165 Fixes #211 Fixes #253 Fixes #326 Fixes #327 Fixes #371 Closes #329 Closes #331 Closes #334 Closes #335 Remove 'Register' methods and create objects directly. Add Gauge and use it for websocket connections Vendoring in github.com/alexcesaro/statsd First stab at integrating raw statsd from #335 Rebase off master 10/23/2020 - NJ
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.
This patch refactors the metrics package to make integration with
aggregation and event based metrics libraries simpler. It drops the
Counter and Timer shim objects and exposes only functions for triggering
metric events.
Namespacing is supported but only for go-metrics based libraries.
The api no longer exposes the current rate and 99% percentile.