Monitoring API #1691

rimey · 2016-04-03T19:15:57Z

This initial version of the monitoring client supports querying of time series, metric descriptors, and monitored resource descriptors.

The authors of this code are @supriyagarg and myself, both at Google.

gcloud/monitoring/label.py

+    :param key: The name of the label.
+
+    :type value_type: string
+    :param value_type: The type of the label. It must be one of ``"STRING"``,


jgeewax · 2016-04-04T12:42:42Z

Awesome - this is great. I think we have a few high-level issues, but thanks so much for throwing this in.

Issues I can see that we need to figure out:

The namedtuple parent class: I don't recall seeing that pattern in this project. Not sure if that's a thing we want to use -- the main question I'd have is about mutability -- do any of those objects need to be manipulated at all?
The nested methods for iterating over lists of things: I think we have an iterator class that does this... Might be worthwhile to know that we don't want to use that.
Squashing commits: I think we might want to squash these 3 into a single PR-commit, but not 100% sure.
Coverage: I think we lose some test coverage here, so we'd need to get that back up.

Thanks again -- this is awesome

rimey · 2016-04-09T15:53:48Z

To your points:

I converted MetricDescriptor, ResourceDescriptor, and LabelDescriptor into ordinary classes. This could well be helpful when we add support for defining new custom metrics.

I left Metric, Resource, TimeSeries, and Point as named tuples. If there is a need to revisit them, it would be best to do it in the context of writing data to custom metrics.

There are already a couple of named tuple classes of this sort in streaming/http_wrapper.py, but they are not exposed.
The code for listing metric descriptors and resource descriptors no longer uses nested functions (not that there is anything wrong with nested functions). It would not be improved by rewriting to use the Iterator base class.

Moreover, it is better for these particular calls to return lists than iterators.
I don't have a preference as to whether to squash or not.
Test coverage is now at 100%. However, a number of the unit tests fail because pandas is not installed in the test environment. See How to handle an optional package dependency in tests? #1710.

@rimey

This initial version of the monitoring client for Datalab allows users to query timeseries data. It introduces a new module under gcp that users can import using: from gcp.stackdriver import monitoring Querying timeseries data: There is a `Query` class that allows users to query timeseries data for their monitored resources. They can initialize the query by specifying a metric type and time interval, and refine it by adding filters to it. The timeseries data is returned as a pandas DataFrame that allows users to further manipulate the data and visualize it within Datalab. IPython magics: There are a couple of IPython magic commands to allow users to list details of the available metrics and resource types. E.g.: %%monitoring list metrics The base library: The _impl directory is a snapshot of an earlier version of a monitoring client library that we have submitted to gcloud-python: googleapis/google-cloud-python#1691 It works as the base library on top of which the Datalab library adds interactive features. The authors of this code are @rimey and myself, both at Google.

1. Removed start_time from the query constructor/factory. 2. Added a select_interval() method for when you actually do have a start time in your hands, or for when you want to set the time interval on an existing query object.

Several related changes: - Removed resource_type from the constructor/factory parameters. - Added support for filtering on "resource_type" as a pseudo-label. - Renamed select_resource_labels() to select_resources() and select_metric_labels() to select_metrics().

The signature of the reduce() method is now as follows: reduce(self, cross_series_reducer, *group_by_fields)

I had dropped this, but I think we want it.

rimey · 2016-04-20T05:10:33Z

@dhermes, I'm done processing your comments. Could you put together a short list of the significant issues that you feel are still open? At this point a VC might indeed be helpful. Would you be available tomorrow morning?

@rimey

This initial version of the monitoring client for Datalab allows users to query timeseries data. It introduces a new module under gcp that users can import using: from gcp.stackdriver import monitoring Querying timeseries data: There is a `Query` class that allows users to query timeseries data for their monitored resources. They can initialize the query by specifying a metric type and time interval, and refine it by adding filters to it. The timeseries data is returned as a pandas DataFrame that allows users to further manipulate the data and visualize it within Datalab. IPython magics: There are a couple of IPython magic commands to allow users to list details of the available metrics and resource types. E.g.: %%monitoring list metrics The base library: The _impl directory is a snapshot of an earlier version of a monitoring client library that we have submitted to gcloud-python: googleapis/google-cloud-python#1691 It works as the base library on top of which the Datalab library adds interactive features. The authors of this code are @rimey and myself, both at Google.

