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.

Sign up for GitHub

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

[coordinator] Store metrics type into the annotation #2628

Merged
merged 18 commits into from
Oct 6, 2020
Merged

[coordinator] Store metrics type into the annotation #2628

Show file tree
Hide file tree
Changes from 6 commits
Commits
Show all changes
18 commits
Select commit Hold shift + click to select a range
78783c8
Rename existing protobuf TimeSeries.type field to m3_type to avoid co…
linasm Sep 10, 2020
cc62479
Add new Prometheus protobuf fields
linasm Sep 10, 2020
27f7f23
Rename the internal MetricsType to M3MetricsType
linasm Sep 10, 2020
62f4bfd
Implement conversions
linasm Sep 10, 2020
06d5f81
Write metrics type as annotation payload
linasm Sep 11, 2020
aeb8036
Merge branch 'master' into linasm/metrics-type-metadata
linasm Sep 11, 2020
521a732
Avoid reusing annotation slices
linasm Sep 23, 2020
ced1245
Merge branch 'master' into linasm/metrics-type-metadata
linasm Sep 23, 2020
08d50fb
Fix test
linasm Sep 23, 2020
d108b4f
Introduce metric family type
linasm Sep 25, 2020
7cb1bf6
Merge branch 'master' into linasm/metrics-type-metadata
linasm Sep 25, 2020
70124bf
Address review feedback
linasm Oct 1, 2020
7e1ea81
Merge branch 'master' into linasm/metrics-type-metadata
linasm Oct 1, 2020
a287fba
Merge branch 'master' into linasm/metrics-type-metadata
linasm Oct 5, 2020
77753b9
Revert "Introduce metric family type"
linasm Oct 5, 2020
1466bf7
Introduce annotation.Payload.handle_value_resets field
linasm Oct 5, 2020
b4dc990
Merge branch 'master' into linasm/metrics-type-metadata
linasm Oct 6, 2020
3eb2ad0
Minor changes according to PR feedback
linasm Oct 6, 2020
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
2 changes: 1 addition & 1 deletion src/cmd/services/m3coordinator/downsample/downsampler.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,7 +58,7 @@ type SamplesAppenderResult struct {
type SampleAppenderOptions struct {
Override bool
OverrideRules SamplesAppenderOverrideRules
MetricType ts.MetricType
MetricType ts.M3MetricType
Copy link
Collaborator

Choose a reason for hiding this comment

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

Can we keep this as MetricType

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Well, we can. But I renamed it in order to match PromMetricType in https://github.com/m3db/m3/pull/2628/files/aeb8036681303b0de6312eb47006d49f59a4a631#diff-0535bd9002d5b34759706aebeec1ee46
I believe having PromMetricType and just MetricType at the same scope/level would be confusing.

Copy link
Collaborator

Choose a reason for hiding this comment

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

My thinking was that MetricType is implicitly scoped to M3MetricType just because we're in the M3 project, so all types can be considered M3Type by default

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I wish it were so simple :) Unfortunately we bring in a protobuf with a different MetricType from Prometheus project which makes this whole point moot.

}

