census-instrumentation / opencensus-specs

Apache License 2.0
188 stars 50 forks source link

Allow metrics to have string value #209

Open lychung83 opened 5 years ago

lychung83 commented 5 years ago

Currently, opencensus metrics can have only integer and float value. Is there any plan to implement opencensus metric which has value type string (with aggregation type LastValue always)?

(Added on 2018-11-08 by recommendation of @adriancole ) links to the comment describing rationale for the proposal

  1. meet with other metric infrastructure especially stackdriver https://github.com/census-instrumentation/opencensus-specs/issues/209#issuecomment-436897004
  2. our own use case proposal: https://github.com/census-instrumentation/opencensus-specs/issues/209#issuecomment-437211496
bogdandrutu commented 5 years ago

Hi @lychung83,

Can you provide an use case for this? Thanks

lychung83 commented 5 years ago

Just for info, answered in another channel.

odeke-em commented 5 years ago

@lychung83 some of us are not privy to those channels :) so if it is a public use case it would be great to enumerate that here for a discussion.

lychung83 commented 5 years ago

@odeke-em The details are rather draft than private, but the basic idea is following.

We can user opencensus (and already implemented exporter) to export metric to stackdriver, but stackdriver defines more metric value type than opencensus does today. ( see https://cloud.google.com/monitoring/api/ref_v3/rest/v3/projects.metricDescriptors#ValueType )

As a result, if we want to collect and export a string-valued metric to stackdriver using opencensus, currently my best attempt is to make some hacky modification of opencensus or its exporter code directly. I think it's reasonable use case in general, (or maybe because I've seen some other string valued metrics in use) so it can be beneficial to add this feature in opencensus code base.

codefromthecrypt commented 5 years ago

besides pre-existence in stackdriver, can you enumerate a real-world use case, like what an existing customer does with string types?

songy23 commented 5 years ago

Hi @lychung83,

I don't think we would add Aggregation or Measurement support for strings, since indeed there's no meaningful way of "aggregating" strings. With the approach you proposed, the code would look like:

var (
    stringMeasure *stats.StringMeasure
)

stats.Record(ctx, stringMeasure.M("some string"))

Record with string Measurements only works with LastValue aggregation and we need to filter out Views with other aggregations, which I don't think it's the best practice for making APIs.

However, we're currently working on a OpenMetrics data model in all languages which introduced first-class support for Gauges. For string values we can add a GAUGE_STRING type to metrics types, and have some additional APIs (other than the Measurement API) to record gauge string values. Example in Java: DerivedDoubleGauge, DerivedLongGauge, and we can add a similar DerivedStringGauge.

lychung83 commented 5 years ago

Hi @songy23 , to me, restricting aggregation type for certain metric seems ok, but as long as there's a programmatic way of collecting/exporting gauge string via opencensus, I'm totally ok.

lychung83 commented 5 years ago

Hi @adriancole , currently we're trying to collect and report metrics on cloudsql-proxy ( https://github.com/GoogleCloudPlatform/cloudsql-proxy ), a program helps you to connect to google cloud sql databases. (WARNING: everything I'm saying here has no guarantee on any level, and many of them are likely to be changed.)

One of many metrics we define for that proxy client is the version of the proxy client itself. It's a string, gauge type. Of course, adding the version as a field to every metric is possible, but we are proposing to make it as a separate metric because of following reasons.

  1. Since those metric has many other fields and adding many fields consumes considerable amount of stackdriver resource, we'd to like drop unnecessary fields from each metric.
  2. Although version information is really helpful, but this value stays same per single proxy client process, so we have enough information if we report it as a string once. (with other information given.)
codefromthecrypt commented 5 years ago

@lychung83 thanks for the info. I would recommend adding this rationale to the description!

lychung83 commented 5 years ago

@adriancole I updated, but simply links to comments (let's reduce duplicates )