Skip to content

[pkg/ottl] Add ParseSeverity function #37280

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

Draft
wants to merge 56 commits into
base: main
Choose a base branch
from
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
56 commits
Select commit Hold shift + click to select a range
89e8285
add ParseSeverity function
bacherfl Jan 14, 2025
b9ad6c8
add internal type for severity mapping
bacherfl Jan 14, 2025
054a232
implement poc
bacherfl Jan 15, 2025
743cef8
implement severity parser
bacherfl Jan 16, 2025
11da9c4
add documentation and changelog entry
bacherfl Jan 17, 2025
6318f99
fix linting
bacherfl Jan 17, 2025
db576b6
add license header
bacherfl Jan 17, 2025
3aaf8c1
fix linting
bacherfl Jan 20, 2025
9d90777
fix linting
bacherfl Jan 20, 2025
ea03c91
Merge branch 'main' into feat/35079/parse-severity
bacherfl Jan 20, 2025
87d7813
adapt type returned by ParseValueExpression to accommodate for change…
bacherfl Jan 20, 2025
d833a0e
fix linting
bacherfl Jan 20, 2025
810e9e5
add support for http status code range placeholders
bacherfl Jan 21, 2025
58e180d
extend readme to document http status code placeholders
bacherfl Jan 21, 2025
98ba8b3
Merge branch 'main' into feat/35079/parse-severity
bacherfl Jan 23, 2025
bb9190b
Merge branch 'main' into feat/35079/parse-severity
bacherfl Jan 29, 2025
f93a027
Merge branch 'main' into feat/35079/parse-severity
bacherfl Feb 3, 2025
da60212
Merge branch 'main' into feat/35079/parse-severity
bacherfl Feb 18, 2025
7a3c7c5
Merge branch 'main' into feat/35079/parse-severity
bacherfl Mar 3, 2025
f952df5
Merge branch 'main' into feat/35079/parse-severity
bacherfl Mar 3, 2025
18637fe
Merge branch 'main' into feat/35079/parse-severity
bacherfl Mar 4, 2025
2538baa
Merge branch 'main' into feat/35079/parse-severity
bacherfl Mar 4, 2025
800f906
Merge branch 'main' into feat/35079/parse-severity
bacherfl Mar 10, 2025
28c5a90
Merge branch 'main' into feat/35079/parse-severity
bacherfl Mar 11, 2025
3eec193
Merge branch 'main' into feat/35079/parse-severity
bacherfl Mar 13, 2025
3fd1d3a
adapt to recent changes on main branch
bacherfl Mar 13, 2025
c86292b
Merge branch 'main' into feat/35079/parse-severity
bacherfl Mar 13, 2025
a7da745
Merge branch 'main' into feat/35079/parse-severity
bacherfl Mar 17, 2025
451be0e
Merge branch 'main' into feat/35079/parse-severity
bacherfl Mar 18, 2025
99d50a4
Merge branch 'main' into feat/35079/parse-severity
bacherfl Mar 19, 2025
f5c59ea
adapt to PR review
bacherfl Mar 20, 2025
4f94931
Merge branch 'main' into feat/35079/parse-severity
bacherfl Mar 20, 2025
cf5d161
adapt to review
bacherfl Mar 24, 2025
d24466a
trigger build
bacherfl Mar 26, 2025
89f5c00
go mod tidy
bacherfl Mar 26, 2025
ecc9642
Merge branch 'main' into feat/35079/parse-severity
bacherfl Apr 1, 2025
6048f96
Merge branch 'main' into feat/35079/parse-severity
bacherfl Apr 2, 2025
891e134
Merge branch 'main' into feat/35079/parse-severity
bacherfl Apr 3, 2025
bd539a6
Merge branch 'main' into feat/35079/parse-severity
bacherfl Apr 4, 2025
df5e816
Merge branch 'main' into feat/35079/parse-severity
bacherfl Apr 7, 2025
6d7588b
Merge branch 'main' into feat/35079/parse-severity
bacherfl Apr 14, 2025
b7e8c03
Merge branch 'main' into feat/35079/parse-severity
bacherfl Apr 22, 2025
4f013c3
adapt to new linting rules
bacherfl Apr 22, 2025
1286238
adapt tests to also cover http status code range placeholder
bacherfl Apr 22, 2025
fe909ce
remove unused function
bacherfl Apr 22, 2025
06a7172
adapt changelog entry to re-trigger chloggen check
bacherfl Apr 22, 2025
28fd8a9
Merge branch 'main' into feat/35079/parse-severity
TylerHelmuth Apr 22, 2025
17d4c85
Merge branch 'main' into feat/35079/parse-severity
bacherfl May 13, 2025
8d2e243
return error on incomplete range criteria
bacherfl May 13, 2025
d38c2c5
Merge branch 'feat/35079/parse-severity' of https://github.com/bacher…
bacherfl May 13, 2025
90998a3
adapt to pr review
bacherfl May 14, 2025
fd45a82
Merge branch 'main' into feat/35079/parse-severity
bacherfl May 14, 2025
404b6a4
Merge branch 'feat/35079/parse-severity' of https://github.com/bacher…
bacherfl May 26, 2025
14ffa05
adapt severity mapping structure
bacherfl May 26, 2025
12b023b
fix linting
bacherfl May 27, 2025
511187e
fix linting and update readme
bacherfl May 27, 2025
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
27 changes: 27 additions & 0 deletions .chloggen/parse-severity-function.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: enhancement

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

