Skip to content

Commit 27c0067

Browse files
lahsivjarChrsMark
authored and
vincentfree
committed
[connector/signaltometrics] Fix metric identity to use name, type, and unit (open-telemetry#39454)
#### Description The component used metric name and description to identify metric that should be produced. Due to this, if duplicate metric names with different metric types were added then it would lead to incorrect results. The PR fixes this case by using the correct metric identity. <!-- Issue number (e.g. open-telemetry#1234) or full URL to issue, if applicable. --> #### Link to tracking issue Fixes open-telemetry#39442 <!--Describe what testing was performed and which tests were added.--> #### Testing Added testcase for logs and traces <!--Describe the documentation added.--> #### Documentation N/A --------- Co-authored-by: Christos Markou <[email protected]>
1 parent b33b94d commit 27c0067

File tree

9 files changed

+475
-20
lines changed

9 files changed

+475
-20
lines changed

‎.chloggen/signaltometrics-fix-metric-identity.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: signaltometricsconnector
8+
9+
# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
10+
note: Fix incorrect result for metrics configured with same name but different type
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: [39442]
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: [user]

‎connector/signaltometricsconnector/connector_test.go

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@ func TestConnectorWithTraces(t *testing.T) {
3737
"sum",
3838
"histograms",
3939
"exponential_histograms",
40+
"metric_identity",
4041
}
4142

4243
ctx, cancel := context.WithCancel(context.Background())
@@ -101,6 +102,7 @@ func TestConnectorWithLogs(t *testing.T) {
101102
"sum",
102103
"histograms",
103104
"exponential_histograms",
105+
"metric_identity",
104106
}
105107

106108
ctx, cancel := context.WithCancel(context.Background())

‎connector/signaltometricsconnector/internal/aggregator/aggregator.go

Lines changed: 13 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -48,8 +48,8 @@ func (a *Aggregator[K]) Aggregate(
4848
resAttrs, srcAttrs pcommon.Map,
4949
defaultCount int64,
5050
) error {
51-
switch {
52-
case md.ExponentialHistogram != nil:
51+
switch md.Key.Type {
52+
case pmetric.MetricTypeExponentialHistogram:
5353
val, count, err := getValueCount(
5454
ctx, tCtx,
5555
md.ExponentialHistogram.Value,
@@ -60,7 +60,7 @@ func (a *Aggregator[K]) Aggregate(
6060
return err
6161
}
6262
return a.aggregateValueCount(md, resAttrs, srcAttrs, val, count)
63-
case md.ExplicitHistogram != nil:
63+
case pmetric.MetricTypeHistogram:
6464
val, count, err := getValueCount(
6565
ctx, tCtx,
6666
md.ExplicitHistogram.Value,
@@ -71,7 +71,7 @@ func (a *Aggregator[K]) Aggregate(
7171
return err
7272
}
7373
return a.aggregateValueCount(md, resAttrs, srcAttrs, val, count)
74-
case md.Sum != nil:
74+
case pmetric.MetricTypeSum:
7575
raw, err := md.Sum.Value.Eval(ctx, tCtx)
7676
if err != nil {
7777
return fmt.Errorf("failed to execute OTTL value for sum: %w", err)
@@ -103,20 +103,20 @@ func (a *Aggregator[K]) Finalize(mds []model.MetricDef[K]) {
103103
destExpHist pmetric.ExponentialHistogram
104104
destExplicitHist pmetric.Histogram
105105
)
106-
switch {
107-
case md.ExponentialHistogram != nil:
106+
switch md.Key.Type {
107+
case pmetric.MetricTypeExponentialHistogram:
108108
destMetric := metrics.AppendEmpty()
109109
destMetric.SetName(md.Key.Name)
110-
destMetric.SetDescription(md.Key.Description)
111-
destMetric.SetUnit(md.Unit)
110+
destMetric.SetUnit(md.Key.Unit)
111+
destMetric.SetDescription(md.Description)
112112
destExpHist = destMetric.SetEmptyExponentialHistogram()
113113
destExpHist.SetAggregationTemporality(pmetric.AggregationTemporalityDelta)
114114
destExpHist.DataPoints().EnsureCapacity(len(dpMap))
115-
case md.ExplicitHistogram != nil:
115+
case pmetric.MetricTypeHistogram:
116116
destMetric := metrics.AppendEmpty()
117117
destMetric.SetName(md.Key.Name)
118-
destMetric.SetDescription(md.Key.Description)
119-
destMetric.SetUnit(md.Unit)
118+
destMetric.SetUnit(md.Key.Unit)
119+
destMetric.SetDescription(md.Description)
120120
destExplicitHist = destMetric.SetEmptyHistogram()
121121
destExplicitHist.SetAggregationTemporality(pmetric.AggregationTemporalityDelta)
122122
destExplicitHist.DataPoints().EnsureCapacity(len(dpMap))
@@ -136,8 +136,8 @@ func (a *Aggregator[K]) Finalize(mds []model.MetricDef[K]) {
136136
metrics := a.smLookup[resID].Metrics()
137137
destMetric := metrics.AppendEmpty()
138138
destMetric.SetName(md.Key.Name)
139-
destMetric.SetDescription(md.Key.Description)
140-
destMetric.SetUnit(md.Unit)
139+
destMetric.SetUnit(md.Key.Unit)
140+
destMetric.SetDescription(md.Description)
141141
destCounter := destMetric.SetEmptySum()
142142
destCounter.SetAggregationTemporality(pmetric.AggregationTemporalityDelta)
143143
destCounter.DataPoints().EnsureCapacity(len(dpMap))

‎connector/signaltometricsconnector/internal/aggregator/valuecountdp.go

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -23,12 +23,12 @@ func newValueCountDP[K any](
2323
attrs pcommon.Map,
2424
) *valueCountDP {
2525
var dp valueCountDP
26-
if md.ExponentialHistogram != nil {
26+
if md.Key.Type == pmetric.MetricTypeExponentialHistogram {
2727
dp.expHistogramDP = newExponentialHistogramDP(
2828
attrs, md.ExponentialHistogram.MaxSize,
2929
)
3030
}
31-
if md.ExplicitHistogram != nil {
31+
if md.Key.Type == pmetric.MetricTypeHistogram {
3232
dp.explicitHistogramDP = newExplicitHistogramDP(
3333
attrs, md.ExplicitHistogram.Buckets,
3434
)

‎connector/signaltometricsconnector/internal/model/model.go

Lines changed: 10 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -9,6 +9,7 @@ import (
99

1010
"go.opentelemetry.io/collector/component"
1111
"go.opentelemetry.io/collector/pdata/pcommon"
12+
"go.opentelemetry.io/collector/pdata/pmetric"
1213

1314
"github.com/open-telemetry/opentelemetry-collector-contrib/connector/signaltometricsconnector/config"
1415
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl"
@@ -21,8 +22,9 @@ type AttributeKeyValue struct {
2122
}
2223

2324
type MetricKey struct {
24-
Name string
25-
Description string
25+
Name string
26+
Type pmetric.MetricType
27+
Unit string
2628
}
2729

2830
type ExplicitHistogram[K any] struct {
@@ -105,7 +107,7 @@ func (s *Sum[K]) fromConfig(
105107

106108
type MetricDef[K any] struct {
107109
Key MetricKey
108-
Unit string
110+
Description string
109111
IncludeResourceAttributes []AttributeKeyValue
110112
Attributes []AttributeKeyValue
111113
Conditions *ottl.ConditionSequence[K]
@@ -120,8 +122,8 @@ func (md *MetricDef[K]) FromMetricInfo(
120122
telemetrySettings component.TelemetrySettings,
121123
) error {
122124
md.Key.Name = mi.Name
123-
md.Key.Description = mi.Description
124-
md.Unit = mi.Unit
125+
md.Key.Unit = mi.Unit
126+
md.Description = mi.Description
125127

126128
var err error
127129
md.IncludeResourceAttributes, err = parseAttributeConfigs(mi.IncludeResourceAttributes)
@@ -145,18 +147,21 @@ func (md *MetricDef[K]) FromMetricInfo(
145147
md.Conditions = &condSeq
146148
}
147149
if mi.Histogram != nil {
150+
md.Key.Type = pmetric.MetricTypeHistogram
148151
md.ExplicitHistogram = new(ExplicitHistogram[K])
149152
if err := md.ExplicitHistogram.fromConfig(mi.Histogram, parser); err != nil {
150153
return fmt.Errorf("failed to parse histogram config: %w", err)
151154
}
152155
}
153156
if mi.ExponentialHistogram != nil {
157+
md.Key.Type = pmetric.MetricTypeExponentialHistogram
154158
md.ExponentialHistogram = new(ExponentialHistogram[K])
155159
if err := md.ExponentialHistogram.fromConfig(mi.ExponentialHistogram, parser); err != nil {
156160
return fmt.Errorf("failed to parse histogram config: %w", err)
157161
}
158162
}
159163
if mi.Sum != nil {
164+
md.Key.Type = pmetric.MetricTypeSum
160165
md.Sum = new(Sum[K])
161166
if err := md.Sum.fromConfig(mi.Sum, parser); err != nil {
162167
return fmt.Errorf("failed to parse sum config: %w", err)

‎connector/signaltometricsconnector/testdata/logs/metric_identity/config.yaml

Lines changed: 18 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,18 @@
1+
signaltometrics:
2+
logs:
3+
- name: metric.log.duration
4+
description: Logrecords as histogram with log.duration from attributes
5+
histogram:
6+
count: "1"
7+
value: attributes["log.duration"]
8+
buckets: [1, 10, 50, 100, 200]
9+
- name: metric.log.duration
10+
description: Logrecords as exponential histogram with log.duration from attributes
11+
exponential_histogram:
12+
count: "1"
13+
value: attributes["log.duration"]
14+
buckets: [1, 10, 50, 100, 200]
15+
- name: metric.log.duration
16+
description: Logrecords as sum with log.duration from attributes
17+
sum:
18+
value: attributes["log.duration"]

‎connector/signaltometricsconnector/testdata/logs/metric_identity/output.yaml

Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
resourceMetrics:
2+
- resource:
3+
attributes:
4+
- key: resource.bar
5+
value:
6+
stringValue: bar
7+
- key: resource.foo
8+
value:
9+
stringValue: foo
10+
- key: signaltometrics.service.instance.id
11+
value:
12+
stringValue: 627cc493-f310-47de-96bd-71410b7dec09
13+
- key: signaltometrics.service.name
14+
value:
15+
stringValue: signaltometrics
16+
- key: signaltometrics.service.namespace
17+
value:
18+
stringValue: test
19+
scopeMetrics:
20+
- metrics:
21+
- description: Logrecords as histogram with log.duration from attributes
22+
histogram:
23+
aggregationTemporality: 1
24+
dataPoints:
25+
- bucketCounts:
26+
- "0"
27+
- "2"
28+
- "1"
29+
- "0"
30+
- "1"
31+
- "0"
32+
count: "4"
33+
explicitBounds:
34+
- 1
35+
- 10
36+
- 50
37+
- 100
38+
- 200
39+
sum: 128
40+
timeUnixNano: "1000000"
41+
name: metric.log.duration
42+
- description: Logrecords as exponential histogram with log.duration from attributes
43+
exponentialHistogram:
44+
aggregationTemporality: 1
45+
dataPoints:
46+
- count: "4"
47+
max: 101.5
48+
min: 7
49+
negative: {}
50+
positive:
51+
bucketCounts:
52+
- "1"
53+
- "0"
54+
- "0"
55+
- "0"
56+
- "0"
57+
- "0"
58+
- "0"
59+
- "1"
60+
- "0"
61+
- "0"
62+
- "0"
63+
- "0"
64+
- "0"
65+
- "0"
66+
- "0"
67+
- "0"
68+
- "0"
69+
- "0"
70+
- "0"
71+
- "0"
72+
- "0"
73+
- "0"
74+
- "0"
75+
- "1"
76+
- "0"
77+
- "0"
78+
- "0"
79+
- "0"
80+
- "0"
81+
- "0"
82+
- "0"
83+
- "0"
84+
- "0"
85+
- "0"
86+
- "0"
87+
- "0"
88+
- "0"
89+
- "0"
90+
- "0"
91+
- "0"
92+
- "0"
93+
- "0"
94+
- "0"
95+
- "0"
96+
- "0"
97+
- "0"
98+
- "0"
99+
- "0"
100+
- "0"
101+
- "0"
102+
- "0"
103+
- "0"
104+
- "0"
105+
- "0"
106+
- "0"
107+
- "0"
108+
- "0"
109+
- "0"
110+
- "0"
111+
- "0"
112+
- "0"
113+
- "0"
114+
- "0"
115+
- "0"
116+
- "0"
117+
- "0"
118+
- "0"
119+
- "0"
120+
- "0"
121+
- "0"
122+
- "0"
123+
- "0"
124+
- "0"
125+
- "0"
126+
- "0"
127+
- "0"
128+
- "0"
129+
- "0"
130+
- "0"
131+
- "0"
132+
- "0"
133+
- "0"
134+
- "0"
135+
- "0"
136+
- "0"
137+
- "0"
138+
- "0"
139+
- "0"
140+
- "0"
141+
- "0"
142+
- "0"
143+
- "0"
144+
- "0"
145+
- "0"
146+
- "0"
147+
- "0"
148+
- "0"
149+
- "0"
150+
- "0"
151+
- "0"
152+
- "0"
153+
- "0"
154+
- "0"
155+
- "0"
156+
- "0"
157+
- "0"
158+
- "0"
159+
- "0"
160+
- "0"
161+
- "0"
162+
- "0"
163+
- "0"
164+
- "0"
165+
- "0"
166+
- "0"
167+
- "0"
168+
- "0"
169+
- "0"
170+
- "0"
171+
- "0"
172+
- "0"
173+
- "0"
174+
- "0"
175+
- "0"
176+
- "1"
177+
offset: 89
178+
scale: 5
179+
sum: 128
180+
timeUnixNano: "1000000"
181+
name: metric.log.duration
182+
- description: Logrecords as sum with log.duration from attributes
183+
name: metric.log.duration
184+
sum:
185+
aggregationTemporality: 1
186+
dataPoints:
187+
- asDouble: 128
188+
timeUnixNano: "1000000"
189+
scope:
190+
name: github.com/open-telemetry/opentelemetry-collector-contrib/connector/signaltometricsconnector

0 commit comments

Comments
 (0)