// SamplesAppenderOverrideRules provides override rules to
Expand Down
4 changes: 2 additions & 2 deletions src/cmd/services/m3coordinator/downsample/downsampler_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -382,7 +382,7 @@ func TestDownsamplerAggregationWithRulesConfigMappingRulesTypeFilter(t *testing.
},
},
sampleAppenderOpts: &SampleAppenderOptions{
MetricType: ts.MetricTypeCounter,
MetricType: ts.M3MetricTypeCounter,
},
ingest: &testDownsamplerOptionsIngest{
gaugeMetrics: []testGaugeMetric{gaugeMetric},
Expand Down Expand Up @@ -436,7 +436,7 @@ func TestDownsamplerAggregationWithRulesConfigMappingRulesTypeFilterNoMatch(t *t
},
},
sampleAppenderOpts: &SampleAppenderOptions{
MetricType: ts.MetricTypeGauge,
MetricType: ts.M3MetricTypeGauge,
},
ingest: &testDownsamplerOptionsIngest{
gaugeMetrics: []testGaugeMetric{gaugeMetric},
Expand Down
8 changes: 4 additions & 4 deletions src/cmd/services/m3coordinator/downsample/metrics_appender.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -153,13 +153,13 @@ func (a *metricsAppender) SamplesAppender(opts SampleAppenderOptions) (SamplesAp
if a.augmentM3Tags {
// NB (@shreyas): Add the metric type tag. The tag has the prefix
// __m3_. All tags with that prefix are only used for the purpose of
// filter matchiand then stripped off before we actually send to the aggregator.
// filter match and then stripped off before we actually send to the aggregator.
switch opts.MetricType {
case ts.MetricTypeCounter:
case ts.M3MetricTypeCounter:
tags.append(metric.M3TypeTag, metric.M3CounterValue)
case ts.MetricTypeGauge:
case ts.M3MetricTypeGauge:
tags.append(metric.M3TypeTag, metric.M3GaugeValue)
case ts.MetricTypeTimer:
case ts.M3MetricTypeTimer:
tags.append(metric.M3TypeTag, metric.M3TimerValue)
}
}
Expand Down
10 changes: 5 additions & 5 deletions src/cmd/services/m3coordinator/ingest/write.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -462,7 +462,7 @@ func (d *downsamplerAndWriter) writeAggregatedBatch(
}

opts := downsample.SampleAppenderOptions{
MetricType: value.Attributes.Type,
MetricType: value.Attributes.M3Type,
}
if downsampleMappingRuleOverrides, ok := d.downsampleOverrideRules(overrides); ok {
opts = downsample.SampleAppenderOptions{
Expand All @@ -484,12 +484,12 @@ func (d *downsamplerAndWriter) writeAggregatedBatch(
}

for _, dp := range value.Datapoints {
switch value.Attributes.Type {
case ts.MetricTypeGauge:
switch value.Attributes.M3Type {
case ts.M3MetricTypeGauge:
err = result.SamplesAppender.AppendGaugeTimedSample(dp.Timestamp, dp.Value)
case ts.MetricTypeCounter:
case ts.M3MetricTypeCounter:
err = result.SamplesAppender.AppendCounterTimedSample(dp.Timestamp, int64(dp.Value))
case ts.MetricTypeTimer:
case ts.M3MetricTypeTimer:
err = result.SamplesAppender.AppendTimerTimedSample(dp.Timestamp, dp.Value)
}
if err != nil {
Expand Down
10 changes: 5 additions & 5 deletions src/cmd/services/m3coordinator/ingest/write_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,13 +129,13 @@ var (
testAnnotation2 = []byte("second")

testAttributesGauge = ts.SeriesAttributes{
Type: ts.MetricTypeGauge,
M3Type: ts.M3MetricTypeGauge,
}
testAttributesCounter = ts.SeriesAttributes{
Type: ts.MetricTypeCounter,
M3Type: ts.M3MetricTypeCounter,
}
testAttributesTimer = ts.SeriesAttributes{
Type: ts.MetricTypeTimer,
M3Type: ts.M3MetricTypeTimer,
}

testEntries = []testIterEntry{
Expand Down Expand Up @@ -565,11 +565,11 @@ func TestDownsampleAndWriteBatchDifferentTypes(t *testing.T) {

mockMetricsAppender.
EXPECT().
SamplesAppender(downsample.SampleAppenderOptions{MetricType: ts.MetricTypeCounter}).
SamplesAppender(downsample.SampleAppenderOptions{MetricType: ts.M3MetricTypeCounter}).
Return(downsample.SamplesAppenderResult{SamplesAppender: mockSamplesAppender}, nil).Times(1)
mockMetricsAppender.
EXPECT().
SamplesAppender(downsample.SampleAppenderOptions{MetricType: ts.MetricTypeTimer}).
SamplesAppender(downsample.SampleAppenderOptions{MetricType: ts.M3MetricTypeTimer}).
Return(downsample.SamplesAppenderResult{SamplesAppender: mockSamplesAppender}, nil).Times(1)
for _, tag := range testTags1.Tags {
mockMetricsAppender.EXPECT().AddTag(tag.Name, tag.Value)
Expand Down
Loading