-
Notifications
You must be signed in to change notification settings - Fork 184
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
feat/add histogram aggregator in defaultAggregator #226
feat/add histogram aggregator in defaultAggregator #226
Conversation
Signed-off-by: niejiangang <niejiangang@harmonycloud.cn>
Signed-off-by: niejiangang <niejiangang@harmonycloud.cn>
Signed-off-by: niejiangang <niejiangang@harmonycloud.cn>
Signed-off-by: niejiangang <niejiangang@harmonycloud.cn>
Signed-off-by: niejiangang <niejiangang@harmonycloud.cn>
Signed-off-by: niejiangang <niejiangang@harmonycloud.cn>
Signed-off-by: niejiangang <niejiangang@harmonycloud.cn>
The raw struct of Gauge is type Gauge struct {
Name string
Value int64
} The new struct of Gauge is type Gauge struct {
Name string
// Data can be assigned by:
// Int
// Histogram
Data isGaugeData
}
type Int struct {
Value int64
}
type Histogram struct {
Sum int64
Count uint64
ExplicitBoundaries []int64
BucketCounts []uint64
}
type isGaugeData interface {
isGaugeData()
}
func (*Gauge_Int) isGaugeData() {}
func (*Gauge_Histogram) isGaugeData() {}
type Gauge_Int struct {
Int *Int
}
type Gauge_Histogram struct {
Histogram *Histogram
} The implementation details of this structure refer to the |
Signed-off-by: niejiangang <niejiangang@harmonycloud.cn>
Signed-off-by: niejiangang <niejiangang@harmonycloud.cn>
1. rename `GaugeGroup` to `DataGroup` 2. rename `Gauge` to `Metric` Signed-off-by: niejiangang <niejiangang@harmonycloud.cn>
Signed-off-by: niejiangang <niejiangang@harmonycloud.cn>
Signed-off-by: niejiangang <niejiangang@harmonycloud.cn>
Signed-off-by: niejiangang <niejiangang@harmonycloud.cn>
Signed-off-by: niejiangang <niejiangang@harmonycloud.cn>
Signed-off-by: niejiangang <niejiangang@harmonycloud.cn>
d1e8462
to
f5a6075
Compare
…c in constvalues Signed-off-by: niejiangang <niejiangang@harmonycloud.cn>
Signed-off-by: niejiangang <niejiangang@harmonycloud.cn>
Signed-off-by: niejiangang <niejiangang@harmonycloud.cn>
@@ -6,20 +6,20 @@ import ( | |||
"sync" | |||
) | |||
|
|||
var otelexporterGaugegroupsReceivedTotal = "kindling_telemetry_otelexporter_gaugegroups_received_total" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Here I changed the self-telemetry metric, will this influence other components?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No.
Signed-off-by: niejiangang <niejiangang@harmonycloud.cn>
Because we have started to use a stand-alone aggregator instead of OTLP-SDK, some functions that were originally implemented by the OTLP-SDK aggregator are gradually being moved to the stand-alone aggregator. Histogram aggregation is one of them and will be needed in standard version of kindling
During this process, I ran into some problems and decided to make some modifications to the
model.Gauge
structureReasons for changing the model of Gauge
Reasons for keeping the model of Gauge
Because implementing the Histogram based on modifying the Gauge structure is far simpler than not modifying it. I provided this PR which provides a simple implementation to illustrate the impact of Gauge changes on other components. At the same time, it has been able to support the aggregation of Histogram type data.
Description
Motivation and Context
The existing performance of the aggregator based on the OTLP-SDK cannot support our needs, and our new aggregate processor can not support Histogram.
How Has This Been Tested?
cd collector && go test -v github.com/Kindling-project/kindling/collector/pkg/aggregator/defaultaggregator
=== RUN Test_aggValues_sum
--- PASS: Test_aggValues_sum (0.00s)
=== RUN Test_aggValues_sum_histogram
--- PASS: Test_aggValues_sum_histogram (0.00s)
=== RUN Test_aggValues_max
--- PASS: Test_aggValues_max (0.01s)
=== RUN Test_aggValues_max_histogram
--- PASS: Test_aggValues_max_histogram (0.00s)
=== RUN Test_aggValues_avg
--- PASS: Test_aggValues_avg (0.01s)
=== RUN Test_aggValues_avg_histogram
--- PASS: Test_aggValues_avg_histogram (0.01s)
=== RUN Test_aggValues_last
--- PASS: Test_aggValues_last (0.00s)
=== RUN Test_aggValues_last_histogram
--- PASS: Test_aggValues_last_histogram (0.00s)
=== RUN Test_aggValues_count
--- PASS: Test_aggValues_count (0.00s)
=== RUN Test_aggValues_count_histogram
--- PASS: Test_aggValues_count_histogram (0.00s)
=== RUN Test_aggValues_histogram
--- PASS: Test_aggValues_histogram (0.01s)
=== RUN Test_aggValues_histogram_histogram
--- PASS: Test_aggValues_histogram_histogram (0.01s)
=== RUN Test_defaultValuesMap_sum
--- PASS: Test_defaultValuesMap_sum (0.00s)
=== RUN Test_defaultValuesMap_avg
--- PASS: Test_defaultValuesMap_avg (0.00s)
=== RUN Test_defaultValuesMap_max
--- PASS: Test_defaultValuesMap_max (0.00s)
=== RUN Test_defaultValuesMap_lastValue
--- PASS: Test_defaultValuesMap_lastValue (0.00s)
=== RUN Test_defaultValuesMap_countValue
--- PASS: Test_defaultValuesMap_countValue (0.00s)
=== RUN Test_defaultValuesMap_histogramValue
--- PASS: Test_defaultValuesMap_histogramValue (0.00s)
=== RUN TestConcurrentAggregator
--- PASS: TestConcurrentAggregator (0.56s)
=== RUN TestRecord
--- PASS: TestRecord (0.00s)
PASS