Skip to content

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

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

Closed
wants to merge 6 commits into from

Conversation

pellared
Copy link
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
Copy link

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
Co-authored-by: Damien Mathieu <42@dmathieu.com>
@pellared
Copy link
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
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
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