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.

Sign up for GitHub

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: rename backend_response to threescale_backend_response #917

Merged
merged 1 commit into from
Oct 2, 2018
Merged

metrics: rename backend_response to threescale_backend_response #917

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
- Prometheus metrics for the 3scale batching policy [PR #902](https://github.com/3scale/apicast/pull/902)
- Support for path in the upstream URL [PR #905](https://github.com/3scale/apicast/pull/905)

### Changed

- Renamed the `backend_response` Prometheus metric to `threescale_backend_response` to avoid confusion with the upstream response [PR #917](https://github.com/3scale/apicast/pull/917)

## [3.3.0-cr2] - 2018-09-25

### Fixed
Expand Down
2 changes: 1 addition & 1 deletion gateway/src/apicast/metrics/3scale_backend_status.lua
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ local _M = {}

local backend_response_metric = prometheus(
'counter',
'backend_response',
'threescale_backend_response',
Copy link
Contributor

Choose a reason for hiding this comment

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

Can they start with a number? threescale should be the last resort how to write3scale

Copy link
Contributor Author

Choose a reason for hiding this comment

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

no, that's why I chose threescale :D

Copy link
Contributor

Choose a reason for hiding this comment

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

Sigh :( I really don't like it. Do we have any alternatives? Like prefixing it with something? If no, then let's merge it.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Hmm we might want to prefix all our metrics with apicast. That would solve the issue.

Copy link
Contributor

Choose a reason for hiding this comment

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

That does not sound that bad. I'm fine with both.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'll add the apicast prefix. I think it makes sense if we take into account that other apps can export metrics to the same Prometheus instance. Also, it's recommended in the Prometheus guidelines: https://prometheus.io/docs/practices/naming/

Copy link
Contributor

Choose a reason for hiding this comment

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

👍

"Response status codes from 3scale's backend",
{ 'status' }
)
Expand Down
14 changes: 7 additions & 7 deletions t/prometheus-metrics.t
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -108,13 +108,13 @@ that does not include the nginx metrics (tested in the previous test).
--- response_body eval
[ "yay, api backend\x{0a}", "yay, api backend\x{0a}", "Authentication failed", "Authentication failed",
<<'METRICS_OUTPUT'
# HELP backend_response Response status codes from 3scale's backend
# TYPE backend_response counter
backend_response{status="2xx"} 2
backend_response{status="4xx"} 2
# HELP nginx_metric_errors_total Number of nginx-lua-prometheus errors
# TYPE nginx_metric_errors_total counter
nginx_metric_errors_total 0
# HELP threescale_backend_response Response status codes from 3scale's backend
# TYPE threescale_backend_response counter
threescale_backend_response{status="2xx"} 2
threescale_backend_response{status="4xx"} 2
METRICS_OUTPUT
]
--- no_error_log
Expand Down Expand Up @@ -174,9 +174,6 @@ We use and env file without the nginx metrics to simplify the output of the
--- response_body eval
[ "yay, api backend\x{0a}", "yay, api backend\x{0a}", "yay, api backend\x{0a}",
<<'METRICS_OUTPUT'
# HELP backend_response Response status codes from 3scale's backend
# TYPE backend_response counter
backend_response{status="2xx"} 1
# HELP batching_policy_auths_cache_hits Hits in the auths cache of the 3scale batching policy
# TYPE batching_policy_auths_cache_hits counter
batching_policy_auths_cache_hits 2
Expand All @@ -186,6 +183,9 @@ batching_policy_auths_cache_misses 1
# HELP nginx_metric_errors_total Number of nginx-lua-prometheus errors
# TYPE nginx_metric_errors_total counter
nginx_metric_errors_total 0
# HELP threescale_backend_response Response status codes from 3scale's backend
# TYPE threescale_backend_response counter
threescale_backend_response{status="2xx"} 1
METRICS_OUTPUT
]
--- no_error_log
Expand Down