Skip to content

Commit d51d861

Browse files
[connector/signaltometrics]Drop get OTTL func in favor of OTTL's ParseValueExpression (#38101)
<!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> #### Description The component was using an internal `get` OTTL function as a stop gap measure to extract value from OTTL expressions due to lack of such function from the core OTTL library. With #35621, support for data extraction using OTTL value expressions was introduced. This PR uses the `ParseValueExpression` method and drops the `get` OTTL function. The function was documented in README as internal function and strictly advised not to be used in configurations so I don't expect any issues. <!-- Issue number (e.g. #1234) or full URL to issue, if applicable. --> #### Link to tracking issue Fixes #38098 <!--Describe what testing was performed and which tests were added.--> #### Testing Unit tests are passing <!--Describe the documentation added.--> #### Documentation README is updated <!--Please delete paragraphs that you did not use before submitting.--> --------- Co-authored-by: Andrzej Stencel <[email protected]>
1 parent 6ea0ea7 commit d51d861

File tree

9 files changed

+72
-94
lines changed

9 files changed

+72
-94
lines changed
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: breaking
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: "`get` OTTL function is removed and expressions are now parsed using `ParseValueExpression`"
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: [38098]
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/README.md

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -229,9 +229,6 @@ signaltometrics.service.instance.id: <service_instance_id_of_the_otel_collector>
229229

230230
### Custom OTTL functions
231231

232-
The component implements a couple of custom OTTL functions:
232+
The component implements the following custom OTTL functions:
233233