# A brief description of the change. Surround your text with quotes ("") if it needs to start with a backtick (`).
note: Add `ParseSeverity` function to define mappings for log severity levels.

# Mandatory: One or more tracking issues related to the change. You can use the PR number here if no issue exists.
issues: [35778]

# (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:

# If your change doesn't affect end users or the exported elements of any package,
# you should instead start your pull request title with [chore] or use the "Skip Changelog" label.
# Optional: The change log or logs in which this entry should be included.
# e.g. '[user]' or '[user, api]'
# Include 'user' if the change is relevant to end users.
# Include 'api' if there is a change to a library API.
# Default: '[user]'
change_logs: []
18 changes: 18 additions & 0 deletions pkg/ottl/e2e/e2e_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1206,6 +1206,24 @@ func Test_e2e_converters(t *testing.T) {
tCtx.GetLogRecord().Attributes().PutInt("test", 2)
},
},
{
statement: `set(
attributes["test"],
ParseSeverity(severity_number,
{
"error":[
{"equals": ["err"]},
{"range": { "min": 3, "max": 4 }}
],
"info":[
{"range": { "min": 1, "max": 2 }}
],
}
))`,
want: func(tCtx ottllog.TransformContext) {
tCtx.GetLogRecord().Attributes().PutStr("test", "info")
},
},
}