gcloud/monitoring/metric.py

+            name=info['name'],
+            type=info['type'],
+            labels=tuple(LabelDescriptor._from_dict(label)
+                         for label in info.get('labels', [])),


(It's disabled project-wide.)

…ses.

@rimey

This initial version of the monitoring client for Datalab allows users to query timeseries data. It introduces a new module under gcp that users can import using: from gcp.stackdriver import monitoring Querying timeseries data: There is a `Query` class that allows users to query timeseries data for their monitored resources. They can initialize the query by specifying a metric type and time interval, and refine it by adding filters to it. The timeseries data is returned as a pandas DataFrame that allows users to further manipulate the data and visualize it within Datalab. IPython magics: There are a couple of IPython magic commands to allow users to list details of the available metrics and resource types. E.g.: %%monitoring list metrics The base library: The _impl directory is a snapshot of an earlier version of a monitoring client library that we have submitted to gcloud-python: googleapis/google-cloud-python#1691 It works as the base library on top of which the Datalab library adds interactive features. The authors of this code are @rimey and myself, both at Google.

dhermes · 2016-04-22T02:36:31Z

@rimey RE:

Could you put together a short list of the significant issues that you feel are still open?

Only 4 left to get through

…column headers.

rimey · 2016-04-22T15:40:37Z

@dhermes PTAL

Trailing backslashes

Done.

Double-list comp.

Done.

filter vs. filter_ and type vs. type_

Done for filter. I'd like to keep type.

Using existing datetime helpers

I changed this code in the unit test to parse its constants using the timestamp format string in gcloud._helpers. I don't want to introduce time zone logic just so I can use a "DRY" helper function instead of a function from the Python standard library.

Good enough?

dhermes · 2016-04-22T16:17:41Z

Yes I noticed! Thanks a lot.

AFAICT you only use type as a positional argument in MetricDescriptor.__init__ and ResourceDescriptor.__init__. So the name is somewhat less important since positional. Though maybe that isn't true for you, I noticed most calls to these constructors uses all keyword args.

As for setting self.type = type_, that isn't problematic since self.type is on the objects namespace, which cascades through self.__dict__, type(self).__dict__, and then down the MRO of the parent class. Thus there is no clobbering of __builtins__ when a builtin name is used as an instance / class property.

Are you still opposed to changing type -> type_ given this distinction between positional argument and instance attribute?

rimey · 2016-04-22T16:38:17Z

@dhermes I'd like to stick with type. You can't judge the external usage from my internal usage, and we will soon have more of both when we add creation and deletion of descriptors for custom metrics.

dhermes · 2016-04-22T16:41:50Z

I just sent out #1737 to re-enable to the redefined-builtin error. What clarity is lost when moving from type to type_? Do we think the extra typing will overwhelm the subset of users that decides to use it as a keyword rather than positional?

rimey · 2016-04-22T17:31:27Z

@dhermes: Done, under protest.

I don't agree with this decision, for reasons I have already explained. I intend to propose reverting the change later if it has a bad effect on the experience for end users.

Okay?

dhermes · 2016-04-22T17:34:17Z

SGTM. Happy to have a non-blocking discussion in an issue.

dhermes · 2016-04-22T17:35:04Z

All issues are resolved, LGTM. I'm going to do a squash merge for the very first time. Here goes.

@rimey

This initial version of the monitoring client for Datalab allows users to query timeseries data. It introduces a new module under gcp that users can import using: from gcp.stackdriver import monitoring Querying timeseries data: There is a `Query` class that allows users to query timeseries data for their monitored resources. They can initialize the query by specifying a metric type and time interval, and refine it by adding filters to it. The timeseries data is returned as a pandas DataFrame that allows users to further manipulate the data and visualize it within Datalab. IPython magics: There are a couple of IPython magic commands to allow users to list details of the available metrics and resource types. E.g.: %%monitoring list metrics The base library: The _impl directory is a snapshot of an earlier version of a monitoring client library that we have submitted to gcloud-python: googleapis/google-cloud-python#1691 It works as the base library on top of which the Datalab library adds interactive features. The authors of this code are @rimey and myself, both at Google.

rimey added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Apr 3, 2016

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Apr 3, 2016

jgeewax reviewed Apr 4, 2016
View reviewed changes

rimey mentioned this pull request Apr 8, 2016

How to handle an optional package dependency in tests? #1710

Closed

supriyagarg mentioned this pull request Apr 8, 2016

Monitoring API googledatalab/datalab#795

Closed

rimey added 12 commits April 11, 2016 17:28

Add gcloud.monitoring with fetching of metric descriptors.

2af55db

Add fetching of resource descriptors.

a03740e

Add querying of time series.

6f57e6a

Add unit tests for everything but time series.

8ea9fc5

Added some links to the documentation, and a comment.

702f874

Dropped two nested generators.

80843b0

Require start time and end time to be datetimes rather than strings.

1d4fcaa

Change how query intervals are specified.

a5bab44

1. Removed start_time from the query constructor/factory. 2. Added a select_interval() method for when you actually do have a start time in your hands, or for when you want to set the time interval on an existing query object.

Add some pseudo-enums providing constants you can use if you wish.

a1959b3

More unit tests, and a few related changes.

ac12ade

The signature of the reduce() method is now as follows: reduce(self, cross_series_reducer, *group_by_fields)

Added a metric_type property.

380e7e4

I had dropped this, but I think we want it.

dhermes reviewed Apr 21, 2016
View reviewed changes

gcloud/monitoring/metric.py

name=info['name'],

type=info['type'],

labels=tuple(LabelDescriptor._from_dict(label)

for label in info.get('labels', [])),

This comment was marked as spam.

Sign in to view

This comment was marked as spam.

Sign in to view

rimey mentioned this pull request Apr 21, 2016

Review the datetime helpers in gcloud._helpers. #1732

Closed

rimey added 4 commits April 21, 2016 16:15

Remove pylint comment disabling redefined-builtin.

fe8ee01

(It's disabled project-wide.)

Cache the label map without having to disable pylint.

1f5e9e7

Excercise the test harness fully by reading past the available respon…

2d29693

…ses.

Change some more empty lists into empty tuples.

2dfd890

dhermes mentioned this pull request Apr 22, 2016

Removing some YAGNI test fixtures and no cover pragmas. #1734

Merged

rimey added 4 commits April 22, 2016 08:47

filter/filter_ -> filter_string

681292d

'%Y-%m-%dT%H:%M:%S.%fZ' -> _RFC3339_MICROS

b6bf58e

Use parentheses instead of backslashes.

a088e33

Reorganize a hard-to-understand bit of code for building multi-level …

ae124ae

…column headers.

dhermes mentioned this pull request Apr 22, 2016

Re-enabling redefined-builtin for Pylint. #1737

Merged

type -> type_

13e98c0

dhermes merged commit 7983938 into googleapis:master Apr 22, 2016

dhermes mentioned this pull request Apr 22, 2016

Upgrading version to 0.13.0. #1740

Merged

rimey added the api: monitoring Issues related to the Cloud Monitoring API. label Apr 23, 2016

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Monitoring API #1691

Monitoring API #1691

rimey commented Apr 3, 2016 •

edited

Loading

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

jgeewax commented Apr 4, 2016

rimey commented Apr 9, 2016

rimey commented Apr 20, 2016

This comment was marked as spam.

This comment was marked as spam.

dhermes commented Apr 22, 2016

rimey commented Apr 22, 2016

dhermes commented Apr 22, 2016

rimey commented Apr 22, 2016

dhermes commented Apr 22, 2016 •

edited

Loading

rimey commented Apr 22, 2016

dhermes commented Apr 22, 2016

dhermes commented Apr 22, 2016

Monitoring API #1691

Monitoring API #1691

Conversation

rimey commented Apr 3, 2016 • edited Loading

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

This comment was marked as spam.

jgeewax commented Apr 4, 2016

rimey commented Apr 9, 2016

rimey commented Apr 20, 2016

This comment was marked as spam.

This comment was marked as spam.

dhermes commented Apr 22, 2016

rimey commented Apr 22, 2016

dhermes commented Apr 22, 2016

rimey commented Apr 22, 2016

dhermes commented Apr 22, 2016 • edited Loading

rimey commented Apr 22, 2016

dhermes commented Apr 22, 2016

dhermes commented Apr 22, 2016

rimey commented Apr 3, 2016 •

edited

Loading

dhermes commented Apr 22, 2016 •

edited

Loading