-
Notifications
You must be signed in to change notification settings - Fork 44
Description
Motivation
Follow-up after #3430
The purpose of this ticket is to use the new WithSpanContext()
function to connect our traces to logs by adding span.id
and trace.id
to the logger fields.
One of the biggest challenges here is that throughout our codebase, logger objects are stored as fields in structs. This means that the hierarchical information stored when calling logger.With()
is lost. For example, take a method of a struct that has a logger that we want to enrich with trace and span ids. When we call logger.WithSpanContext()
, the new logger needs to be propagated to callees. If we re-assign the logger in the struct field, span and trace id will be wrong e.g. for the next call of the same function. If we pass a new logger object to callees, we end up in a confusing situation where we have both loggers saved in structs and arguments.
Note: in both of the approaches below, keep in mind that we should avoid repeating the fields more than once to make sure they show up correctly in Elastic APM. See: uber-go/zap#622
Approach 1
We should instead, rely on context.Context
objects to store our hierarchical information, including loggers.
DOD
- Refactor most important structs of cloudbeat to remove loggers
- Implement getting & storing loggers in
context.Context
- Potentially, merge the
clog
andtelemetry
packages inobservability
so that all the observability-related fields are stored in one struct incontext.Context
that encapsulates observability-related objects.
Approach 2
Alternatively, we can pass in ctx context.Context
as an argument to all log.Info
, .Warn
, .Error
etc calls and infer the span from the ctx
.
DOD
- Add
context.Context
as an argument to all calls ofclog.Logger
- Dynamically fetch the span from the
ctx
and append the IDs to the logged fields