Skip to content

custom metrics API: simplification suggestion #2333

Closed

Description

@balopat

Suggestion:

Remove the Metric class and have MetricDescriptor settable on CreateTimeSeriesRequest - or something similar.

Background:

Based on the user guide to create custom metrics, I have to create a MetricDescriptor:

final MetricServiceClient client = MetricServiceClient.create();
ProjectName name = ProjectName.create(projectId);

MetricDescriptor descriptor = MetricDescriptor.newBuilder()
    .setType(metricType)
    .setDescription("This is a simple example of a custom metric.")
    .setMetricKind(MetricDescriptor.MetricKind.GAUGE)
    .setValueType(MetricDescriptor.ValueType.DOUBLE)
    .build();

CreateMetricDescriptorRequest request = CreateMetricDescriptorRequest.newBuilder()
    .setNameWithProjectName(name)
    .setMetricDescriptor(descriptor)
    .build();

client.createMetricDescriptor(request);

Great, now I have a metricDescriptor. What can I do with it? Nothing really.
In order to write data, I'll need to create a TimeSeries, and that needs another representation of this metric/metricdescriptor called Metric:

// Prepares the metric descriptor
Metric metric = Metric.newBuilder()
    .setType("custom.googleapis.com/my_metric")
    .build();
...
// create time series
TimeSeries timeSeries = TimeSeries.newBuilder()
    .setMetric(metric)
...

OKay, so I can't reuse the MetricDescriptor, I'll use Metric.
But then on the TimeSeries, I have .setMetricKind, .setMetricKindValue... I guess it is there so that if I create a TimeSeries without a MetricDescriptor first, the descriptor can be implicitly created. But then why introduce a separete class for it called Metric that doesn't even have the Kind, KindValue (~= ValueType? or just repetition?).

image

image

I find the situation confusing, as

  • the difference between Metric and MetricDescriptor is unclear
  • Metric related methods are lingering in the CreateTimeSeriesRequest
  • the difference between CreateTimeSeriesRequest.setMetricKind and CreateTimeSeriesRequest.setMetricKindValue is unclear for first look...is it possible to get rid of setMetricKindValue?

I understand that this code is largely generated for now from the protobufs (#2331), I would offer this issue as an example where the user experience can be increased significantly and how the generated code is not user friendly by default.

Activity

  1. added
    api: monitoringIssues related to the Cloud Monitoring API.
    priority: p2Moderately-important priority. Fix may not be included in next release.
    type: feature request‘Nice-to-have’ improvement, new feature or different behavior or design.
    on Aug 11, 2017
  2. garrettjonesgoogle commented on Aug 16, 2017

    @garrettjonesgoogle
    Contributor

    @balopat could you update this issue with your current thoughts (given that we have had some internal discussions)?

  3. balopat commented on Aug 17, 2017

    @balopat
    Author

    @garrettjonesgoogle, of course! Let me recap:

    From the underlying API perspective there is not much that's expected to happen, and here are the summarized explanations (feel free to edit if I'm still missing something):

    1. Metric vs MetricDescriptor After reading the docs a bit further, the difference between Metric vs MetricDescriptor is clearer. MetricDescriptor is to class what Metric is to object. Labels are the "fields" of the MetricDescriptor.
    2. Metric related methods in TimeSeries: TimeSeries is used in three modes: writing, reading, reading aggregated:
    • TimeSeries.setMetric
      • writing: is there to define the metric similar to the resource, as TimeSeries is a "linking" concept between them in a many-to-many relationship
      • reading: gets set to the metric related to the timeseries
      • reading aggregated: gets set to the metric related to the timeseries, however the kind could be different due to alignment/reduction
    • TimeSeries.setMetricKind:
      • writing: this got introduced later. If the MetricDescriptor doesn't exist, it autocreates it, sets up this metricKind (or the GAUGE by default if this field is missed)
      • reading: returns the MetricDescriptor's kind
      • reading aggregated: returns the aggregated timeseries kind - that might be different from the original MetricDescriptor's
    • TimeSeries.setMetricKindValue: it's the same as setMetricKind, however it's just the way setters for protobuf3 singular enum fields get generated by default

    I accept that even though this terminology and relatively complex setup might not be to my liking but is unlikely to change (or only in next major versions with big features coming in) as it's well established deep in the API design.

    I think that from a google-cloud-java perspective however, this issue still might be useful to highlight opportunities for improving the generator to lower the need for mental load required to learn the API as a Java developer:

    • simplify the exposed methods:
      • don't expose the embedded protobuf message setters: setMetricKindValue vs setMetricKind
      • don't expose protobuf "setField", "setRepeatedField", "setUknownFields"
    • create simpler API for write/read/read aggregate user journeys: I understand that this might be hard to implement with a generator (maybe via custom options?)
  4. yihanzhen commented on Jan 12, 2018

    @yihanzhen
    Contributor

    This has been added to our feature backlog: https://github.com/GoogleCloudPlatform/google-cloud-java/wiki/Feature-backlog . This issue will be closed but is linked in the backlog and can continue to be used for comment and discussion.

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

Metadata

Metadata

Assignees

Labels

api: monitoringIssues related to the Cloud Monitoring API.priority: p2Moderately-important priority. Fix may not be included in next release.type: feature request‘Nice-to-have’ improvement, new feature or different behavior or design.

Type

No type

Projects

No projects

    Milestone

    No milestone

    Relationships

    None yet

    Development

    No branches or pull requests

    Issue actions