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

CHANGELOG.md

@@ -69,6 +69,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
 ### Fixed
 
 - Fix data race when writing log entries with `context.Context` fields in `go.opentelemetry.io/contrib/bridges/otelzap`. (#7368)
+- Fix `LogProcessor` in `go.opentelemetry.io/contrib/processors/minsev` so that it filters out only the records with `Severity` in the `log.SeverityTrace1`..`log.SeverityFatal4` range. (#7424)
 
 <!-- Released section -->
 <!-- Don't change this section unless doing release -->

processors/minsev/minsev.go

@@ -8,38 +8,39 @@ package minsev // import "go.opentelemetry.io/contrib/processors/minsev"
 import (
 	"context"
 
+	logapi "go.opentelemetry.io/otel/log"
 	"go.opentelemetry.io/otel/sdk/log"
 )
 
 // NewLogProcessor returns a new [LogProcessor] that wraps the downstream
 // [log.Processor].
 //
-// severity reports the minimum record severity that will be logged. The
+// Severitier reports the minimum record severity that will be logged. The
 // LogProcessor discards records with lower severities. If severity is nil,
-// SeverityInfo is used as a default. The LogProcessor calls severity.Severity
+// SeverityInfo is used as a default. The LogProcessor calls severitier.Severity
 // for each record processed or queried; to adjust the minimum level
 // dynamically, use a [SeverityVar].
 //
 // If downstream is nil a default No-Op [log.Processor] is used. The returned
 // processor will not be enabled for nor emit any records.
-func NewLogProcessor(downstream log.Processor, severity Severitier) *LogProcessor {
+func NewLogProcessor(downstream log.Processor, severitier Severitier) *LogProcessor {
 	if downstream == nil {
 		downstream = defaultProcessor
 	}
-	if severity == nil {
-		severity = SeverityInfo
+	if severitier == nil {
+		severitier = SeverityInfo
 	}
-	p := &LogProcessor{Processor: downstream, sev: severity}
+	p := &LogProcessor{Processor: downstream, sev: severitier}
 	if fp, ok := downstream.(log.FilterProcessor); ok {
 		p.filter = fp
 	}
 	return p
 }
 
 // LogProcessor is an [log.Processor] implementation that wraps another
-// [log.Processor]. It will pass-through calls to OnEmit and Enabled for
-// records with severity greater than or equal to a minimum. All other method
-// calls are passed to the wrapped [log.Processor].
+// [log.Processor]. It filters out log records with severity below a minimum
+// severity level, which is provided by a [Severitier] interface, that are
+// within the [logapi.SeverityTrace1]..[logapi.SeverityFatal4] range.
 //
 // If the wrapped [log.Processor] is nil, calls to the LogProcessor methods
 // will panic. Use [NewLogProcessor] to create a new LogProcessor that ensures
@@ -57,26 +58,33 @@ var (
 	_ log.FilterProcessor = (*LogProcessor)(nil)
 )
 
-// OnEmit passes ctx and r to the [log.Processor] that p wraps if the severity
-// of record is greater than or equal to p.Minimum. Otherwise, record is
-// dropped.
+// OnEmit drops records with severity less than the one returned by [Severitier]
+// and inside the [logapi.SeverityTrace1]..[logapi.SeverityFatal4] range.
+// If the severity of record is greater than or equal to the one returned by [Severitier],
+// it calls the wrapped [log.Processor] with ctx and record.
 func (p *LogProcessor) OnEmit(ctx context.Context, record *log.Record) error {
-	if record.Severity() >= p.sev.Severity() {
-		return p.Processor.OnEmit(ctx, record)
+	sev := record.Severity()
+	if sev >= logapi.SeverityTrace1 && sev <= logapi.SeverityFatal4 && sev < p.sev.Severity() {
+		return nil
 	}
-	return nil
+	return p.Processor.OnEmit(ctx, record)
 }
 
-// Enabled returns if the [log.Processor] that p wraps is enabled if the
-// severity of param is greater than or equal to p.Minimum. Otherwise false is
-// returned.
+// Enabled returns false if the severity of param is inside the
+// [logapi.SeverityTrace1]..[logapi.SeverityFatal4] range and less than
+// the one returned by [Severitier].
+// Otherwise, it returns the result of calling Enabled on the wrapped
+// [log.Processor] if it implements [log.FilterProcessor].
+// If the wrapped [log.Processor] does not implement [log.FilterProcessor], it returns true.
 func (p *LogProcessor) Enabled(ctx context.Context, param log.EnabledParameters) bool {
 	sev := param.Severity
+	if sev >= logapi.SeverityTrace1 && sev <= logapi.SeverityFatal4 && sev < p.sev.Severity() {
+		return false
+	}
 	if p.filter != nil {
-		return sev >= p.sev.Severity() &&
-			p.filter.Enabled(ctx, param)
+		return p.filter.Enabled(ctx, param)
 	}
-	return sev >= p.sev.Severity()
+	return true
 }
 
 var defaultProcessor = noopProcessor{}

processors/minsev/minsev_test.go

@@ -5,6 +5,7 @@ package minsev
 
 import (
 	"context"
+	"slices"
 	"testing"
 
 	"github.com/stretchr/testify/assert"
@@ -106,7 +107,10 @@ func TestLogProcessorOnEmit(t *testing.T) {
 		p := NewLogProcessor(wrapped, SeverityTrace1)
 		ctx := context.Background()
 		r := &log.Record{}
-		for _, sev := range severities {
+
+		sevs := slices.Clone(severities)
+		sevs = append(sevs, api.Severity(-100), api.Severity(100)) // Include severities below and above the OpenTelemetry range.
+		for _, sev := range sevs {
 			r.SetSeverity(sev)
 			assert.ErrorIs(t, p.OnEmit(ctx, r), assert.AnError, sev.String())
 
@@ -142,7 +146,10 @@ func TestLogProcessorEnabled(t *testing.T) {
 		p := NewLogProcessor(wrapped, SeverityTrace1)
 		ctx := context.Background()
 		param := log.EnabledParameters{}
-		for _, sev := range severities {
+
+		sevs := slices.Clone(severities)
+		sevs = append(sevs, api.Severity(-100), api.Severity(100)) // Include severities below and above the OpenTelemetry range.
+		for _, sev := range sevs {
 			param.Severity = sev
 			assert.True(t, p.Enabled(ctx, param), sev.String())
 
@@ -187,6 +194,12 @@ func TestLogProcessorEnabled(t *testing.T) {
 		params.Severity = api.SeverityError
 		assert.True(t, p.Enabled(ctx, params))
 
+		params.Severity = api.Severity(-100)
+		assert.True(t, p.Enabled(ctx, params), "Severity below of the range of OpenTelemetry log severities should be enabled")
+
+		params.Severity = api.Severity(100)
+		assert.True(t, p.Enabled(ctx, params), "Severity above of the range of OpenTelemetry log severities should be enabled")
+
 		assert.Empty(t, wrapped.EnabledCalls)
 	})
 }