opentelemetry-specification
opentelemetry-specification copied to clipboard
Refine only logs reads are permitted section
This is a portion added by @jack-berg in https://github.com/open-telemetry/opentelemetry-specification/pull/2969. I would rather not add it may lead to implementing processors which may lead to race conditions.
I could change it to something like
processing) only reads should be used
as modifications can lead to race conditions.
We could also say that modifications could be done if the log record used for asynchronous processing is cloned (e.g. via an isolating processor introduced here https://github.com/open-telemetry/opentelemetry-specification/pull/4062). But I think such clarifications should be better scoped in a separate PR.
@cijothomas, @jack-berg, thoughts? I am fine applying my suggestion if @jack-berg finds it good, but I would prefer not defining anything about cloning+modifications for asynchronous processing in scope of this PR.
Originally posted by @pellared in https://github.com/open-telemetry/opentelemetry-specification/pull/4067#discussion_r1656585974
@pellared The PR linked in this issue has materially changed. Is this issue still applicable?
@open-telemetry/technical-committee, yes it is still applicable.
What is more concurrent reads can also be unsafe if the log record was synchronously modified in a processor. That is why in Go Logs SDK we are cloning the log record before enqueuing it in the batch processor. See: https://github.com/open-telemetry/opentelemetry-go/blob/48fedfa58e35b5f3b08fe54e45cfad441dd82426/sdk/log/batch.go#L185-L187.
I can be a sponsor and try to refine the spec.