Skip to content

Commit

Permalink
Remove common labels from MetricDescriptor (#144)
Browse files Browse the repository at this point in the history
Resolves: #141

Metric proto previously allowed Labels in 2 places: in the MetricDescriptor and in each DataPoint.

The semantics is that Labels in MetricDescriptor apply to all DataPoints.
This requires complicated and expensive merging of Labels maps (which
possibly is made even more expensive since they are stored as lists,
requiring either intermediary data structures or O(nm) merging) when performing
translations from OTLP to other formats that have no equivalent notion of
having label keys per data point and per group of data points (e.g. OpenCensus).

This commit removes Labels from MetricDescriptor. My understanding is that
there is currently no client library that emits MetricDescriptor with labels
so this is not used and can be safely removed.

This will simplify and make faster translation code in the Collector.

If this is needed we can think carefully about adding it back after there
is a clear justification and tradeoffs are well understood.

Co-authored-by: Bogdan Drutu <bogdandrutu@gmail.com>
  • Loading branch information
tigrannajaryan and bogdandrutu committed Apr 27, 2020
1 parent ba6a259 commit 3a34ab5
Show file tree
Hide file tree
Showing 2 changed files with 62 additions and 76 deletions.
134 changes: 62 additions & 72 deletions gen/go/metrics/v1/metrics.pb.go

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 0 additions & 4 deletions opentelemetry/proto/metrics/v1/metrics.proto
Original file line number Diff line number Diff line change
Expand Up @@ -171,10 +171,6 @@ message MetricDescriptor {
SUMMARY = 7;
}
Type type = 4;

// The set of labels associated with the metric descriptor. Labels in this list apply to
// all data points.
repeated opentelemetry.proto.common.v1.StringKeyValue labels = 5;
}

// Int64DataPoint is a single data point in a timeseries that describes the time-varying
Expand Down

0 comments on commit 3a34ab5

Please sign in to comment.