Skip to content

Commit

Permalink
[connectors/spanmetrics] Rename latency histogram metrics to `durat…
Browse files Browse the repository at this point in the history
…ion`. (#19371)
  • Loading branch information
kovrus authored Mar 9, 2023
1 parent 8b5e451 commit e010363
Show file tree
Hide file tree
Showing 6 changed files with 63 additions and 45 deletions.
16 changes: 16 additions & 0 deletions .chloggen/spanmetrics-rename-latency.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,16 @@
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: breaking

# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
component: spanmetricsconnector

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Rename `latency` histogram metrics to `duration`.

# One or more tracking issues related to the change
issues: [19214]

# (Optional) One or more lines of additional information to render under the primary note.
# These lines will be padded with 2 spaces and then inserted directly into the document.
# Use pipe (|) for multiline entries.
subtext:
7 changes: 4 additions & 3 deletions connector/spanmetricsconnector/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ a user wishes to view call counts just on `service.name` and `span.name`.
**Error** counts are computed from the Request counts which have an `Error` Status Code metric dimension.

**Duration** is computed from the difference between the span start and end times and inserted into the
relevant latency histogram time bucket for each unique set dimensions.
relevant duration histogram time bucket for each unique set dimensions.

Each metric will have _at least_ the following dimensions because they are common
across all spans:
Expand All @@ -41,10 +41,11 @@ visit the [Connectors README].
The following settings can be optionally configured:

- `histogram` (default: `explicit_buckets`): Use to configure the type of histogram to record
calculated from spans latency measurements.
calculated from spans duration measurements.
- `unit` (default: `ms`, allowed values: `ms`, `s`): The time unit for recording duration measurements.
calculated from spans duration measurements.
- `explicit`:
- `buckets`: the list of durations defining the latency histogram buckets. Default
- `buckets`: the list of durations defining the duration histogram buckets. Default
buckets: `[2ms, 4ms, 6ms, 8ms, 10ms, 50ms, 100ms, 200ms, 400ms, 800ms, 1s, 1400ms, 2s, 5s, 10s, 15s]`
- `exponential`:
- `max_size` (default: 160) the maximum number of buckets per positive or negative number range.
Expand Down
26 changes: 13 additions & 13 deletions connector/spanmetricsconnector/connector.go
Original file line number Diff line number Diff line change
Expand Up @@ -44,8 +44,8 @@ const (

defaultDimensionsCacheSize = 1000

metricNameLatency = "latency"
metricNameCalls = "calls"
metricNameDuration = "duration"
metricNameCalls = "calls"

defaultUnit = "ms"
)
Expand Down Expand Up @@ -230,25 +230,25 @@ func (p *connectorImp) buildMetrics() pmetric.Metrics {
ilm := m.ResourceMetrics().AppendEmpty().ScopeMetrics().AppendEmpty()
ilm.Scope().SetName("spanmetricsconnector")

p.buildCallsMetrics(ilm)
p.buildLatencyMetrics(ilm)
p.buildCallsMetric(ilm)
p.buildDurationMetric(ilm)

return m
}

// buildCallsMetrics collects the raw call count metrics and builds
// buildDurationMetric collects the raw call count metrics and builds
// a explicit or exponential buckets histogram scope metric.
func (p *connectorImp) buildLatencyMetrics(ilm pmetric.ScopeMetrics) {
func (p *connectorImp) buildDurationMetric(ilm pmetric.ScopeMetrics) {
m := ilm.Metrics().AppendEmpty()
m.SetName(buildMetricName(p.config.Namespace, metricNameLatency))
m.SetName(buildMetricName(p.config.Namespace, metricNameDuration))
m.SetUnit(p.config.Histogram.Unit)

p.histograms.BuildMetrics(m, p.startTimestamp, p.config.GetAggregationTemporality())
}

// buildCallsMetrics collects the raw call count metrics and builds
// buildCallsMetric collects the raw call count metrics and builds
// a sum scope metric.
func (p *connectorImp) buildCallsMetrics(ilm pmetric.ScopeMetrics) {
func (p *connectorImp) buildCallsMetric(ilm pmetric.ScopeMetrics) {
m := ilm.Metrics().AppendEmpty()
m.SetName(buildMetricName(p.config.Namespace, metricNameCalls))

Expand Down Expand Up @@ -291,11 +291,11 @@ func (p *connectorImp) aggregateMetrics(traces ptrace.Traces) {
for k := 0; k < spans.Len(); k++ {
span := spans.At(k)
// Protect against end timestamps before start timestamps. Assume 0 duration.
latency := float64(0)
duration := float64(0)
startTime := span.StartTimestamp()
endTime := span.EndTimestamp()
if endTime > startTime {
latency = float64(endTime-startTime) / float64(unitDivider)
duration = float64(endTime-startTime) / float64(unitDivider)
}
key := p.buildKey(serviceName, span, p.dimensions, resourceAttr)

Expand All @@ -307,9 +307,9 @@ func (p *connectorImp) aggregateMetrics(traces ptrace.Traces) {

// aggregate histogram metrics
h := p.histograms.GetOrCreate(key, attributes)
h.Observe(latency)
h.Observe(duration)
if !span.TraceID().IsEmpty() {
h.AddExemplar(span.TraceID(), span.SpanID(), latency)
h.AddExemplar(span.TraceID(), span.SpanID(), duration)
}

// aggregate sums metrics
Expand Down
41 changes: 21 additions & 20 deletions connector/spanmetricsconnector/connector_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,9 +58,8 @@ const (
regionResourceAttrName = "region"
DimensionsCacheSize = 2

sampleRegion = "us-east-1"
sampleLatency = float64(11)
sampleLatencyDuration = time.Duration(sampleLatency) * time.Millisecond
sampleRegion = "us-east-1"
sampleDuration = float64(11)
)

// metricID represents the minimum attributes that uniquely identifies a metric in our tests.
Expand Down Expand Up @@ -114,7 +113,7 @@ func verifyMultipleCumulativeConsumptions() func(t testing.TB, input pmetric.Met
// This is the best point to verify the computed metrics from spans are as expected.
func verifyConsumeMetricsInput(t testing.TB, input pmetric.Metrics, expectedTemporality pmetric.AggregationTemporality, numCumulativeConsumptions int) bool {
require.Equal(t, 6, input.DataPointCount(),
"Should be 3 for each of call count and latency. Each group of 3 data points is made of: "+
"Should be 3 for each of calls count and duration. Each group of 3 data points is made of: "+
"service-a (server kind) -> service-a (client kind) -> service-b (service kind)",
)

Expand Down Expand Up @@ -144,10 +143,10 @@ func verifyConsumeMetricsInput(t testing.TB, input pmetric.Metrics, expectedTemp
}

h := m.At(1)
assert.Equal(t, metricNameLatency, h.Name())
assert.Equal(t, metricNameDuration, h.Name())
assert.Equal(t, defaultUnit, h.Unit())

// The remaining 3 data points are for latency.
// The remaining 3 data points are for duration.
if h.Type() == pmetric.MetricTypeExponentialHistogram {
hist := h.ExponentialHistogram()
assert.Equal(t, expectedTemporality, hist.AggregationTemporality())
Expand All @@ -167,9 +166,9 @@ func verifyExplicitHistogramDataPoints(t testing.TB, dps pmetric.HistogramDataPo
dp := dps.At(dpi)
assert.Equal(
t,
sampleLatency*float64(numCumulativeConsumptions),
sampleDuration*float64(numCumulativeConsumptions),
dp.Sum(),
"Should be a 11ms latency measurement, multiplied by the number of stateful accumulations.")
"Should be a 11ms duration measurement, multiplied by the number of stateful accumulations.")
assert.NotZero(t, dp.Timestamp(), "Timestamp should be set")

// Verify bucket counts.
Expand All @@ -178,19 +177,19 @@ func verifyExplicitHistogramDataPoints(t testing.TB, dps pmetric.HistogramDataPo
// https://github.com/open-telemetry/opentelemetry-proto/blob/main/opentelemetry/proto/metrics/v1/metrics.proto.
assert.Equal(t, dp.ExplicitBounds().Len()+1, dp.BucketCounts().Len())

// Find the bucket index where the 11ms latency should belong in.
var foundLatencyIndex int
for foundLatencyIndex = 0; foundLatencyIndex < dp.ExplicitBounds().Len(); foundLatencyIndex++ {
if dp.ExplicitBounds().At(foundLatencyIndex) > sampleLatency {
// Find the bucket index where the 11ms duration should belong in.
var foundDurationIndex int
for foundDurationIndex = 0; foundDurationIndex < dp.ExplicitBounds().Len(); foundDurationIndex++ {
if dp.ExplicitBounds().At(foundDurationIndex) > sampleDuration {
break
}
}

// Then verify that all histogram buckets are empty except for the bucket with the 11ms latency.
// Then verify that all histogram buckets are empty except for the bucket with the 11ms duration.
var wantBucketCount uint64
for bi := 0; bi < dp.BucketCounts().Len(); bi++ {
wantBucketCount = 0
if bi == foundLatencyIndex {
if bi == foundDurationIndex {
wantBucketCount = uint64(numCumulativeConsumptions)
}
assert.Equal(t, wantBucketCount, dp.BucketCounts().At(bi))
Expand All @@ -206,9 +205,9 @@ func verifyExponentialHistogramDataPoints(t testing.TB, dps pmetric.ExponentialH
dp := dps.At(dpi)
assert.Equal(
t,
sampleLatency*float64(numCumulativeConsumptions),
sampleDuration*float64(numCumulativeConsumptions),
dp.Sum(),
"Should be a 11ms latency measurement, multiplied by the number of stateful accumulations.")
"Should be a 11ms duration measurement, multiplied by the number of stateful accumulations.")
assert.Equal(t, uint64(numCumulativeConsumptions), dp.Count())
assert.Equal(t, []uint64{uint64(numCumulativeConsumptions)}, dp.Positive().BucketCounts().AsRaw())
assert.NotZero(t, dp.Timestamp(), "Timestamp should be set")
Expand Down Expand Up @@ -261,7 +260,8 @@ func buildBadSampleTrace() ptrace.Traces {
now := time.Now()
// Flipping timestamp for a bad duration
span.SetEndTimestamp(pcommon.NewTimestampFromTime(now))
span.SetStartTimestamp(pcommon.NewTimestampFromTime(now.Add(sampleLatencyDuration)))
span.SetStartTimestamp(
pcommon.NewTimestampFromTime(now.Add(time.Duration(sampleDuration) * time.Millisecond)))
return badTrace
}

Expand Down Expand Up @@ -323,7 +323,8 @@ func initSpan(span span, s ptrace.Span) {
s.Status().SetCode(span.statusCode)
now := time.Now()
s.SetStartTimestamp(pcommon.NewTimestampFromTime(now))
s.SetEndTimestamp(pcommon.NewTimestampFromTime(now.Add(sampleLatencyDuration)))
s.SetEndTimestamp(
pcommon.NewTimestampFromTime(now.Add(time.Duration(sampleDuration) * time.Millisecond)))

s.Attributes().PutStr(stringAttrName, "stringAttrValue")
s.Attributes().PutInt(intAttrName, 99)
Expand Down Expand Up @@ -859,8 +860,8 @@ func TestConnectorConsumeTracesEvictedCacheKey(t *testing.T) {
mcon := &mocks.MetricsConsumer{}

wantDataPointCounts := []int{
6, // (calls + latency) * (service-a + service-b + service-c)
4, // (calls + latency) * (service-b + service-c)
6, // (calls + duration) * (service-a + service-b + service-c)
4, // (calls + duration) * (service-b + service-c)
}

// Ensure the assertion that wantDataPointCounts is performed only after all ConsumeMetrics
Expand Down
16 changes: 8 additions & 8 deletions connector/spanmetricsconnector/factory_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,18 +29,18 @@ func TestNewConnector(t *testing.T) {
defaultMethod := "GET"
defaultMethodValue := pcommon.NewValueStr(defaultMethod)
for _, tc := range []struct {
name string
latencyHistogramBuckets []time.Duration
dimensions []Dimension
wantLatencyHistogramBuckets []float64
wantDimensions []dimension
name string
durationHistogramBuckets []time.Duration
dimensions []Dimension
wantDurationHistogramBuckets []float64
wantDimensions []dimension
}{
{
name: "simplest config (use defaults)",
},
{
name: "1 configured latency histogram bucket should result in 1 explicit latency bucket (+1 implicit +Inf bucket)",
latencyHistogramBuckets: []time.Duration{2 * time.Millisecond},
name: "1 configured duration histogram bucket should result in 1 explicit duration bucket (+1 implicit +Inf bucket)",
durationHistogramBuckets: []time.Duration{2 * time.Millisecond},
dimensions: []Dimension{
{Name: "http.method", Default: &defaultMethod},
{Name: "http.status_code"},
Expand All @@ -58,7 +58,7 @@ func TestNewConnector(t *testing.T) {
creationParams := connectortest.NewNopCreateSettings()
cfg := factory.CreateDefaultConfig().(*Config)
cfg.Histogram.Explicit = &ExplicitHistogramConfig{
Buckets: tc.latencyHistogramBuckets,
Buckets: tc.durationHistogramBuckets,
}
cfg.Dimensions = tc.dimensions

Expand Down
2 changes: 1 addition & 1 deletion connector/spanmetricsconnector/internal/metrics/metrics.go
Original file line number Diff line number Diff line change
Expand Up @@ -215,7 +215,7 @@ func (h *explicitHistogram) Observe(value float64) {
h.sum += value
h.count++

// Binary search to find the latencyMs bucket index.
// Binary search to find the value bucket index.
index := sort.SearchFloat64s(h.bounds, value)
h.bucketCounts[index]++
}
Expand Down

0 comments on commit e010363

Please sign in to comment.