Skip to content

Commit c2fe2d2

Browse files
Frapschenf7o
Frapschen
authored and
f7o
committed
[connector/servicegraph]Fix incorrectly reversed latency settings(resolve open-telemetry#34562 unit fail) (open-telemetry#34933)
**Description:** <Describe what has changed.> This PR resolves the open-telemetry#34562 unit failure and adds back this bugfix after revert open-telemetry#34865.
1 parent 395ce8f commit c2fe2d2

File tree

5 files changed

+113
-87
lines changed

5 files changed

+113
-87
lines changed

‎.chloggen/fix-wrong-latency.yaml

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,27 @@
1+
# Use this changelog template to create an entry for release notes.
2+
3+
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
4+
change_type: bug_fix
5+
6+
# The name of the component, or a single word describing the area of concern, (e.g. filelogreceiver)
7+
component: servicegraphconnector
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Fix incorrectly reversed latency settings for the server and client
11+
12+
# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
13+
issues: [34933]
14+
15+
# (Optional) One or more lines of additional information to render under the primary note.
16+
# These lines will be padded with 2 spaces and then inserted directly into the document.
17+
# Use pipe (|) for multiline entries.
18+
subtext:
19+
20+
# If your change doesn't affect end users or the exported elements of any package,
21+
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
22+
# Optional: The change log or logs in which this entry should be included.
23+
# e.g. '[user]' or '[user, api]'
24+
# Include 'user' if the change is relevant to end users.
25+
# Include 'api' if there is a change to a library API.
26+
# Default: '[user]'
27+
change_logs: []

‎connector/servicegraphconnector/connector.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -545,9 +545,9 @@ func (p *serviceGraphConnector) collectClientLatencyMetrics(ilm pmetric.ScopeMet
545545
dpDuration.SetStartTimestamp(pcommon.NewTimestampFromTime(p.startTime))
546546
dpDuration.SetTimestamp(timestamp)
547547
dpDuration.ExplicitBounds().FromRaw(p.reqDurationBounds)
548-
dpDuration.BucketCounts().FromRaw(p.reqServerDurationSecondsBucketCounts[key])
549-
dpDuration.SetCount(p.reqServerDurationSecondsCount[key])
550-
dpDuration.SetSum(p.reqServerDurationSecondsSum[key])
548+
dpDuration.BucketCounts().FromRaw(p.reqClientDurationSecondsBucketCounts[key])
549+
dpDuration.SetCount(p.reqClientDurationSecondsCount[key])
550+
dpDuration.SetSum(p.reqClientDurationSecondsSum[key])
551551

552552
// TODO: Support exemplars
553553
dimensions, ok := p.dimensionsForSeries(key)
@@ -575,9 +575,9 @@ func (p *serviceGraphConnector) collectServerLatencyMetrics(ilm pmetric.ScopeMet
575575
dpDuration.SetStartTimestamp(pcommon.NewTimestampFromTime(p.startTime))
576576
dpDuration.SetTimestamp(timestamp)
577577
dpDuration.ExplicitBounds().FromRaw(p.reqDurationBounds)
578-
dpDuration.BucketCounts().FromRaw(p.reqClientDurationSecondsBucketCounts[key])
579-
dpDuration.SetCount(p.reqClientDurationSecondsCount[key])
580-
dpDuration.SetSum(p.reqClientDurationSecondsSum[key])
578+
dpDuration.BucketCounts().FromRaw(p.reqServerDurationSecondsBucketCounts[key])
579+
dpDuration.SetCount(p.reqServerDurationSecondsCount[key])
580+
dpDuration.SetSum(p.reqServerDurationSecondsSum[key])
581581

582582
// TODO: Support exemplars
583583
dimensions, ok := p.dimensionsForSeries(key)

‎connector/servicegraphconnector/connector_test.go

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -81,7 +81,7 @@ func TestConnectorConsume(t *testing.T) {
8181
},
8282
},
8383
sampleTraces: buildSampleTrace(t, "val"),
84-
verifyMetrics: verifyHappyCaseMetrics,
84+
verifyMetrics: verifyHappyCaseMetricsWithDuration(2, 1),
8585
},
8686
{
8787
name: "test fix failed label not work",
@@ -163,7 +163,7 @@ func TestConnectorConsume(t *testing.T) {
163163
},
164164
sampleTraces: buildSampleTrace(t, "val"),
165165
gates: []*featuregate.Gate{legacyLatencyUnitMsFeatureGate},
166-
verifyMetrics: verifyHappyCaseMetricsWithDuration(1000),
166+
verifyMetrics: verifyHappyCaseMetricsWithDuration(2000, 1000),
167167
},
168168
} {
169169
t.Run(tc.name, func(t *testing.T) {
@@ -209,11 +209,7 @@ func getGoldenTraces(t *testing.T, file string) ptrace.Traces {
209209
return td
210210
}
211211

212-
func verifyHappyCaseMetrics(t *testing.T, md pmetric.Metrics) {
213-
verifyHappyCaseMetricsWithDuration(1)(t, md)
214-
}
215-
216-
func verifyHappyCaseMetricsWithDuration(durationSum float64) func(t *testing.T, md pmetric.Metrics) {
212+
func verifyHappyCaseMetricsWithDuration(serverDurationSum, clientDurationSum float64) func(t *testing.T, md pmetric.Metrics) {
217213
return func(t *testing.T, md pmetric.Metrics) {
218214
assert.Equal(t, 3, md.MetricCount())
219215

@@ -231,11 +227,11 @@ func verifyHappyCaseMetricsWithDuration(durationSum float64) func(t *testing.T,
231227

232228
mServerDuration := ms.At(1)
233229
assert.Equal(t, "traces_service_graph_request_server_seconds", mServerDuration.Name())
234-
verifyDuration(t, mServerDuration, durationSum)
230+
verifyDuration(t, mServerDuration, serverDurationSum, []uint64{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0})
235231

236232
mClientDuration := ms.At(2)
237233
assert.Equal(t, "traces_service_graph_request_client_seconds", mClientDuration.Name())
238-
verifyDuration(t, mClientDuration, durationSum)
234+
verifyDuration(t, mClientDuration, clientDurationSum, []uint64{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0})
239235
}
240236
}
241237

@@ -259,16 +255,16 @@ func verifyCount(t *testing.T, m pmetric.Metric) {
259255
verifyAttr(t, attributes, "client_some-attribute", "val")
260256
}
261257

262-
func verifyDuration(t *testing.T, m pmetric.Metric, durationSum float64) {
258+
func verifyDuration(t *testing.T, m pmetric.Metric, durationSum float64, bs []uint64) {
263259
assert.Equal(t, pmetric.MetricTypeHistogram, m.Type())
264260
dps := m.Histogram().DataPoints()
265261
assert.Equal(t, 1, dps.Len())
266262

267263
dp := dps.At(0)
268-
assert.Equal(t, durationSum, dp.Sum()) // Duration: 1sec
264+
assert.Equal(t, durationSum, dp.Sum()) // Duration: client is 1sec, server is 2sec
269265
assert.Equal(t, uint64(1), dp.Count())
270266
buckets := pcommon.NewUInt64Slice()
271-
buckets.FromRaw([]uint64{0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 1, 0, 0, 0, 0, 0, 0})
267+
buckets.FromRaw(bs)
272268
assert.Equal(t, buckets, dp.BucketCounts())
273269

274270
attributes := dp.Attributes()
@@ -287,7 +283,10 @@ func verifyAttr(t *testing.T, attrs pcommon.Map, k, expected string) {
287283

288284
func buildSampleTrace(t *testing.T, attrValue string) ptrace.Traces {
289285
tStart := time.Date(2022, 1, 2, 3, 4, 5, 6, time.UTC)
290-
tEnd := time.Date(2022, 1, 2, 3, 4, 6, 6, time.UTC)
286+
// client: 1s
287+
cEnd := time.Date(2022, 1, 2, 3, 4, 6, 6, time.UTC)
288+
// server: 2s
289+
sEnd := time.Date(2022, 1, 2, 3, 4, 7, 6, time.UTC)
291290

292291
traces := ptrace.NewTraces()
293292

@@ -312,7 +311,7 @@ func buildSampleTrace(t *testing.T, attrValue string) ptrace.Traces {
312311
clientSpan.SetTraceID(traceID)
313312
clientSpan.SetKind(ptrace.SpanKindClient)
314313
clientSpan.SetStartTimestamp(pcommon.NewTimestampFromTime(tStart))
315-
clientSpan.SetEndTimestamp(pcommon.NewTimestampFromTime(tEnd))
314+
clientSpan.SetEndTimestamp(pcommon.NewTimestampFromTime(cEnd))
316315
clientSpan.Attributes().PutStr("some-attribute", attrValue) // Attribute selected as dimension for metrics
317316
serverSpan := scopeSpans.Spans().AppendEmpty()
318317
serverSpan.SetName("server span")
@@ -321,7 +320,7 @@ func buildSampleTrace(t *testing.T, attrValue string) ptrace.Traces {
321320
serverSpan.SetParentSpanID(clientSpanID)
322321
serverSpan.SetKind(ptrace.SpanKindServer)
323322
serverSpan.SetStartTimestamp(pcommon.NewTimestampFromTime(tStart))
324-
serverSpan.SetEndTimestamp(pcommon.NewTimestampFromTime(tEnd))
323+
serverSpan.SetEndTimestamp(pcommon.NewTimestampFromTime(sEnd))
325324

326325
return traces
327326
}

‎connector/servicegraphconnector/testdata/virtual-node-label-client-expected-metrics.yaml

Lines changed: 64 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -32,74 +32,74 @@ resourceMetrics:
3232
- histogram:
3333
aggregationTemporality: 2
3434
dataPoints:
35-
- attributes:
36-
- key: client
37-
value:
38-
stringValue: user
39-
- key: connection_type
40-
value:
41-
stringValue: virtual_node
42-
- key: failed
43-
value:
44-
boolValue: false
45-
- key: server
46-
value:
47-
stringValue: bar-requester
48-
- key: server_peer.service
49-
value:
50-
stringValue: external-platform
51-
- key: virtual_node
52-
value:
53-
stringValue: client
54-
bucketCounts:
55-
- "1"
56-
- "0"
57-
- "0"
58-
- "0"
59-
count: "1"
60-
explicitBounds:
61-
- 0.1
62-
- 1
63-
- 10
64-
startTimeUnixNano: "1000000"
65-
sum: 0.000000
66-
timeUnixNano: "2000000"
35+
- attributes:
36+
- key: client
37+
value:
38+
stringValue: user
39+
- key: connection_type
40+
value:
41+
stringValue: virtual_node
42+
- key: failed
43+
value:
44+
boolValue: false
45+
- key: server
46+
value:
47+
stringValue: bar-requester
48+
- key: server_peer.service
49+
value:
50+
stringValue: external-platform
51+
- key: virtual_node
52+
value:
53+
stringValue: client
54+
bucketCounts:
55+
- "1"
56+
- "0"
57+
- "0"
58+
- "0"
59+
count: "1"
60+
explicitBounds:
61+
- 0.1
62+
- 1
63+
- 10
64+
startTimeUnixNano: "1000000"
65+
sum: 1e-06
66+
timeUnixNano: "2000000"
6767
name: traces_service_graph_request_server_seconds
6868
- histogram:
6969
aggregationTemporality: 2
7070
dataPoints:
71-
- attributes:
72-
- key: client
73-
value:
74-
stringValue: user
75-
- key: connection_type
76-
value:
77-
stringValue: virtual_node
78-
- key: failed
79-
value:
80-
boolValue: false
81-
- key: server
82-
value:
83-
stringValue: bar-requester
84-
- key: server_peer.service
85-
value:
86-
stringValue: external-platform
87-
- key: virtual_node
88-
value:
89-
stringValue: client
90-
bucketCounts:
91-
- "1"
92-
- "0"
93-
- "0"
94-
- "0"
95-
count: "1"
96-
explicitBounds:
97-
- 0.1
98-
- 1
99-
- 10
100-
startTimeUnixNano: "1000000"
101-
sum: 0.000001
102-
timeUnixNano: "2000000"
71+
- attributes:
72+
- key: client
73+
value:
74+
stringValue: user
75+
- key: connection_type
76+
value:
77+
stringValue: virtual_node
78+
- key: failed
79+
value:
80+
boolValue: false
81+
- key: server
82+
value:
83+
stringValue: bar-requester
84+
- key: server_peer.service
85+
value:
86+
stringValue: external-platform
87+
- key: virtual_node
88+
value:
89+
stringValue: client
90+
bucketCounts:
91+
- "1"
92+
- "0"
93+
- "0"
94+
- "0"
95+
count: "1"
96+
explicitBounds:
97+
- 0.1
98+
- 1
99+
- 10
100+
startTimeUnixNano: "1000000"
101+
sum: 0
102+
timeUnixNano: "2000000"
103103
name: traces_service_graph_request_client_seconds
104104
scope:
105105
name: traces_service_graph

‎connector/servicegraphconnector/testdata/virtual-node-label-server-expected-metrics.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,7 @@ resourceMetrics:
5656
- 1
5757
- 10
5858
startTimeUnixNano: "1000000"
59-
sum: 0.000001
59+
sum: 0
6060
timeUnixNano: "2000000"
6161
name: traces_service_graph_request_server_seconds
6262
- histogram:
@@ -89,7 +89,7 @@ resourceMetrics:
8989
- 1
9090
- 10
9191
startTimeUnixNano: "1000000"
92-
sum: 0.000000
92+
sum: 1e-06
9393
timeUnixNano: "2000000"
9494
name: traces_service_graph_request_client_seconds
9595
scope:

0 commit comments

Comments
 (0)