for _, tt := range tests {
Expand Down
29 changes: 29 additions & 0 deletions pkg/ottl/ottlfuncs/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -500,6 +500,7 @@ Available Converters:
- [ParseCSV](#parsecsv)
- [ParseJSON](#parsejson)
- [ParseKeyValue](#parsekeyvalue)
- [ParseSeverity](#parseseverity)
- [ParseSimplifiedXML](#parsesimplifiedxml)
- [ParseXML](#parsexml)
- [ProfileID](#profileid)
Expand Down Expand Up @@ -1558,6 +1559,34 @@ Examples:
- `ParseKeyValue("k1!v1_k2!v2_k3!v3", "!", "_")`
- `ParseKeyValue(log.attributes["pairs"])`

### ParseSeverity

`ParseSeverity(target, severityMapping)`

The `ParseSeverity` converter returns a `string` that represents one of the log levels defined by `severityMapping`.

`target` is a Getter that returns a string or an integer.
`severityMapping` is a map containing the log levels, and a list of values they are mapped from. These values can be either
strings, or map items containing a numeric range, defined by a `min` and `max` key (inclusive bounds), for the given log level.
A value will be mapped to the given log level if any of these conditions are true.
For example, the following mapping will map to the `info` level, if the `target` is either a string with the value `inf`,
or an integer in the range `[200,299]`:

`{"info":[{"equals": ["inf"]}, {"range":{"min":200, "max":299}}]}`

There is also support for expressing certain status code ranges via a placeholder string. The supported placeholders are the following:

- `"2xx"`: This string matches integer values between `[200,299]`
- `"3xx"`: This string matches integer values between `[300,399]`
- `"4xx"`: This string matches integer values between `[400,499]`
- `"5xx"`: This string matches integer values between `[500,599]`

Examples:

- `ParseSeverity(attributes["log-level"] {"info":[{"equals": ["inf"]}, {"range":{"min":200, "max":299}}]})`
- `ParseSeverity(attributes["log-level"] {"info":[{"range":"2xx""}]})`
- `ParseSeverity(severity_number {"info":[{"equals": ["inf"]}, {"range":{"min":200, "max":299}}], "error":[{"range":{"min":400, "max":499}}]})`

### ParseSimplifiedXML

`ParseSimplifiedXML(target)`
Expand Down
200 changes: 200 additions & 0 deletions pkg/ottl/ottlfuncs/func_parse_severity.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,200 @@
// Copyright The OpenTelemetry Authors
// SPDX-License-Identifier: Apache-2.0

package ottlfuncs // import "github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl/ottlfuncs"

import (
"context"
"errors"
"fmt"

"github.com/open-telemetry/opentelemetry-collector-contrib/pkg/ottl"
)

const (
// http2xx is a special key that is represents a range from 200 to 299
http2xx = "2xx"

// http3xx is a special key that is represents a range from 300 to 399
http3xx = "3xx"

// http4xx is a special key that is represents a range from 400 to 499
http4xx = "4xx"

// http5xx is a special key that is represents a range from 500 to 599
http5xx = "5xx"

minKey = "min"
maxKey = "max"

rangeKey = "range"
equalsKey = "equals"
)

type ParseSeverityArguments[K any] struct {
Target ottl.Getter[K]
Mapping ottl.PMapGetter[K]
}

func NewParseSeverityFactory[K any]() ottl.Factory[K] {
return ottl.NewFactory("ParseSeverity", &ParseSeverityArguments[K]{}, createParseSeverityFunction[K])
}

func createParseSeverityFunction[K any](_ ottl.FunctionContext, oArgs ottl.Arguments) (ottl.ExprFunc[K], error) {
args, ok := oArgs.(*ParseSeverityArguments[K])

if !ok {
return nil, errors.New("ParseSeverityFactory args must be of type *ParseSeverityArguments[K")
}

return parseSeverity[K](args.Target, args.Mapping), nil
}

func parseSeverity[K any](target ottl.Getter[K], mapping ottl.PMapGetter[K]) ottl.ExprFunc[K] {
return func(ctx context.Context, tCtx K) (any, error) {
severityMap, err := mapping.Get(ctx, tCtx)
if err != nil {
return nil, fmt.Errorf("cannot get severity mapping: %w", err)
}

value, err := target.Get(ctx, tCtx)
if err != nil {
return nil, fmt.Errorf("could not get log level: %w", err)
}

logLevel, err := evaluateSeverity(value, severityMap.AsRaw())
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My main concern about this function is performance, building up/looping through the severity mappings for every single execution can be expensive. I wish we could enforce the Mapping argument to be a map literal value, so we could improve the lookups, but I think that's currently not supported by OTTL.

That said, I'd try to avoid as many loops as possible, and use the pcommon.Map and pcommon.Slices values instead of parsing them to .AsRaw(). I might be wrong, but I think it would save some cycles/allocs that might worth the change. WDYT?

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My main concern about this function is performance, building up/looping through the severity mappings for every single execution can be expensive.

I fully agree with this part at least. This primary principle behind the stanza implementation is to build the mapping once and then having constant lookup times while processing each record. If we can't make this happen somehow in OTTL then we will pay quite a cost.

Can we build this mapping inside of parseSeverity but outside of the function returned by parseSeverity?

func parseSeverity[K any](target ottl.Getter[K], mapping ottl.PMapGetter[K]) ottl.ExprFunc[K] {
	// build static map of severities once
	return func(ctx context.Context, tCtx K) (any, error) {
		// use static map in every execution
	}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe there's a better way but if nothing else, I think we could declare the compiled mapping as a var within parseSeverity and outside the function it returns, and use a sync.Once to compile it the first time the function executes. This is oversimplified psuedocode, but basically:

func parseSeverity[K any](target ottl.Getter[K], mapping ottl.PMapGetter[K]) ottl.ExprFunc[K] {
	var compileOnce sync.Once
	var staticMapping map[string]int
	return func(ctx context.Context, tCtx K) (any, error) {
		compileOnce.Do(func() {
			precompiledMapping, getMappingErr = args.Mapping.Get(ctx, tCtx)
		})

		value, err := target.Get(ctx, tCtx)
		if err != nil {
			return nil, fmt.Errorf("could not get log level: %w", err)
		}

		sev, ok := staticMapping[value]
		if !ok {
			return defaultSeverity
		}
		return sev
	}
}

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

My favorite solution is finding a way to force the input to be a literal. It will take OTTL changes, but that is really what we want.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it even theoretically possible for a range to be expressed as a literal? Granted, it's not strictly necessary that we support ranges but there are some use cases where it is quite convenient.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it even theoretically possible for a range to be expressed as a literal?

Yes I think it's possible, not exactly the range, but the whole map as literal, which for OTTL essentially means that the function's parameter value can be retrieved at the bootstrap time, and differently from the ottl.Getters, it's immutable and does not depend on the transformation context to get accessed.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it even theoretically possible for a range to be expressed as a literal?

Yes I think it's possible, not exactly the range, but the whole map as literal, which for OTTL essentially means that the function's parameter value can be retrieved at the bootstrap time, and differently from the ottl.Getters, it's immutable and does not depend on the transformation context to get accessed.

I agree supporting literal inputs is nice but my point is that sometimes it's more user friendly to support something which can be interpreted once when the function is built. I think the notion of a range is one of those situations because e.g. no one wants to have to list all the HTTP status codes in order to assign them to severity levels. This shouldn't be a choice between literal inputs and recomputing a mapping for every context. If we can provide more user friendly inputs AND compute a complete mapping only once, this is better than literal inputs and also better than recomputing the same thing repeatedly.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree supporting literal inputs is nice but my point is that sometimes it's more user friendly to support something which can be interpreted once when the function is built

I might be missing the point, but that's what literals should do. The term "literals" is somehow confusing here, for OTTL it means we have a function's parameter that is not a getter, instead, it's a raw immutable value that is available when the function is built.

To support this use case, for example, OTTL needs to be changed so it knowns how to parse inputs like { "error":[ "err", { "min": 3, "max": 4 }]} into a raw pcommon.Map (required by the function argument).
The function usage doesn't change, the only thing that wouldn't be supported is using non-literal values (getters), as they're mutable, and their value might change from one statement to another, for example:

log_statements:
- context: log
  statements:
    - set(severity_number, ParseSeverity(severity_number, { "error":["err", { "min": 3, "max": 4 }]})) # Would work as the argument value is a literal, and cannot be changed by other statements.
    - set(cache["mappings"], { "error":["err", { "min": 3, "max": 4 }]}) 
    - set(severity_number, ParseSeverity(severity_number, cache["mappings"])) # Wouldn't work, as the cache["mappings"] path is mutable (getter), so it's not a literal and needs to be evaluate in every execution.
    - set(cache["mappings"], { "error":["err", { "min": 5, "max": 6 }]}) 

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I agree with your description of literals and agree we need them. My point though is about what happens after we have parsed the syntax.

Every single line of evaluateSeverityNumberMapping is unnecessary if we build a mapping (immediate lookup, not function evaluation).

Severity ranges tend to have a very reasonable and finite number of possible values so there's no need to evaluate logic every time we see a log. "Is this a range criteria?" "Is there a min?" "What is the min?", "Is this value greater than the min?", etc are all unnecessary if we can precompile this into a lookup table that is instant access.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh I see, my answer was more focused on the OTTL side and how we could get the map parsed as literal.
I agree with you, we definitely need to build a lookup table for that, ideally with O(1) access.

if err != nil {
return nil, fmt.Errorf("could not map log level: %w", err)
}

return logLevel, nil
}
}

func evaluateSeverity(value any, severities map[string]any) (string, error) {
for level, criteria := range severities {
criteriaList, ok := criteria.([]any)
if !ok {
return "", errors.New("criteria for mapping log level must be []any")
}
match, err := evaluateSeverityMapping(value, criteriaList)
if err != nil {
return "", fmt.Errorf("could not evaluate log level of value '%v': %w", value, err)
}
if match {
return level, nil
}
}
return "", fmt.Errorf("no matching log level found for value '%v'", value)
}

func evaluateSeverityMapping(value any, criteria []any) (bool, error) {
switch v := value.(type) {
case string:
return evaluateSeverityStringMapping(v, criteria), nil
case int64:
return evaluateSeverityNumberMapping(v, criteria)
default:
return false, fmt.Errorf("log level must be either string or int64, but got %T", v)
}
}

func evaluateSeverityNumberMapping(value int64, criteria []any) (bool, error) {
for _, crit := range criteria {
criteriaItem, ok := crit.(map[string]any)
if !ok {
continue
}

// right now, we only have a "range" criteria for numeric log levels, so we specifically check for this here
rangeMapObj, ok := criteriaItem[rangeKey]
if !ok {
continue
}

// if we have a numeric severity number, we need to match with number ranges
rangeMap, ok := rangeMapObj.(map[string]any)
if !ok {
rangeMap, ok = parseValueRangePlaceholder(rangeMapObj)
if !ok {
continue
}
}
rangeMin, gotMin := rangeMap[minKey]
rangeMax, gotMax := rangeMap[maxKey]
if !gotMin || !gotMax {
return false, errors.New("range criteria must contain min and max values")
}
rangeMinInt, ok := rangeMin.(int64)
if !ok {
return false, fmt.Errorf("min must be int64, but got %T", rangeMin)
}
rangeMaxInt, ok := rangeMax.(int64)
if !ok {
return false, fmt.Errorf("max must be int64, but got %T", rangeMax)
}

if rangeMinInt <= value && rangeMaxInt >= value {
return true, nil
}
}
return false, nil
}

func parseValueRangePlaceholder(crit any) (map[string]any, bool) {
placeholder, ok := crit.(string)
if !ok {
return nil, false
}

switch placeholder {
case http2xx:
return map[string]any{
"min": int64(200),
"max": int64(299),
}, true
case http3xx:
return map[string]any{
"min": int64(300),
"max": int64(399),
}, true
case http4xx:
return map[string]any{
"min": int64(400),
"max": int64(499),
}, true
case http5xx:
return map[string]any{
"min": int64(500),
"max": int64(599),
}, true
default:
return nil, false
}
}

func evaluateSeverityStringMapping(value string, criteria []any) bool {
for _, crit := range criteria {
criteriaItem, ok := crit.(map[string]any)
if !ok {
return false
}
criteriaEquals, ok := criteriaItem[equalsKey]
if !ok {
return false
}

equalsObjs, ok := criteriaEquals.([]any)
if !ok {
return false
}
for _, equals := range equalsObjs {
if equalsStr, ok := equals.(string); ok {
if equalsStr == value {
return true
}
}
}
}
return false
}
Loading
Loading