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

[editorial] Clarify additional LogRecord interfaces #3937

Closed
wants to merge 1 commit into from

Conversation

pellared
Copy link
Member

This PR is a simpler version of #3898.

Currently, the Logs SDK specification says:

In addition to the definition for LogRecord, the following LogRecord-like interfaces are defined in the SDK:

This suggests that both types should have a representation in the SDK.

The problem is that each abstraction/type conversion can lead (depending on language) to performance overhead.

E.g. for Go:

  • casting a type to an interface can lead to a heap allocation12 which can noticeably affect the performance3
  • converting to a different struct (value type) is also not free

Moreover, having less abstractions reduces the API surface and makes the design simpler.

I believe that for Logs signal, performance is more important than API esthetics. Based on https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/README.md, I think that the Logs SDK should be designed in a way to achieve to have high performance for a scenario when a OTLP exporter with a batch processor is used.

In my opinion, the ReadableLogRecord, ReadWriteLogRecord terms should only be used to describe what the functionalities accepting log records MUST be able to do with them. The key is that the log processor MUST be able to mutate the log record that is received in OnEmit while all other functionalities do not need mutate the record so they MAY accept an immutable value.

I noticed that .NET SDK does not define anything like ReadableLogRecord.

Maybe it will help other languages as well in implementing the SDK.

Footnotes

  1. https://go101.org/optimizations/0.3-memory-allocations.html

  2. https://github.com/open-telemetry/opentelemetry-go/blob/main/log/DESIGN.md#record-as-interface

  3. https://tip.golang.org/doc/gc-guide#Eliminating_heap_allocations

@pellared pellared closed this Mar 13, 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.

1 participant