henry3260 commented on PR #73820:
URL: https://github.com/apache/airflow/pull/73820#issuecomment-5886569696
> > SocketLogHandler.Handle builds one map per log line and writes the four
standard
> > fields into it. event and timestamp went in before the user attributes,
so a
> > user attribute of the same name overwrote them. level and logger went in
> > after the attributes and were unaffected.
>
> Would it make more sense to keep the current behavior that if user
explicitly set the "event" field themself, we should just keep it instead of
overriding with the timestamp.
For `event` , I don't think we can, because slog only has one place to put
the message: it's the positional argument to `logger.Info(...)`, and it goes
out in the `event` key. The supervisor reads it back from there. So keeping the
user's `event` doesn't just reorder a field, it drops the message the user
wrote, with nothing left in the line
to show it existed. Python doesn't even allow this collision , structlog
raises `TypeError` for `logger.info("msg", event="x")`.
For `timestamp`, the supervisor decodes it with msgspec and that line sits
outside the try/except that
guards malformed log lines, so a non-RFC3339 value raises there rather than
just spoiling one line.
That said, you have a point that a value is dropped either way. Preserving
both would mean renaming the colliding attribute instead of overwriting it,
maybe we can follow the same way (logrus prefixes such keys with `fields.`), so
a user attribute called `event` would surface as `fields.event`. Happy to do
that in a follow-up.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
