Skip to content

[DO NOT MERGE] minsev: Fix to filter out only in SeverityTrace1..SeverityFatal4 severity range#7424

Closed
pellared wants to merge 6 commits intoopen-telemetry:mainfrom
pellared:fix-minsev
Closed

[DO NOT MERGE] minsev: Fix to filter out only in SeverityTrace1..SeverityFatal4 severity range#7424
pellared wants to merge 6 commits intoopen-telemetry:mainfrom
pellared:fix-minsev

Conversation

@pellared
Copy link
Copy Markdown
Member

@pellared pellared commented Jun 4, 2025

Per open-telemetry/opentelemetry-specification#4541

From https://github.com/open-telemetry/opentelemetry-specification/blob/bdcf53d55285ae42339f37908a24ba6f3ac167cb/specification/logs/data-model.md?plain=1#L418-L423:

In the contexts where severity participates in less-than / greater-than
comparisons SeverityNumber field should be used. SeverityNumber can be
compared to another SeverityNumber or to numbers in the 1..24 range (or to the
corresponding short names).

Currently the spec only describes how to compare values between 1 and 24.
All other values have undefined meaning.

The main reason is to NOT filter out records with log.SeverityUndefined severity which can be a common scenario.

I also did my best to improve the docs.

@github-actions github-actions Bot requested a review from MrAlias June 4, 2025 07:26
@codecov
Copy link
Copy Markdown

codecov Bot commented Jun 4, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 82.2%. Comparing base (bc2c50a) to head (216ad10).
Report is 113 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@          Coverage Diff          @@
##            main   #7424   +/-   ##
=====================================
  Coverage   82.2%   82.2%           
=====================================
  Files        205     205           
  Lines      17949   17952    +3     
=====================================
+ Hits       14768   14774    +6     
+ Misses      2743    2741    -2     
+ Partials     438     437    -1     
Files with missing lines Coverage Δ
processors/minsev/minsev.go 100.0% <100.0%> (ø)

... and 1 file with indirect coverage changes

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@pellared pellared marked this pull request as ready for review June 4, 2025 07:31
@pellared pellared requested a review from a team as a code owner June 4, 2025 07:31
Comment thread processors/minsev/minsev.go Outdated
Co-authored-by: Damien Mathieu <42@dmathieu.com>
@pellared
Copy link
Copy Markdown
Member Author

pellared commented Jun 5, 2025

@pellared pellared added the blocked: specification Waiting on clarification of the OpenTelemetry specification before progress can be made label Jun 5, 2025
@pellared pellared changed the title minsev: Fix to filter out only in SeverityTrace1..SeverityFatal4 severity range [DO NOT MERGE] minsev: Fix to filter out only in SeverityTrace1..SeverityFatal4 severity range Jun 5, 2025
@dmathieu dmathieu self-requested a review June 6, 2025 07:48
@pellared
Copy link
Copy Markdown
Member Author

pellared commented Jun 12, 2025

From the SIG meetings discussions it seems that most prefer our current implementation: open-telemetry/opentelemetry-specification#4541 (comment).

I will extract the Go docs improvements to a separate PR.

@pellared pellared marked this pull request as draft June 12, 2025 12:29
@pellared pellared closed this Jul 10, 2025
@pellared
Copy link
Copy Markdown
Member Author

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

blocked: specification Waiting on clarification of the OpenTelemetry specification before progress can be made

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants