sdk/log: move Enabled method from FilterProcessor to Processor #7639

pellared · 2025-11-27T07:20:40Z

Why

Simplify the Logs SDK by unifying processor capabilities into a single interface.

What

Add Enabled(ctx context.Context, p EnabledParameters) bool to sdk/log.Processor.
Remove sdk/log.FilterProcessor interface.

Processor implementations must now implement the Enabled method.
Custom processors that do not filter records can implement Enabled to return true.

…interface Fixes open-telemetry#7617

codecov · 2025-11-27T07:24:18Z

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 86.1%. Comparing base (a0a0acd) to head (c360f38).

Additional details and impacted files

@@           Coverage Diff           @@
##            main   #7639     +/-   ##
=======================================
- Coverage   86.1%   86.1%   -0.1%     
=======================================
  Files        298     298             
  Lines      21694   21694             
=======================================
- Hits       18695   18693      -2     
- Misses      2623    2625      +2     
  Partials     376     376

Files with missing lines	Coverage Δ
sdk/log/batch.go	`100.0% <100.0%> (ø)`
sdk/log/logger.go	`100.0% <100.0%> (ø)`
sdk/log/provider.go	`97.7% <ø> (-0.1%)`	⬇️
sdk/log/simple.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.

Copilot

Pull request overview

This pull request simplifies the Logs SDK by consolidating processor capabilities into a single unified interface. Previously, processors that wanted to support filtering had to implement a separate FilterProcessor interface with an Enabled method. Now, all processors must implement the Enabled method as part of the core Processor interface.

Key Changes:

The Enabled(ctx context.Context, param EnabledParameters) bool method has been moved from the FilterProcessor interface to the Processor interface
The FilterProcessor interface and the filter_processor.go file have been removed entirely
The EnabledParameters struct has been moved to processor.go

Reviewed changes

Copilot reviewed 10 out of 10 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
sdk/log/processor.go	Adds `Enabled` method and `EnabledParameters` struct to the core `Processor` interface, updates documentation
sdk/log/filter_processor.go	Removes the entire file containing the `FilterProcessor` interface
sdk/log/simple.go	Implements the `Enabled` method for `SimpleProcessor`, returning true for all records
sdk/log/batch.go	Implements the `Enabled` method for `BatchProcessor`, returning true for all records
sdk/log/provider.go	Removes tracking of `FilterProcessor` separately from `Processor`, simplifies processor registration
sdk/log/logger.go	Simplifies `Enabled` logic to iterate through all processors instead of tracking filter processors separately
sdk/log/provider_test.go	Adds `Enabled` method to test processor, removes `FilterProcessor` interface assertion
sdk/log/example_test.go	Renames example function, adds `Enabled` implementation to example processors, simplifies `ContextFilterProcessor`
sdk/log/bench_test.go	Updates benchmark processor `Enabled` signatures to use `EnabledParameters` instead of `Record`
CHANGELOG.md	Documents the addition of `Enabled` to `Processor` interface and removal of `FilterProcessor` interface

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

bboreham · 2025-11-27T17:08:54Z

sdk/log/processor.go

+	// The passed param is likely to be partial record information being
+	// provided (e.g. a param with only the Severity set).


This comment text seems to apply more to the previous version. EnabledParameters contains at most the Severity and EventName.

Which is still partial record information. This may contain more fields in the future. Still we can rephrase it if you have a proposal.

Yet, I prefer doing it in a separate PR. This PR tries to keep the existing documentation.

bboreham · 2025-11-27T17:11:14Z

sdk/log/processor.go

+	// If a Processor needs more information than is provided, it
+	// is said to be in an indeterminate state (see below).
+	//
+	// The returned value will be true when the Processor will process for the
+	// provided context and param, and will be false if the Processor will not
+	// process. The returned value may be true or false in an indeterminate state.
+	// An implementation should default to returning true for an indeterminate
+	// state, but may return false if valid reasons in particular circumstances
+	// exist (e.g. performance, correctness).


I can't see why this belongs as documentation in the interface. What would I as a user do based on this information?

This is for users implementing this interface.

I will try rephrasing but I prefer doing it in a separate PR. This PR tries to keep the existing documentation.

bboreham · 2025-11-27T17:13:29Z

sdk/log/processor.go

+	// The param should not be held by the implementation. A copy should be
+	// made if the param needs to be held after the call returns.
+	//
+	// Processor implementations are expected to re-evaluate the [Record] passed
+	// to OnEmit. It is not expected that the caller to OnEmit will
+	// use the result from Enabled prior to calling OnEmit.


This all seems to apply only to the previous implementation?

