Skip to content
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

Added prometheus metrics #366

Merged
merged 1 commit into from
Mar 2, 2022
Merged

Added prometheus metrics #366

merged 1 commit into from
Mar 2, 2022

Conversation

neufeldtech
Copy link
Contributor

@neufeldtech neufeldtech commented Feb 25, 2022

Implements #365

Added two different collectors for Prometheus metrics that can be enabled independently of each other with CLI flags. These metrics are served from /metrics HTTP api endpoint when at least one of the collectors is enabled.

  1. Runtime metrics -runtime-metrics
    These runtime metrics are 'common metrics' that come from these collectors:
  1. Proxy Metrics -proxy-metrics
    These are toxiproxy-specific metrics that expose the internal state/metrics of toxiproxy. See METRICS.md for description of these metrics.

The current proxy metrics look like this:

# HELP toxiproxy_proxy_received_bytes_total The total number of bytes received on a given proxy link in a given direction
# TYPE toxiproxy_proxy_received_bytes_total counter
toxiproxy_proxy_received_bytes_total{direction="downstream",listener="[::]:8080",proxy="my-proxy",upstream="httpbin.org:80"} 463
toxiproxy_proxy_received_bytes_total{direction="upstream",listener="[::]:8080",proxy="my-proxy",upstream="httpbin.org:80"} 89

# HELP toxiproxy_proxy_sent_bytes_total The total number of bytes sent on a given proxy link in a given direction
# TYPE toxiproxy_proxy_sent_bytes_total counter
toxiproxy_proxy_sent_bytes_total{direction="downstream",listener="[::]:8080",proxy="my-proxy",upstream="httpbin.org:80"} 463
toxiproxy_proxy_sent_bytes_total{direction="upstream",listener="[::]:8080",proxy="my-proxy",upstream="httpbin.org:80"} 89

@neufeldtech neufeldtech marked this pull request as ready for review February 27, 2022 18:36
README.md Show resolved Hide resolved
link.go Outdated Show resolved Hide resolved
metrics/metrics.go Outdated Show resolved Hide resolved
link.go Outdated Show resolved Hide resolved
@miry
Copy link
Contributor

miry commented Feb 28, 2022

I think by default the metrics should be disabled. We should allow to enable metrics.
It would help to have initialization part for metrics, than could be configured.

Some example that looks like covers my comments: https://developpaper.com/add-prometheus-monitoring-indicators-for-go-applications/

@neufeldtech
Copy link
Contributor Author

I think by default the metrics should be disabled. We should allow to enable metrics. It would help to have initialization part for metrics, than could be configured.

Some example that looks like covers my comments: https://developpaper.com/add-prometheus-monitoring-indicators-for-go-applications/

It would help to have initialization part for metrics, than could be configured.

I will refactor to make metrics optional (feature flagged) via a command line arg (and likely an environment variable as well, for easy Kubernetes compatibility).

@neufeldtech
Copy link
Contributor Author

We might need to discuss how to integrate this a little further - I just found another instance of NewProxy() where we don't have access to the server parent struct to get the metrics config data

https://github.com/Shopify/toxiproxy/blob/master/proxy_collection.go#L97-L101

api.go Outdated Show resolved Hide resolved
metrics/common.go Outdated Show resolved Hide resolved
metrics/proxy.go Outdated Show resolved Hide resolved
api_test.go Show resolved Hide resolved
link.go Outdated Show resolved Hide resolved
metrics.go Outdated Show resolved Hide resolved
scripts/test-e2e Show resolved Hide resolved
Copy link
Contributor

@miry miry left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Small comments left and linting errors. Overall it looks awesome.

cmd/server/server.go Outdated Show resolved Hide resolved
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants