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

Add support for adding callables as extra fields #95

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

piotrmaslanka
Copy link

@piotrmaslanka piotrmaslanka commented Mar 20, 2024

Support was added for specifying callables as extra fields. This is super useful if we're trying to attach span_id and trace_id to a log entry in order to cross-match it with a tracing system, such as Jaeger, and we need that data dynamically rather than statically at startup.

Example:

get_context = lambda: tracer.active_span.context
add_trace_id = lambda: hex(get_context().trace_id)[2:] if tracer is not None and tracer.active_span is not None else None
add_span_id = lambda: hex(get_context().span_id)[2:] if tracer is not None and tracer.active_span else None

formatter = LogstashFormatter(extra={'span_id': add_span_id, 'trace_id': add_trace_id})
logging.getLogger().setFormatter(formatter)

extra_fields.update(self._extra)
for field_name, field_value in self._extra.items():
if callable(field_value):
field_value = field_value()
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if we should catch errors from the callable (and if so, move it into a seperate method).

If the callable throws an error, this way the whole log event will be lost.
If the error is handled, the log event can be sent anyways and we could log the error for later analysis.

What do you think?

Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@piotrmaslanka what do you think?

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.

2 participants