The implementation has not changed. However, I think you are right that this comment is unnecessary (param is immutable).

I will propose removing but I prefer doing it in a separate PR. This PR tries to keep the existing documentation.

bboreham · 2025-11-27T17:13:54Z

sdk/log/processor.go

+	// The SDK's Logger.Enabled returns false if all the registered processors
+	// return false. Otherwise, it returns true.


Belongs with Logger; does not belong on the generic interface.

The issue is that the Logger implantation is not exported so I am not sure where is a better place to put this information. Do you have any proposal?

bboreham · 2025-11-27T17:14:59Z

sdk/log/bench_test.go


 type attrAddProcessor struct{}

+func (attrAddProcessor) Enabled(context.Context, EnabledParameters) bool {


Nit: moving this up 7 lines creates unnecessary diffs.

True. I wanted to keep consistency in the repo and have Enabled method before OnEmit. 234241d. Should I scope it to a separate PR?

bboreham · 2025-11-27T17:15:55Z

sdk/log/bench_test.go


 type attrSetDecorator struct{}

+func (attrSetDecorator) Enabled(context.Context, EnabledParameters) bool {


Nit: moving this up 7 lines creates unnecessary diffs.

sdk/log: Move Enabled method from FilterProcessor to Processor …

fccca34

…interface Fixes open-telemetry#7617

pellared requested review from MrAlias, XSAM, dashpole, dmathieu and flc1125 as code owners November 27, 2025 07:20

pellared changed the title ~~sdk/log: Move Enabled method from FilterProcessor to Processor interface~~ sdk/log: Move Enabled method from FilterProcessor to Processor interface Nov 27, 2025

pellared changed the title ~~sdk/log: Move Enabled method from FilterProcessor to Processor interface~~ sdk/log: move Enabled method from FilterProcessor to Processor interface Nov 27, 2025

Update changelog

b83a30e

pellared requested a review from Copilot November 27, 2025 07:22

Copilot started reviewing on behalf of pellared November 27, 2025 07:22 View session

pellared self-assigned this Nov 27, 2025

pellared added enhancement New feature or request pkg:SDK Related to an SDK package area:logs Part of OpenTelemetry logs labels Nov 27, 2025

pellared added this to Go: Logs (GA) Nov 27, 2025

github-project-automation bot moved this to Todo in Go: Logs (GA) Nov 27, 2025

pellared moved this from Todo to In Progress in Go: Logs (GA) Nov 27, 2025

Copilot finished reviewing on behalf of pellared November 27, 2025 07:25

fix unused-receiver revive lint issues

988eeae

Copilot AI reviewed Nov 27, 2025

View reviewed changes

pellared marked this pull request as draft November 27, 2025 07:33

pellared added 2 commits November 27, 2025 08:40

move Enabled definitions before OnEmit

234241d

add unit tests for Enabled of simple and batch processor

c360f38

pellared marked this pull request as ready for review November 27, 2025 07:43

pellared changed the title ~~sdk/log: move Enabled method from FilterProcessor to Processor interface~~ sdk/log: move Enabled method from FilterProcessor to Processor Nov 27, 2025

dmathieu approved these changes Nov 27, 2025

View reviewed changes

bboreham reviewed Nov 27, 2025

View reviewed changes

pellared requested a review from bboreham November 27, 2025 19:58

		// The passed param is likely to be partial record information being
		// provided (e.g. a param with only the Severity set).

		// The SDK's Logger.Enabled returns false if all the registered processors
		// return false. Otherwise, it returns true.


		type attrAddProcessor struct{}

		func (attrAddProcessor) Enabled(context.Context, EnabledParameters) bool {


		type attrSetDecorator struct{}

		func (attrSetDecorator) Enabled(context.Context, EnabledParameters) bool {

sdk/log: move Enabled method from FilterProcessor to Processor #7639

Are you sure you want to change the base?

sdk/log: move Enabled method from FilterProcessor to Processor #7639

Uh oh!

Conversation

pellared commented Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why

What

Uh oh!

codecov bot commented Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

bboreham Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

pellared Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bboreham Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

pellared Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bboreham Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

pellared Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bboreham Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

pellared Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bboreham Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

pellared Nov 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bboreham Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

pellared Nov 27, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

pellared commented Nov 27, 2025 •

edited

Loading

codecov bot commented Nov 27, 2025 •

edited

Loading

pellared Nov 27, 2025 •

edited

Loading

pellared Nov 27, 2025 •

edited

Loading

pellared Nov 27, 2025 •

edited

Loading

pellared Nov 27, 2025 •

edited

Loading

pellared Nov 27, 2025 •

edited

Loading