Skip to content
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

Define data ownership for LogRecordProcessor #2969

Merged
merged 6 commits into from
Dec 2, 2022

Conversation

jack-berg
Copy link
Member

@jack-berg jack-berg commented Nov 21, 2022

Resolves #2955.

States that LogRecordProcessor#onEmit can modify logRecord until it returns. If a reference to logRecord is held after onEmit returns, only reads are permitted.

If a processor wants to do asynchronous modification it needs to first make a copy is responsible for calling any downstream processors / exporters that expect to see the changes. Described more here.

specification/logs/sdk.md Outdated Show resolved Hide resolved
specification/logs/sdk.md Outdated Show resolved Hide resolved
@tigrannajaryan
Copy link
Member

We haven't concluded the discussion last time. What are we aiming for here? Chained processors?

@jack-berg
Copy link
Member Author

What are we aiming for here? Chained processors?

The approach with this PR is to try to resolve the mutability question while maintaining a LogRecordProcessor design as similar as possible to SpanProcessors: LogRecordProcessors are called by the SDK LoggerProvider in the order they were provided. There is no responsibility for a processor to call the next processor (i.e. no chaining).

It's not the only approach, but should be a frontrunner in my opinion since its simple to implement, consistent with tracing, and is quite flexible.

@jack-berg
Copy link
Member Author

@open-telemetry/specs-logs-approvers please take a look.

@reyang reyang merged commit 150451d into open-telemetry:main Dec 2, 2022
carlosalberto pushed a commit to carlosalberto/opentelemetry-specification that referenced this pull request Oct 31, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Resolve lingering question of LogRecord mutability in LogRecordProcessor
5 participants