-
Notifications
You must be signed in to change notification settings - Fork 897
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
Clarify what LogRecordProcessor is able to modify #3902
Comments
cc @open-telemetry/java-approvers @open-telemetry/dotnet-approvers @open-telemetry/php-approvers @open-telemetry/cpp-approvers @open-telemetry/specs-logs-approvers |
Discussed in the 2/27/24 spec SIG. The intent was not for log processors to be able to mutate scopes or resource. We should clarify this and treat it as a bug. The log processor should be able to modify all the other fields on the log record, with exceptions for things like dropped_attributes_count, which are computed.
Trace context fields weren't discussed as an exception, so while I can't think of reasons why a processor would want to change them, I think allowing a processor to do so is aligned with the spirit of the specification.
I support. |
Fixes #3902 ## Changes Fix what can be modified via `ReadWriteLogRecord`.
Fixes open-telemetry#3902 ## Changes Fix what can be modified via `ReadWriteLogRecord`.
What are you trying to achieve?
I want to model
LogRecordProcessor
for Go SDK (open-telemetry/opentelemetry-go#4896).Currently this is what the spec says about
ReadWriteLogRecord
(theOnEmit
argument):Take notice that the full LogRecord contains info such us
Resource
andInstrumentationScope
. According to the spec the LogRecordProcessor should enable modifying them. Personally, I am not sure if it is desired.What should we do if the processor modifies the resource? Should we alter it for records emitted by the SDK or only this one being processed?
What should we do if the processor modifies the instrumentation? Should we alter it for all records associated with the instrumentation scope or only this one being processed?
Can the processor also modify the counts for attributes due to collection limits?
Shouldn't the processor be only allowing to modify the fields which can be set via
Emit
(via `Logger) using the Bridge API?Additionally, I am also not certain if the processor should be able to alter the Trace Context Fields.
I propose to explicitly list what LogRecordProcessor is able to modify.
Additional context
This is what it is possible to do with a
LogRecordProcessor
in different stable logs SDK implementations:The text was updated successfully, but these errors were encountered: