Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

data model proto #126

Merged
merged 6 commits into from
Nov 5, 2020
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions proto/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
# NB(rob): Move this to top level Makefile once merged to master,
# then can add a rule to CircleCI to build the protobuf to make sure
# it is valid.
%.pb.go: %.proto
protoc --go_out=. $<

all: $(patsubst %.proto,%.pb.go,$(wildcard *.proto))
214 changes: 214 additions & 0 deletions proto/openmetrics_data_model.proto
Original file line number Diff line number Diff line change
@@ -0,0 +1,214 @@
syntax = "proto3";

// The OpenMetrics protobuf schema which defines the protobuf wire format.
// Ensure to interpret "required" as semantically required for a valid message.
// All string fields MUST be UTF-8 encoded strings.
package openmetrics;

import "google/protobuf/timestamp.proto";

// The top-level container type that is encoded and sent over the wire.
message MetricSet {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Probably a better name is MetricList. Set semantics are a bit unclear here.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's a set, you aren't allowed have duplicates.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then probably document that, to make it clear why is a set and what is the "hash" function.

// Each MetricPoints has one or more points for a single metric.
repeated MetricFamily metric_families = 1;
}

// One or more metrics for a single metric family, where each metric
// has one or more points.
message MetricFamily {
// Required.
string name = 1;

// Optional.
MetricType type = 2;

// Optional.
string unit = 3;

// Optional.
string help = 4;

// Optional.
repeated Metric metrics = 5;
}

// The type of a metric.
enum MetricType {
// Unknown must use unknown point values.
UNKNOWN = 0;
// Gauge must use gauge point values.
GAUGE = 1;
// Counter must use counter point values.
COUNTER = 2;
// State set must use state set value points.
STATE_SET = 3;
// Info must use info value points.
INFO = 4;
// Histogram must use histogram value points.
HISTOGRAM = 5;
// Gauge histogram must use histogram value points.
GAUGE_HISTOGRAM = 6;
// Summary quantiles must use summary value points.
SUMMARY = 7;
}

// A single metric with a unique set of labels within a metric family.
message Metric {
// Optional.
repeated Label labels = 1;

// Optional.
repeated Point points = 2;
}

// A name-value pair. These are used in multiple places: identifying
// timeseries, value of INFO metrics, and exemplars in Histograms.
message Label {

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because label is key, value you can use Labels as map<String,String>.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment should be ignored and marked as resolved.

// Required.
string name = 1;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do we want to support a label description? Human readable description of this metric?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We discussed this previously, and no.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

roger that.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment should be marked as resolved.


// Required.
string value = 2;
}

// A point in a metric.
message Point {
// Required.
oneof value {
UnknownValue unknown_value = 1;
GaugeValue gauge_value = 2;
CounterValue counter_value = 3;
HistogramValue histogram_value = 4;
StateSetValue state_set_value = 5;
InfoValue info_value = 6;
SummaryValue summary_value = 7;
}

// Optional.
google.protobuf.Timestamp timestamp = 8;
}

// Value for UNKNOWN point.
message UnknownValue {
// Required.
oneof value {
double double_value = 1;
int64 int_value = 2;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hmm, did we agree uint64 here?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be a little strange that double value can be negative but int value cannot be?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason to restrict it to unsigned? The proto doc has int64

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason this cannot be negative for a gauge?

}
}

// Value for GAUGE point.
message GaugeValue {
// Required.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Neither of these should be required

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why not?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Some systems don't have them (MySQL)

oneof value {
double double_value = 1;
int64 int_value = 2;
}
}

// Value for COUNTER point.
message CounterValue {
// Required.
oneof total {
double double_value = 1;
uint64 int_value = 2;
}

// The time values began being collected for this counter.
// Optional.
google.protobuf.Timestamp created = 3;

// Optional.
Exemplar exemplar = 4;
}

// Value for HISTOGRAM or GAUGE_HISTOGRAM point.
message HistogramValue {
// Optional.
oneof sum {
double double_value = 1;
int64 int_value = 2;
}

// Optional.
uint64 count = 3;

// The time values began being collected for this histogram.
// Optional.
google.protobuf.Timestamp created = 4;

// Optional.
repeated Bucket buckets = 5;

// Bucket is the number of values for a bucket in the histogram
// with an optional exemplar.
message Bucket {
// Required.
uint64 count = 1;

// Optional.
double upper_bound = 2;

// Optional.
Exemplar exemplar = 3;
}
}

message Exemplar {
// Required.
double value = 1;

// Optional.
google.protobuf.Timestamp timestamp = 2;

// Labels are additional information about the exemplar value (e.g. trace id).
// Optional.
repeated Label label = 3;
}

// Value for STATE_SET point.
message StateSetValue {
// Optional.
repeated State states = 1;

message State {
// Required.
bool enabled = 1;

// Required.
string name = 2;

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this not a regular label? Recommended key can be state_name the label value is the state then this becomes a single boolean.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Then it'd not be a first-class type.

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am fine not having this as a first class. Probably on the gauge supporting boolean is enough

}
}

// Value for INFO point.
message InfoValue {
// Optional.
repeated Label info = 1;
}

// Value for SUMMARY point.
message SummaryValue {
// Optional.
oneof sum {
double double_value = 1;
int64 int_value = 2;
}

// Optional.
uint64 count = 2;

// The time sum and count values began being collected for this summary.
// Optional.
google.protobuf.Timestamp created = 3;

// Optional.
repeated Quantile quantile = 4;

message Quantile {
// Required.
double quantile = 1;

// Required.
double value = 2;
}
}