mirror of
https://github.com/open-telemetry/opentelemetry-go.git
synced 2025-04-11 11:21:59 +02:00
log: Add design doc (#4809)
This commit is contained in:
parent
cfaf1f08e9
commit
c921815474
2
Makefile
2
Makefile
@ -315,4 +315,4 @@ add-tags: | $(MULTIMOD)
|
||||
|
||||
.PHONY: lint-markdown
|
||||
lint-markdown:
|
||||
docker run -v "$(CURDIR):$(WORKDIR)" docker://avtodev/markdown-lint:v1 -c $(WORKDIR)/.markdownlint.yaml $(WORKDIR)/**/*.md
|
||||
docker run -v "$(CURDIR):$(WORKDIR)" avtodev/markdown-lint:v1 -c $(WORKDIR)/.markdownlint.yaml $(WORKDIR)/**/*.md
|
||||
|
781
log/DESIGN.md
Normal file
781
log/DESIGN.md
Normal file
@ -0,0 +1,781 @@
|
||||
# Logs Bridge API
|
||||
|
||||
## Abstract
|
||||
|
||||
`go.opentelemetry.io/otel/log` provides
|
||||
[Logs Bridge API](https://opentelemetry.io/docs/specs/otel/logs/bridge-api/).
|
||||
|
||||
The initial version of the design and the prototype
|
||||
was created in [#4725](https://github.com/open-telemetry/opentelemetry-go/pull/4725).
|
||||
|
||||
## Background
|
||||
|
||||
The key challenge is to create a performant API compliant with the [specification](https://opentelemetry.io/docs/specs/otel/logs/bridge-api/)
|
||||
with an intuitive and user friendly design.
|
||||
Performance is seen as one of the most important characteristics of logging libraries in Go.
|
||||
|
||||
## Design
|
||||
|
||||
This proposed design aims to:
|
||||
|
||||
- be specification compliant,
|
||||
- be similar to Trace and Metrics API,
|
||||
- take advantage of both OpenTelemetry and `slog` experience to achieve acceptable performance.
|
||||
|
||||
### Module structure
|
||||
|
||||
The API is published as a single `go.opentelemetry.io/otel/log` Go module.
|
||||
|
||||
The module name is compliant with
|
||||
[Artifact Naming](https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/bridge-api.md#artifact-naming)
|
||||
and the package structure is the same as for Trace API and Metrics API.
|
||||
|
||||
The Go module consits of the following packages:
|
||||
|
||||
- `go.opentelemetry.io/otel/log`
|
||||
- `go.opentelemetry.io/otel/log/embedded`
|
||||
- `go.opentelemetry.io/otel/log/noop`
|
||||
|
||||
Rejected alternative:
|
||||
|
||||
- [Reuse slog](#reuse-slog)
|
||||
|
||||
### LoggerProvider
|
||||
|
||||
The [`LoggerProvider` abstraction](https://opentelemetry.io/docs/specs/otel/logs/bridge-api/#loggerprovider)
|
||||
is defined as an interface:
|
||||
|
||||
```go
|
||||
type LoggerProvider interface {
|
||||
embedded.LoggerProvider
|
||||
Logger(name string, options ...LoggerOption) Logger
|
||||
}
|
||||
```
|
||||
|
||||
The specification may add new operations to `LoggerProvider`.
|
||||
The interface may have methods added without a package major version bump.
|
||||
This embeds `embedded.LoggerProvider` to help inform an API implementation
|
||||
author about this non-standard API evolution.
|
||||
This approach is already used in Trace API and Metrics API.
|
||||
|
||||
#### LoggerProvider.Logger
|
||||
|
||||
The `Logger` method implements the [`Get a Logger` operation](https://opentelemetry.io/docs/specs/otel/logs/bridge-api/#get-a-logger).
|
||||
|
||||
The required `name` parameter is accepted as a `string` method argument.
|
||||
|
||||
The following options are defined to support optional parameters:
|
||||
|
||||
```go
|
||||
func WithInstrumentationVersion(version string) LoggerOption
|
||||
func WithInstrumentationAttributes(attr ...attribute.KeyValue) LoggerOption
|
||||
func WithSchemaURL(schemaURL string) LoggerOption
|
||||
```
|
||||
|
||||
Implementation requirements:
|
||||
|
||||
- The [specification requires](https://opentelemetry.io/docs/specs/otel/logs/bridge-api/#concurrency-requirements)
|
||||
the method to be safe to be called concurrently.
|
||||
|
||||
- The method should use some default name if the passed name is empty
|
||||
in order to meet the [specification's SDK requirement](https://opentelemetry.io/docs/specs/otel/logs/sdk/#logger-creation)
|
||||
to return a working logger when an invalid name is passed
|
||||
as well as to resemble the behavior of getting tracers and meters.
|
||||
|
||||
`Logger` can be extended by adding new `LoggerOption` options
|
||||
and adding new exported fields to the `LoggerConfig` struct.
|
||||
This design is already used in Trace API for getting tracers
|
||||
and in Metrics API for getting meters.
|
||||
|
||||
Rejected alternative:
|
||||
|
||||
- [Passing struct as parameter to LoggerProvider.Logger](#passing-struct-as-parameter-to-loggerproviderlogger).
|
||||
|
||||
### Logger
|
||||
|
||||
The [`Logger` abstraction](https://opentelemetry.io/docs/specs/otel/logs/bridge-api/#logger)
|
||||
is defined as an interface:
|
||||
|
||||
```go
|
||||
type Logger interface {
|
||||
embedded.Logger
|
||||
Emit(ctx context.Context, record Record)
|
||||
}
|
||||
```
|
||||
|
||||
The specification may add new operations to `Logger`.
|
||||
The interface may have methods added without a package major version bump.
|
||||
This embeds `embedded.Logger` to help inform an API implementation
|
||||
author about this non-standard API evolution.
|
||||
This approach is already used in Trace API and Metrics API.
|
||||
|
||||
### Logger.Emit
|
||||
|
||||
The `Emit` method implements the [`Emit a LogRecord` operation](https://opentelemetry.io/docs/specs/otel/logs/bridge-api/#emit-a-logrecord).
|
||||
|
||||
[`Context` associated with the `LogRecord`](https://opentelemetry.io/docs/specs/otel/context/)
|
||||
is accepted as a `context.Context` method argument.
|
||||
|
||||
Calls to `Emit` are supposed to be on the hot path.
|
||||
Therefore, in order to reduce the number of heap allocations,
|
||||
the [`LogRecord` abstraction](https://opentelemetry.io/docs/specs/otel/logs/bridge-api/#emit-a-logrecord),
|
||||
is defined as a `Record` type:
|
||||
|
||||
```go
|
||||
type Record struct {
|
||||
timestamp time.Time
|
||||
observedTimestamp time.Time
|
||||
severity Severity
|
||||
severityText string
|
||||
body Value
|
||||
|
||||
// The fields below are for optimizing the implementation of
|
||||
// attributes.
|
||||
front [5]KeyValue
|
||||
nFront int // The number of attributes in front.
|
||||
back []KeyValue
|
||||
}
|
||||
```
|
||||
|
||||
[`Timestamp`](https://opentelemetry.io/docs/specs/otel/logs/data-model/#field-timestamp)
|
||||
is accessed using following methods:
|
||||
|
||||
```go
|
||||
func (r *Record) Timestamp() time.Time
|
||||
func (r *Record) SetTimestamp(t time.Time)
|
||||
```
|
||||
|
||||
[`ObservedTimestamp`](https://opentelemetry.io/docs/specs/otel/logs/data-model/#field-observedtimestamp)
|
||||
is accessed using following methods:
|
||||
|
||||
```go
|
||||
func (r *Record) ObservedTimestamp() time.Time
|
||||
func (r *Record) SetObservedTimestamp(t time.Time)
|
||||
```
|
||||
|
||||
[`SeverityNumber`](https://opentelemetry.io/docs/specs/otel/logs/data-model/#field-severitynumber)
|
||||
is accessed using following methods:
|
||||
|
||||
```go
|
||||
func (r *Record) Severity() Severity
|
||||
func (r *Record) SetSeverity(s Severity)
|
||||
```
|
||||
|
||||
`Severity` type is defined and constants are based on
|
||||
[Displaying Severity recommendation](https://opentelemetry.io/docs/specs/otel/logs/data-model/#displaying-severity).
|
||||
Additionally, `Severity[Level]1` constants are defined to make the API more readable and user friendly.
|
||||
|
||||
```go
|
||||
type Severity int
|
||||
|
||||
const (
|
||||
SeverityTrace1 Severity = 1 // TRACE
|
||||
SeverityTrace2 Severity = 2 // TRACE2
|
||||
SeverityTrace3 Severity = 3 // TRACE3
|
||||
SeverityTrace4 Severity = 4 // TRACE4
|
||||
|
||||
SeverityDebug1 Severity = 5 // DEBUG
|
||||
SeverityDebug2 Severity = 6 // DEBUG2
|
||||
SeverityDebug3 Severity = 7 // DEBUG3
|
||||
SeverityDebug4 Severity = 8 // DEBUG4
|
||||
|
||||
SeverityInfo1 Severity = 9 // INFO
|
||||
SeverityInfo2 Severity = 10 // INFO2
|
||||
SeverityInfo3 Severity = 11 // INFO3
|
||||
SeverityInfo4 Severity = 12 // INFO4
|
||||
|
||||
SeverityWarn1 Severity = 13 // WARN
|
||||
SeverityWarn2 Severity = 14 // WARN2
|
||||
SeverityWarn3 Severity = 15 // WARN3
|
||||
SeverityWarn4 Severity = 16 // WARN4
|
||||
|
||||
SeverityError1 Severity = 17 // ERROR
|
||||
SeverityError2 Severity = 18 // ERROR2
|
||||
SeverityError3 Severity = 19 // ERROR3
|
||||
SeverityError4 Severity = 20 // ERROR4
|
||||
|
||||
SeverityFatal1 Severity = 21 // FATAL
|
||||
SeverityFatal2 Severity = 22 // FATAL2
|
||||
SeverityFatal3 Severity = 23 // FATAL3
|
||||
SeverityFatal4 Severity = 24 // FATAL4
|
||||
|
||||
SeverityTrace = SeverityTrace1
|
||||
SeverityDebug = SeverityDebug1
|
||||
SeverityInfo = SeverityInfo1
|
||||
SeverityWarn = SeverityWarn1
|
||||
SeverityError = SeverityError1
|
||||
SeverityFatal = SeverityFatal1
|
||||
)
|
||||
```
|
||||
|
||||
[`SeverityText`](https://opentelemetry.io/docs/specs/otel/logs/data-model/#field-severitytext)
|
||||
is accessed using following methods:
|
||||
|
||||
```go
|
||||
func (r *Record) SeverityText() string
|
||||
func (r *Record) SetSeverityText(s string)
|
||||
```
|
||||
|
||||
[`Body`](https://opentelemetry.io/docs/specs/otel/logs/data-model/#field-body)
|
||||
is accessed using following methods:
|
||||
|
||||
```go
|
||||
func (r *Record) Body() Value
|
||||
func (r *Record) SetBody(v Value)
|
||||
```
|
||||
|
||||
[Log record attributes](https://opentelemetry.io/docs/specs/otel/logs/data-model/#field-attributes)
|
||||
are accessed using following methods:
|
||||
|
||||
```go
|
||||
func (r *Record) WalkAttributes(f func(KeyValue) bool)
|
||||
func (r *Record) AddAttributes(attrs ...KeyValue)
|
||||
```
|
||||
|
||||
`Record` has a `AttributesLen` method that returns
|
||||
the number of attributes to allow slice preallocation
|
||||
when converting records to a different representation:
|
||||
|
||||
```go
|
||||
func (r *Record) AttributesLen() int
|
||||
```
|
||||
|
||||
The records attributes design and implemntation is based on
|
||||
[`slog.Record`](https://pkg.go.dev/log/slog#Record).
|
||||
It allows achieving high-performance access and manipulation of the attributes
|
||||
while keeping the API user friendly.
|
||||
It relieves the user from making his own improvements
|
||||
for reducing the number of allocations when passing attributes.
|
||||
|
||||
The following defintions are implementing the abstractions
|
||||
described in [the specification](https://opentelemetry.io/docs/specs/otel/logs/#new-first-party-application-logs):
|
||||
|
||||
```go
|
||||
type Value struct{}
|
||||
|
||||
type Kind int
|
||||
|
||||
const (
|
||||
KindEmpty Kind = iota
|
||||
KindBool
|
||||
KindFloat64
|
||||
KindInt64
|
||||
KindString
|
||||
KindBytes
|
||||
KindList
|
||||
KindMap
|
||||
)
|
||||
|
||||
func (v Value) Kind() Kind
|
||||
|
||||
// Value factories:
|
||||
|
||||
func StringValue(value string) Value
|
||||
|
||||
func IntValue(v int) Value
|
||||
|
||||
func Int64Value(v int64) Value
|
||||
|
||||
func Float64Value(v float64) Value
|
||||
|
||||
func BoolValue(v bool) Value
|
||||
|
||||
func BytesValue(v []byte) Value
|
||||
|
||||
func ListValue(vs ...Value) Value
|
||||
|
||||
func MapValue(kvs ...KeyValue) Value
|
||||
|
||||
// Value accessors:
|
||||
|
||||
func (v Value) AsAny() any
|
||||
|
||||
func (v Value) AsString() string
|
||||
|
||||
func (v Value) AsInt64() int64
|
||||
|
||||
func (v Value) AsBool() bool
|
||||
|
||||
func (v Value) AsFloat64() float64
|
||||
|
||||
func (v Value) AsBytes() []byte
|
||||
|
||||
func (v Value) AsList() []Value
|
||||
|
||||
func (v Value) AsMap() []KeyValue
|
||||
|
||||
func (v Value) Empty() bool
|
||||
|
||||
// Value equality comparison:
|
||||
|
||||
func (v Value) Equal(w Value) bool
|
||||
|
||||
|
||||
type KeyValue struct {
|
||||
Key string
|
||||
Value Value
|
||||
}
|
||||
|
||||
// KeyValue factories:
|
||||
|
||||
func String(key, value string)
|
||||
|
||||
func Int64(key string, value int64) KeyValue
|
||||
|
||||
func Int(key string, value int) KeyValue
|
||||
|
||||
func Float64(key string, v float64) KeyValue
|
||||
|
||||
func Bool(key string, v bool) KeyValue
|
||||
|
||||
func Bytes(key string, v []byte) KeyValue
|
||||
|
||||
func List(key string, args ...Value) KeyValue
|
||||
|
||||
func Map(key string, args ...KeyValue) KeyValue
|
||||
|
||||
// KeyValue equality comparison:
|
||||
|
||||
func (a KeyValue) Equal(b KeyValue) bool
|
||||
```
|
||||
|
||||
`Value` is representing `any`.
|
||||
`KeyValue` is representing a key(string)-value(`any`) pair.
|
||||
|
||||
`Kind` is an enumeration used for specifying the underlying value type.
|
||||
`KindEmpty` is used for an empty (zero) value.
|
||||
`KindBool` is used for boolean value.
|
||||
`KindFloat64` is used for a double precision floating point (IEEE 754-1985) value.
|
||||
`KindInt64` is used for a signed integer value.
|
||||
`KindString` is used for a string value.
|
||||
`KindBytes` is used for a slice of bytes (in spec: A byte array).
|
||||
`KindList` is used for a slice of values (in spec: an array (a list) of any values).
|
||||
`KindMap` is used for a slice of key-value pairs (in spec: `map<string, any>`).
|
||||
|
||||
These types are defined in `go.opentelemetry.io/otel/log` package
|
||||
as their are tightly coupled with the API and different from common attributes.
|
||||
|
||||
The internal implementation of `Value` is based on
|
||||
[`slog.Value`](https://pkg.go.dev/log/slog#Value)
|
||||
and the API is mostly inspired by
|
||||
[`attribute.Value`](https://pkg.go.dev/go.opentelemetry.io/otel/attribute#Value).
|
||||
The benchmarks[^1] show that the implementation is more performant than
|
||||
[`attribute.Value`](https://pkg.go.dev/go.opentelemetry.io/otel/attribute#Value).
|
||||
|
||||
The `Severity`, `Kind`, `Value`, `KeyValue` may implement
|
||||
the [`fmt.Stringer`](https://pkg.go.dev/fmt#Stringer) interface.
|
||||
However, it is not needed for the first stable release
|
||||
and the `String` methods can be added later.
|
||||
|
||||
The caller must not subsequently mutate the record passed to `Emit`.
|
||||
This would allow the implementation to not clone the record,
|
||||
but simply retain, modify or discard it.
|
||||
The implementation may still choose to clone the record or copy its attributes
|
||||
if it needs to retain or modify it,
|
||||
e.g. in case of asynchronous processing to eliminate the possibility of data races,
|
||||
because the user can technically reuse the record and add new attributes
|
||||
after the call (even when the documentation says that the caller must not do it).
|
||||
|
||||
Implementation requirements:
|
||||
|
||||
- The [specification requires](https://opentelemetry.io/docs/specs/otel/logs/bridge-api/#concurrency-requirements)
|
||||
the method to be safe to be called concurrently.
|
||||
|
||||
- The method must not interrupt the record processing if the context is canceled
|
||||
per ["ignoring context cancellation" guideline](../CONTRIBUTING.md#ignoring-context-cancellation).
|
||||
|
||||
- The [specification requires](https://opentelemetry.io/docs/specs/otel/logs/bridge-api/#emit-a-logrecord)
|
||||
use the current time as observed timestamp if the passed is empty.
|
||||
|
||||
- The method should handle the trace context passed via `ctx` argument in order to meet the
|
||||
[specification's SDK requirement](https://opentelemetry.io/docs/specs/otel/logs/sdk/#readablelogrecord)
|
||||
to populate the trace context fields from the resolved context.
|
||||
|
||||
`Emit` can be extended by adding new exported fields to the `Record` struct.
|
||||
|
||||
Rejected alternatives:
|
||||
|
||||
- [Record as interface](#record-as-interface)
|
||||
- [Options as parameter to Logger.Emit](#options-as-parameter-to-loggeremit)
|
||||
- [Passing record as pointer to Logger.Emit](#passing-record-as-pointer-to-loggeremit)
|
||||
- [Logger.WithAttributes](#loggerwithattributes)
|
||||
- [Record attributes as slice](#record-attributes-as-slice)
|
||||
- [Use any instead of defining Value](#use-any-instead-of-defining-value)
|
||||
- [Severity type encapsulating number and text](#severity-type-encapsulating-number-and-text)
|
||||
- [Reuse attribute package](#reuse-attribute-package)
|
||||
- [Mix receiver types for Record](#mix-receiver-types-for-record)
|
||||
- [Add XYZ method to Logger](#add-xyz-method-to-logger)
|
||||
- [Rename KeyValue to Attr](#rename-keyvalue-to-attr)
|
||||
|
||||
### noop package
|
||||
|
||||
The `go.opentelemetry.io/otel/log/noop` package provides
|
||||
[Logs Bridge API No-Op Implementation](https://opentelemetry.io/docs/specs/otel/logs/noop/).
|
||||
|
||||
### Trace context correlation
|
||||
|
||||
The bridge implementation should do its best to pass
|
||||
the `ctx` containing the trace context from the caller
|
||||
so it can later be passed via `Logger.Emit`.
|
||||
|
||||
It is not expected that users (caller or bridge implementation) reconstruct
|
||||
a `context.Context`. Reconstructing a `context.Context` with
|
||||
[`trace.ContextWithSpanContext`](https://pkg.go.dev/go.opentelemetry.io/otel/trace#ContextWithSpanContext)
|
||||
and [`trace.NewSpanContext`](https://pkg.go.dev/go.opentelemetry.io/otel/trace#NewSpanContext)
|
||||
would usually involve more memory allocations.
|
||||
|
||||
The logging libraries which have recording methods that accepts `context.Context`,
|
||||
such us [`slog`](https://pkg.go.dev/log/slog),
|
||||
[`logrus`](https://pkg.go.dev/github.com/sirupsen/logrus),
|
||||
[`zerolog`](https://pkg.go.dev/github.com/rs/zerolog),
|
||||
makes passing the trace context trivial.
|
||||
|
||||
However, some libraries do not accept a `context.Context` in their recording methods.
|
||||
Structured logging libraries,
|
||||
such as [`logr`](https://pkg.go.dev/github.com/go-logr/logr)
|
||||
and [`zap`](https://pkg.go.dev/go.uber.org/zap),
|
||||
offer passing `any` type as a log attribute/field.
|
||||
Therefore, their bridge implementations can define a "special" log attributes/field
|
||||
that will be used to capture the trace context.
|
||||
|
||||
[The prototype](https://github.com/open-telemetry/opentelemetry-go/pull/4725)
|
||||
has bridge implementations that handle trace context correlation efficiently.
|
||||
|
||||
## Benchmarking
|
||||
|
||||
The benchmarks take inspiration from [`slog`](https://pkg.go.dev/log/slog),
|
||||
because for the Go team it was also critical to create API that would be fast
|
||||
and interoperable with existing logging packages.[^2][^3]
|
||||
|
||||
The benchmark results can be found in [the prototype](https://github.com/open-telemetry/opentelemetry-go/pull/4725).
|
||||
|
||||
## Rejected alternatives
|
||||
|
||||
### Reuse slog
|
||||
|
||||
The API must not be coupled to [`slog`](https://pkg.go.dev/log/slog),
|
||||
nor any other logging library.
|
||||
|
||||
The API needs to evolve orthogonally to `slog`.
|
||||
|
||||
`slog` is not compliant with the [Logs Bridge API](https://opentelemetry.io/docs/specs/otel/logs/bridge-api/).
|
||||
and we cannot expect the Go team to make `slog` compliant with it.
|
||||
|
||||
The interoperabilty can be achieved using [a log bridge](https://opentelemetry.io/docs/specs/otel/glossary/#log-appender--bridge).
|
||||
|
||||
You can read more about OpenTelemetry Logs design on [opentelemetry.io](https://opentelemetry.io/docs/concepts/signals/logs/).
|
||||
|
||||
### Record as interface
|
||||
|
||||
`Record` is defined as a `struct` because of the following reasons.
|
||||
|
||||
Log record is a value object without any behavior.
|
||||
It is used as data input for Logger methods.
|
||||
|
||||
The log record resembles the instrument config structs like [metric.Float64CounterConfig](https://pkg.go.dev/go.opentelemetry.io/otel/metric#Float64CounterConfig).
|
||||
|
||||
Using `struct` instead of `interface` improves the performance as e.g.
|
||||
indirect calls are less optimized,
|
||||
usage of interfaces tend to increase heap allocations.[^3]
|
||||
|
||||
### Options as parameter to Logger.Emit
|
||||
|
||||
One of the initial ideas was to have:
|
||||
|
||||
```go
|
||||
type Logger interface{
|
||||
embedded.Logger
|
||||
Emit(ctx context.Context, options ...RecordOption)
|
||||
}
|
||||
```
|
||||
|
||||
The main reason was that design would be similar
|
||||
to the [Meter API](https://pkg.go.dev/go.opentelemetry.io/otel/metric#Meter)
|
||||
for creating instruments.
|
||||
|
||||
However, passing `Record` directly, instead of using options,
|
||||
is more performant as it reduces heap allocations.[^4]
|
||||
|
||||
Another advantage of passing `Record` is that API would not have functions like `NewRecord(options...)`,
|
||||
which would be used by the SDK and not by the users.
|
||||
|
||||
Finally, the definition would be similar to [`slog.Handler.Handle`](https://pkg.go.dev/log/slog#Handler)
|
||||
that was designed to provide optimization opportunities.[^2]
|
||||
|
||||
### Passing record as pointer to Logger.Emit
|
||||
|
||||
So far the benchmarks do not show differences that would
|
||||
favor passing the record via pointer (and vice versa).
|
||||
|
||||
Passing via value feels safer because of the following reasons.
|
||||
|
||||
The user would not be able to pass `nil`.
|
||||
Therefore, it reduces the possiblity to have a nil pointer dereference.
|
||||
|
||||
It should reduce the possibility of a heap allocation.
|
||||
|
||||
It follows the design of [`slog.Handler`](https://pkg.go.dev/log/slog#Handler).
|
||||
|
||||
If follows one of Google's Go Style Decisions
|
||||
to prefer [passing values](https://google.github.io/styleguide/go/decisions#pass-values).
|
||||
|
||||
### Passing struct as parameter to LoggerProvider.Logger
|
||||
|
||||
Similarly to `Logger.Emit`, we could have something like:
|
||||
|
||||
```go
|
||||
type LoggerProvider interface{
|
||||
embedded.LoggerProvider
|
||||
Logger(name context.Context, config LoggerConfig)
|
||||
}
|
||||
```
|
||||
|
||||
The drawback of this idea would be that this would be
|
||||
a different design from Trace and Metrics API.
|
||||
|
||||
The performance of acquiring a logger is not as critical
|
||||
as the performance of emitting a log record. While a single
|
||||
HTTP/RPC handler could write hundreds of logs, it should not
|
||||
create a new logger for each log entry.
|
||||
The bridge implementation should reuse loggers whenever possible.
|
||||
|
||||
### Logger.WithAttributes
|
||||
|
||||
We could add `WithAttributes` to the `Logger` interface.
|
||||
Then `Record` could be a simple struct with only exported fields.
|
||||
The idea was that the SDK would implement the performance improvements
|
||||
instead of doing it in the API.
|
||||
This would allow having different optimization strategies.
|
||||
|
||||
During the analysis[^5], it occurred that the main problem of this proposal
|
||||
is that the variadic slice passed to an interface method is always heap allocated.
|
||||
|
||||
Moreover, the logger returned by `WithAttribute` was allocated on the heap.
|
||||
|
||||
Lastly, the proposal was not specification compliant.
|
||||
|
||||
### Record attributes as slice
|
||||
|
||||
One of the proposals[^6] was to have `Record` as a simple struct:
|
||||
|
||||
```go
|
||||
type Record struct {
|
||||
Timestamp time.Time
|
||||
ObservedTimestamp time.Time
|
||||
Severity Severity
|
||||
SeverityText string
|
||||
Body Value
|
||||
Attributes []KeyValue
|
||||
```
|
||||
|
||||
The bridge implementations could use [`sync.Pool`](https://pkg.go.dev/sync#Pool)
|
||||
for reducing the number of allocations when passing attributes.
|
||||
|
||||
The benchmarks results were better.
|
||||
|
||||
In such a design, most bridges would have a `sync.Pool`
|
||||
to reduce the number of heap allocations.
|
||||
However, the `sync.Pool` will not work correctly with API implementations
|
||||
that would take ownership of the record
|
||||
(e.g. implementations that do not copy records for asynchronous processing).
|
||||
The current design, even in case of improper API implementation,
|
||||
has lower chances of encountering a bug as most bridges would
|
||||
create a record, pass it, and forget about it.
|
||||
|
||||
For reference, here is the reason why `slog` does not use `sync.Pool`[^3]
|
||||
as well:
|
||||
|
||||
> We can use a sync pool for records though we decided not to.
|
||||
You can but it's a bad idea for us. Why?
|
||||
Because users have control of Records.
|
||||
Handler writers can get their hands on a record
|
||||
and we'd have to ask them to free it
|
||||
or try to free it magically at some some point.
|
||||
But either way, they could get themselves in trouble by freeing it twice
|
||||
or holding on to one after they free it.
|
||||
That's a use after free bug and that's why `zerolog` was problematic for us.
|
||||
`zerolog` as as part of its speed exposes a pool allocated value to users
|
||||
if you use `zerolog` the normal way, that you'll see in all the examples,
|
||||
you will never encounter a problem.
|
||||
But if you do something a little out of the ordinary you can get
|
||||
use after free bugs and we just didn't want to put that in the standard library.
|
||||
|
||||
Therefore, we decided to not follow the proposal as it is
|
||||
less user friendly (users and bridges would use e.g. a `sync.Pool` to reduce
|
||||
the number of heap allocation), less safe (more prone to use after free bugs
|
||||
and race conditions), and the benchmark differences were not significant.
|
||||
|
||||
### Use any instead of defining Value
|
||||
|
||||
[Logs Data Model](https://opentelemetry.io/docs/specs/otel/logs/data-model/#field-body)
|
||||
defines Body to be `any`.
|
||||
One could propose to define `Body` (and attribute values) as `any`
|
||||
instead of a defining a new type (`Value`).
|
||||
|
||||
First of all, [`any` type defined in the specification](https://opentelemetry.io/docs/specs/otel/logs/data-model/#type-any)
|
||||
is not the same as `any` (`interface{}`) in Go.
|
||||
|
||||
Moreover, using `any` as a field would decrease the performance.[^7]
|
||||
|
||||
Notice it will be still possible to add following kind and factories
|
||||
in a backwards compatible way:
|
||||
|
||||
```go
|
||||
const KindMap Kind
|
||||
|
||||
func AnyValue(value any) KeyValue
|
||||
|
||||
func Any(key string, value any) KeyValue
|
||||
```
|
||||
|
||||
However, currently, it would not be specification compliant.
|
||||
|
||||
### Severity type encapsulating number and text
|
||||
|
||||
We could combine severity into a single field defining a type:
|
||||
|
||||
```go
|
||||
type Severity struct {
|
||||
Number SeverityNumber
|
||||
Text string
|
||||
}
|
||||
```
|
||||
|
||||
However, the [Logs Data Model](https://opentelemetry.io/docs/specs/otel/logs/data-model/#log-and-event-record-definition)
|
||||
define it as independent fields.
|
||||
It should be more user friendly to have them separated.
|
||||
Especially when having getter and setter methods, setting one value
|
||||
when the other is already set would be unpleasant.
|
||||
|
||||
## Reuse attribute package
|
||||
|
||||
It was tempting to reuse the existing
|
||||
[https://pkg.go.dev/go.opentelemetry.io/otel/attribute] package
|
||||
for defining log attributes and body.
|
||||
|
||||
However, this would be wrong because [the log attribute definition](https://opentelemetry.io/docs/specs/otel/logs/data-model/#field-attributes)
|
||||
is different from [the common attribute definition](https://opentelemetry.io/docs/specs/otel/common/#attribute).
|
||||
|
||||
Moreover, it there is nothing telling that [the body definition](https://opentelemetry.io/docs/specs/otel/logs/data-model/#field-body)
|
||||
has anything in common with a common attribute value.
|
||||
|
||||
Therefore, we define new types representing the abstract types defined
|
||||
in the [Logs Data Model](https://opentelemetry.io/docs/specs/otel/logs/data-model/#definitions-used-in-this-document).
|
||||
|
||||
## Mix receiver types for Record
|
||||
|
||||
Methods of [`slog.Record`](https://pkg.go.dev/log/slog#Record)
|
||||
have different receiver types.
|
||||
|
||||
In `log/slog` GitHub issue we can only find that the reason is:[^8]
|
||||
|
||||
>> some receiver of Record struct is by value
|
||||
> Passing Records by value means they incur no heap allocation.
|
||||
> That improves performance overall, even though they are copied.
|
||||
|
||||
However, the benchmarks do not show any noticeable differences.[^9]
|
||||
|
||||
The compiler is smart-enough to not make a heap allocation for any of these methods.
|
||||
The use of a pointer receiver does not cause any heap allocation.
|
||||
From Go FAQ:[^10]
|
||||
|
||||
> In the current compilers, if a variable has its address taken,
|
||||
> that variable is a candidate for allocation on the heap.
|
||||
> However, a basic escape analysis recognizes some cases
|
||||
> when such variables will not live past the return from the function
|
||||
> and can reside on the stack.
|
||||
|
||||
The [Understanding Allocations: the Stack and the Heap](https://www.youtube.com/watch?v=ZMZpH4yT7M0)
|
||||
presentation by Jacob Walker describes the escape analysis with details.
|
||||
|
||||
Moreover, also from Go FAQ:[^10]
|
||||
|
||||
> Also, if a local variable is very large,
|
||||
> it might make more sense to store it on the heap rather than the stack.
|
||||
|
||||
Therefore, even if we use a value receiver and the value is very large
|
||||
it may be heap allocated.
|
||||
|
||||
Both [Go Code Review Comments](https://go.dev/wiki/CodeReviewComments#receiver-type)
|
||||
and [Google's Go Style Decisions](https://google.github.io/styleguide/go/decisions#receiver-type)
|
||||
highly recommend making the methods for a type either all pointer methods
|
||||
or all value methods. Google's Go Style Decisions even goes further and says:
|
||||
|
||||
> There is a lot of misinformation about whether passing a value or a pointer
|
||||
> to a function can affect performance.
|
||||
> The compiler can choose to pass pointers to values on the stack
|
||||
> as well as copying values on the stack,
|
||||
> but these considerations should not outweigh the readability
|
||||
> and correctness of the code in most circumstances.
|
||||
> When the performance does matter, it is important to profile both approaches
|
||||
> with a realistic benchmark before deciding that one approach outperforms the other.
|
||||
|
||||
Because, the benchmarks[^9] do not proof any performance difference
|
||||
and the general recommendation is to not mix receiver types,
|
||||
we decided to use pointer receivers for all `Record` methods.
|
||||
|
||||
### Add XYZ method to Logger
|
||||
|
||||
The `Logger` does not have methods like `Enabled`, `SetSeverity`, etc.
|
||||
as the Bridge API needs to follow (be compliant with)
|
||||
the [specification](https://opentelemetry.io/docs/specs/otel/logs/bridge-api/)
|
||||
|
||||
Moreover, the Bridge API is intendend to be used to implement bridges.
|
||||
Applications should not use it directly. The applications should use logging packages
|
||||
such as [`slog`](https://pkg.go.dev/log/slog),
|
||||
[`logrus`](https://pkg.go.dev/github.com/sirupsen/logrus),
|
||||
[`zap`](https://pkg.go.dev/go.uber.org/zap),
|
||||
[`zerolog`](https://pkg.go.dev/github.com/rs/zerolog),
|
||||
[`logr`](https://pkg.go.dev/github.com/go-logr/logr).
|
||||
|
||||
### Rename KeyValue to Attr
|
||||
|
||||
There was a proposal to rename `KeyValue` to `Attr` (or `Attribute`).[^11]
|
||||
New developers may not intuitively know that `log.KeyValue` is an attribute in
|
||||
the OpenTelemetry parlance.
|
||||
|
||||
During the discussion we agreed to keep the `KeyValue` name.
|
||||
|
||||
The type is used in two semantics:
|
||||
|
||||
- as a log attribute
|
||||
- as a map item
|
||||
|
||||
As for map item semantics, this type is a key-value pair, not an attribute.
|
||||
Naming the type as `Attr` would convey semantical meaning
|
||||
that would not be correct for a map.
|
||||
|
||||
We expect that most of the Bridge API users will be OpenTelemetry contributors.
|
||||
We plan to implement bridges for the most popular logging libraries ourselves.
|
||||
Given we will all have the context needed to disambiguate these overlapping
|
||||
names, developers' confusion should not be an issue.
|
||||
|
||||
For bridges not developed by us,
|
||||
developers will likely look at our existing bridges for inspiration.
|
||||
Our correct use of these types will be a reference to them.
|
||||
|
||||
At last, we could consider a design defining both types: `KeyValue` and `Attr`.
|
||||
However, in this approach we would need have factory functions for both types.
|
||||
It would make the API surface unnecessarily big,
|
||||
and we may even have problems naming the functions.
|
||||
|
||||
## Open issues
|
||||
|
||||
The Logs Bridge API MUST NOT be released as stable
|
||||
before all issues below are closed:
|
||||
|
||||
- [Clarify that log attributes are NOT common attributes](https://github.com/open-telemetry/opentelemetry-specification/issues/3849)
|
||||
- [Clarify handling empty (null) values in Logs Data Model](https://github.com/open-telemetry/opentelemetry-specification/issues/3835)
|
||||
- [Clarify attributes parameter type of Get a Logger operation](https://github.com/open-telemetry/opentelemetry-specification/issues/3841)
|
||||
|
||||
[^1]: [Handle structured body and attributes](https://github.com/pellared/opentelemetry-go/pull/7)
|
||||
[^2]: Jonathan Amsterdam, [The Go Blog: Structured Logging with slog](https://go.dev/blog/slog)
|
||||
[^3]: Jonathan Amsterdam, [GopherCon Europe 2023: A Fast Structured Logging Package](https://www.youtube.com/watch?v=tC4Jt3i62ns)
|
||||
[^4]: [Emit definition discussion with benchmarks](https://github.com/open-telemetry/opentelemetry-go/pull/4725#discussion_r1400869566)
|
||||
[^5]: [Logger.WithAttributes analysis](https://github.com/pellared/opentelemetry-go/pull/3)
|
||||
[^6]: [Record attributes as field and use sync.Pool for reducing allocations](https://github.com/pellared/opentelemetry-go/pull/4) and [Record attributes based on slog.Record](https://github.com/pellared/opentelemetry-go/pull/6)
|
||||
[^7]: [Record.Body as any](https://github.com/pellared/opentelemetry-go/pull/5)
|
||||
[^8]: [log/slog: structured, leveled logging](https://github.com/golang/go/issues/56345#issuecomment-1302563756)
|
||||
[^9]: [Record with pointer receivers only](https://github.com/pellared/opentelemetry-go/pull/8)
|
||||
[^10]: [Go FAQ: Stack or heap](https://go.dev/doc/faq#stack_or_heap)
|
||||
[^11]: [Rename KeyValue to Attr discussion](https://github.com/open-telemetry/opentelemetry-go/pull/4809#discussion_r1476080093)
|
Loading…
x
Reference in New Issue
Block a user