234234
1. `AdjustedCount`: a converter capable of calculating [adjusted count for a span](https://github.com/open-telemetry/oteps/blob/main/text/trace/0235-sampling-threshold-in-trace-state.md).
235-
2. `get`: a temporary solution to parse OTTL expressions with only values. This is
236-
only for internal usage and MUST NOT be used explicitly as it is a stopgap measure
237-
([see this for more details](https://github.com/open-telemetry/opentelemetry-collector-contrib/issues/35621)).

connector/signaltometricsconnector/config/config.go

Lines changed: 19 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -239,38 +239,42 @@ func validateMetricInfo[K any](mi MetricInfo, parser ottl.Parser[K]) error {
239239
return fmt.Errorf("sum validation failed: %w", err)
240240
}
241241

242-
// Exactly one metric should be defined
243-
var (
244-
metricsDefinedCount int
245-
statements []string
246-
)
242+
// Exactly one metric should be defined. Also, validate OTTL expressions,
243+
// note that, here we only evaluate if statements are valid. Check for
244+
// required statements are left to the other validations.
245+
var metricsDefinedCount int
247246
if mi.Histogram != nil {
248247
metricsDefinedCount++
249248
if mi.Histogram.Count != "" {
250-
statements = append(statements, customottl.ConvertToStatement(mi.Histogram.Count))
249+
if _, err := parser.ParseValueExpression(mi.Histogram.Count); err != nil {
250+
return fmt.Errorf("failed to parse count OTTL expression for explicit histogram: %w", err)
251+
}
252+
}
253+
if _, err := parser.ParseValueExpression(mi.Histogram.Value); err != nil {
254+
return fmt.Errorf("failed to parse value OTTL expression for explicit histogram: %w", err)
251255
}
252-
statements = append(statements, customottl.ConvertToStatement(mi.Histogram.Value))
253256
}
254257
if mi.ExponentialHistogram != nil {
255258
metricsDefinedCount++
256259
if mi.ExponentialHistogram.Count != "" {
257-
statements = append(statements, customottl.ConvertToStatement(mi.ExponentialHistogram.Count))
260+
if _, err := parser.ParseValueExpression(mi.ExponentialHistogram.Count); err != nil {
261+
return fmt.Errorf("failed to parse count OTTL expression for exponential histogram: %w", err)
262+
}
263+
}
264+
if _, err := parser.ParseValueExpression(mi.ExponentialHistogram.Value); err != nil {
265+
return fmt.Errorf("failed to parse value OTTL expression for exponential histogram: %w", err)
258266
}
259-
statements = append(statements, customottl.ConvertToStatement(mi.ExponentialHistogram.Value))
260267
}
261268
if mi.Sum != nil {
262269
metricsDefinedCount++
263-
statements = append(statements, customottl.ConvertToStatement(mi.Sum.Value))
270+
if _, err := parser.ParseValueExpression(mi.Sum.Value); err != nil {
271+
return fmt.Errorf("failed to parse value OTTL expression for summary: %w", err)
272+
}
264273
}
265274
if metricsDefinedCount != 1 {
266275
return fmt.Errorf("exactly one of the metrics must be defined, %d found", metricsDefinedCount)
267276
}
268277

269-
// validate OTTL statements, note that, here we only evaluate if statements
270-
// are valid. Check for required statements is left to the other validations.
271-
if _, err := parser.ParseStatements(statements); err != nil {
272-
return fmt.Errorf("failed to parse OTTL statements: %w", err)
273-
}
274278
// validate OTTL conditions
275279
if _, err := parser.ParseConditions(mi.Conditions); err != nil {
276280
return fmt.Errorf("failed to parse OTTL conditions: %w", err)

connector/signaltometricsconnector/config/config_test.go

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -84,11 +84,11 @@ func TestConfig(t *testing.T) {
8484
},
8585
},
8686
{
87-
path: "invalid_ottl_statements",
87+
path: "invalid_ottl_value_expression",
8888
errorMsgs: []string{
89-
fullErrorForSignal(t, "spans", "failed to parse OTTL statements"),
90-
fullErrorForSignal(t, "datapoints", "failed to parse OTTL statements"),
91-
fullErrorForSignal(t, "logs", "failed to parse OTTL statements"),
89+
fullErrorForSignal(t, "spans", "failed to parse value OTTL expression"),
90+
fullErrorForSignal(t, "datapoints", "failed to parse value OTTL expression"),
91+
fullErrorForSignal(t, "logs", "failed to parse value OTTL expression"),
9292
},
9393
},
9494
{

connector/signaltometricsconnector/internal/aggregator/aggregator.go

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ func (a *Aggregator[K]) Aggregate(
7272
}
7373
return a.aggregateValueCount(md, resAttrs, srcAttrs, val, count)
7474
case md.Sum != nil:
75-
raw, _, err := md.Sum.Value.Execute(ctx, tCtx)
75+
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)
7878
}
@@ -237,7 +237,7 @@ func (a *Aggregator[K]) getResourceID(resourceAttrs pcommon.Map) [16]byte {
237237
// value is missing.
238238
func getValueCount[K any](
239239
ctx context.Context, tCtx K,
240-
valueExpr, countExpr *ottl.Statement[K],
240+
valueExpr, countExpr *ottl.ValueExpression[K],
241241
defaultCount int64,
242242
) (float64, int64, error) {
243243
val, err := getDoubleFromOTTL(ctx, tCtx, valueExpr)
@@ -257,12 +257,12 @@ func getValueCount[K any](
257257
func getIntFromOTTL[K any](
258258
ctx context.Context,
259259
tCtx K,
260-
s *ottl.Statement[K],
260+
s *ottl.ValueExpression[K],
261261
) (int64, error) {
262262
if s == nil {
263263
return 0, nil
264264
}
265-
raw, _, err := s.Execute(ctx, tCtx)
265+
raw, err := s.Eval(ctx, tCtx)
266266
if err != nil {
267267
return 0, err
268268
}
@@ -282,12 +282,12 @@ func getIntFromOTTL[K any](
282282
func getDoubleFromOTTL[K any](
283283
ctx context.Context,
284284
tCtx K,
285-
s *ottl.Statement[K],
285+
s *ottl.ValueExpression[K],
286286
) (float64, error) {
287287
if s == nil {
288288
return 0, nil
289289
}
290-
raw, _, err := s.Execute(ctx, tCtx)
290+
raw, err := s.Eval(ctx, tCtx)
291291
if err != nil {
292292
return 0, err
293293
}

connector/signaltometricsconnector/internal/customottl/funcs.go

Lines changed: 1 addition & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -27,8 +27,5 @@ func LogFuncs() map[string]ottl.Factory[ottllog.TransformContext] {
2727
}
2828

2929
func commonFuncs[K any]() map[string]ottl.Factory[K] {
30-
getFactory := NewGetFactory[K]()
31-
standard := ottlfuncs.StandardFuncs[K]()
32-
standard[getFactory.Name()] = getFactory
33-
return standard
30+
return ottlfuncs.StandardFuncs[K]()
3431
}

connector/signaltometricsconnector/internal/customottl/get.go

Lines changed: 0 additions & 46 deletions
This file was deleted.

connector/signaltometricsconnector/internal/model/model.go

Lines changed: 14 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,6 @@ import (
1111
"go.opentelemetry.io/collector/pdata/pcommon"
1212

1313
"github.com/open-telemetry/opentelemetry-collector-contrib/connector/signaltometricsconnector/config"
14-
"github.com/open-telemetry/opentelemetry-collector-contrib/connector/signaltometricsconnector/internal/customottl"
1514
"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl"
1615
)
1716

@@ -27,8 +26,8 @@ type MetricKey struct {
2726

2827
type ExplicitHistogram[K any] struct {
2928
Buckets []float64
30-
Count *ottl.Statement[K]
31-
Value *ottl.Statement[K]
29+
Count *ottl.ValueExpression[K]
30+
Value *ottl.ValueExpression[K]
3231
}
3332

3433
func (h *ExplicitHistogram[K]) fromConfig(
@@ -42,12 +41,12 @@ func (h *ExplicitHistogram[K]) fromConfig(
4241
var err error
4342
h.Buckets = mi.Buckets
4443
if mi.Count != "" {
45-
h.Count, err = parser.ParseStatement(customottl.ConvertToStatement(mi.Count))
44+
h.Count, err = parser.ParseValueExpression(mi.Count)
4645
if err != nil {
47-
return fmt.Errorf("failed to parse count statement for explicit histogram: %w", err)
46+
return fmt.Errorf("failed to parse count OTTL expression for explicit histogram: %w", err)
4847
}
4948
}
50-
h.Value, err = parser.ParseStatement(customottl.ConvertToStatement(mi.Value))
49+
h.Value, err = parser.ParseValueExpression(mi.Value)
5150
if err != nil {
5251
return fmt.Errorf("failed to parse value statement for explicit histogram: %w", err)
5352
}
@@ -56,8 +55,8 @@ func (h *ExplicitHistogram[K]) fromConfig(
5655

5756
type ExponentialHistogram[K any] struct {
5857
MaxSize int32
59-
Count *ottl.Statement[K]
60-
Value *ottl.Statement[K]
58+
Count *ottl.ValueExpression[K]
59+
Value *ottl.ValueExpression[K]
6160
}
6261

6362
func (h *ExponentialHistogram[K]) fromConfig(
@@ -71,20 +70,20 @@ func (h *ExponentialHistogram[K]) fromConfig(
7170
var err error
7271
h.MaxSize = mi.MaxSize
7372
if mi.Count != "" {
74-
h.Count, err = parser.ParseStatement(customottl.ConvertToStatement(mi.Count))
73+
h.Count, err = parser.ParseValueExpression(mi.Count)
7574
if err != nil {
76-
return fmt.Errorf("failed to parse count statement for exponential histogram: %w", err)
75+
return fmt.Errorf("failed to parse count OTTL expression for exponential histogram: %w", err)
7776
}
7877
}
79-
h.Value, err = parser.ParseStatement(customottl.ConvertToStatement(mi.Value))
78+
h.Value, err = parser.ParseValueExpression(mi.Value)
8079
if err != nil {
81-
return fmt.Errorf("failed to parse value statement for exponential histogram: %w", err)
80+
return fmt.Errorf("failed to parse value OTTL expression for exponential histogram: %w", err)
8281
}
8382
return nil
8483
}
8584

8685
type Sum[K any] struct {
87-
Value *ottl.Statement[K]
86+
Value *ottl.ValueExpression[K]
8887
}
8988

9089
func (s *Sum[K]) fromConfig(
@@ -96,9 +95,9 @@ func (s *Sum[K]) fromConfig(
9695
}
9796

9897
var err error
99-
s.Value, err = parser.ParseStatement(customottl.ConvertToStatement(mi.Value))
98+
s.Value, err = parser.ParseValueExpression(mi.Value)
10099
if err != nil {
101-
return fmt.Errorf("failed to parse value statement for sum: %w", err)
100+
return fmt.Errorf("failed to parse value OTTL expression for sum: %w", err)
102101
}
103102
return nil
104103
}

0 commit comments

Comments
 (0)