Skip to content

Conversation

@maxkuzkin
Copy link
Contributor

Google Monitoring API filters support <,<=,>,>= operators as described
in its documentation:
https://cloud.google.com/monitoring/api/v3/filters.

Current version of the library only has mappings of:

  1. ’{key}_prefix:{value}' -> '{key} = starts_with("{value}")'
  2. ’{key}_suffix:{value}' -> '{key} = ends_with("{value}")'

This patch introduces additional mappings of:

  1. ’{key}_greater:{value}' -> '{key} > {value}'
  2. ’{key}_greaterequal:{value}' -> '{key} >= {value}'
  3. ’{key}_less:{value}' -> '{key} < {value}'
  4. ’{key}_lessequal:{value}' -> '{key} <= {value}'

So that it is possible, for example, to filter based on the
response_code values range, as shown below:

metric.type = "appengine.googleapis.com/http/server/response_count"
AND metric.label.response_code < 600
AND metric.label.response_code >= 500

This patch will allow to remove “hack” in the following tool:
https://github.com/odin-public/gcpmetrics that enables
queries to the monitoring api from the command line.

Google Monitoring API filters support <,<=,>,>= operators as described
in its documentation:
https://cloud.google.com/monitoring/api/v3/filters.

Current version of the library only has mappings of:

1. ’{key}_prefix:{value}' -> '{key} = starts_with("{value}")'
2. ’{key}_suffix:{value}' -> '{key} = ends_with("{value}")'

This patch introduces additional mappings of:

3. ’{key}_greater:{value}' -> '{key} > {value}'
4. ’{key}_greaterequal:{value}' -> '{key} >= {value}'
5. ’{key}_less:{value}' -> '{key} < {value}'
6. ’{key}_lessequal:{value}' -> '{key} <= {value}'

So that it is possible, for example, to filter based on the
response_code values range, as shown below:

metric.type = "appengine.googleapis.com/http/server/response_count"
    AND metric.label.response_code < 600
    AND metric.label.response_code >= 500

This patch will allow to remove “hack” in the following tool:
https://github.com/odin-public/gcpmetrics that enables
queries to the monitoring api from the command line.
@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label Sep 1, 2016
Fixed `tox -e cover` to have 100% coverage after proposed patch
@rimey
Copy link
Contributor

rimey commented Sep 2, 2016

I would suggest inserting something like the following in the docstring for select_metrics at Line 315, immediately after the explanation of _prefix and _suffix. I don't think we need to say anything in the docstring for select_resources, because all of the currently defined monitored resource labels are strings.

        If the label's value type is ``INT64``, a similar notation can be
        used to express inequalities:

        ``<label>_less=<value>`` generates::

            metric.label.<label> < <value>

        ``<label>_lessequal=<value>`` generates::

            metric.label.<label> <= <value>

        ``<label>_greater=<value>`` generates::

            metric.label.<label> > <value>

        ``<label>_greaterequal=<value>`` generates::

            metric.label.<label> >= <value>

if key.endswith('_prefix') or key.endswith('_suffix'):
ends = ['_prefix', '_suffix', '_greater', '_greaterequal',
'_less', '_lessequal']
if key.endswith(tuple(ends)):

This comment was marked as spam.

@rimey
Copy link
Contributor

rimey commented Sep 2, 2016

I made just a couple of comments. Thank you for the contribution!

1. Added docstring for select_metrics with the explanation of
additional endings. No reason to do the same for select_resources,
because all of the currently defined monitored resource labels are
strings.
2. Removed array->tuple conversion, moved constant directly into the if
condition.
oops i did it again: messed docstring indentation while copy/pasting…
:-)
@maxkuzkin
Copy link
Contributor Author

Hi Ken, updated PR now has improvements you've suggested.

@rimey
Copy link
Contributor

rimey commented Sep 2, 2016

LGTM

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

cla: yes This human has signed the Contributor License Agreement.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants