How to enable logging automatically

[noklam]

Introduction

Previous discussion:

• Consider whether we should introduce a .env file to setup logging #2428
• Consider removing support for project-side logging.yml #2281 (comment)

In previous discussion we banned the idea of introducing .env to enable logging, we introduce a new environment variable KEDRO_LOGGING_CONFIG that user need to set in their environment (maybe in terminal or settings.py).

With the new kedro new flow, when logging is selected, it is not automatically used until user set KEDRO_LOGGING_CONFIG manually.

Context

(Edited: removed discussion above .env since it's off topic and confused people)

One more problem of KEDRO_LOGGING_CONFIG is that it's not immediately obvious where should user put this environment. There are many options:

1. ~/.bashrc or it's variant, this way the environment variable is loaded in the beginning
2. putting it in one of the python file (which one? settings.py is too late)

[astrojuanlu]

Not sure about this. There's a myriad mechanisms to manage environment variables, for example:

• direnv https://direnv.net/ uses .envrc (and, only optionally, .env)
• rtx https://github.com/jdx/rtx uses its own config file:

$ cat .rtx.toml
[tools]
python = {version="3.10.11", virtualenv=".venv"}

[env]
GOOGLE_APPLICATION_CREDENTIALS = 'kedro-pypi-stats-dde2342d3c96.json'

• And then of course dotenv https://www.dotenv.org/ uses .env

I'm not sure what makes .env special. On one hand we could say "this is what we support", but what if folks are using something else? Also, what if there's a clash between Kedro loading .env and then direnv reading .envrc?

I think we should stay away from being too smart about the user environment, and let them manage it.

The core issue is:

With the new kedro new flow, when logging is selected, it is not automatically used until user set KEDRO_LOGGING_CONFIG manually.

which is a real user problem. But maybe we need to explore other solutions.

[noklam]

Updated the title to focus more on the logging problem.

[noklam]

Supplement on this, this will be a breaking change - because the config will no longer accept a full path but only the name of the files (or only relative to conf_source)

[astrojuanlu]

Another user confused by this https://linen-slack.kedro.org/t/16362029/hi-i-have-recently-updated-the-kedro-version-but-now-i-am-ge#f6a6d4d0-7615-496e-b1a5-18037b944eda

[DimedS]

Why is this environment variable necessary? Can we instead observe the project directory for a 'logging.yml' file and use it if found, or default to standard logging options if not?
From what I understand, the environment variable might also introduce a security issue we're currently unable to address, as outlined here: (#3474)

[astrojuanlu]

I think we should consider @noklam's #3591 seriously, abstain from fiddling with user-specified logging settings (hence avoid #3474 instead of ignoring it), and if the user wants to provide a file-based logging config, they can always do so in their own code and place it wherever they like. It's clear that logging is an ongoing pain (almost 2 years since #1470 was opened), and we haven't yet fully solved it.

[noklam]

@DimedS IIRC it's because of two conflicting features:

1. ConfigLoader is able to load from conf_source which can be a zip file (default of kedro package)
2. Logging is decoupled from ConfigLoader

This means that we need to read logging without configloader, and it's very common to put logging.yml in conf folder even it's not used by the config loader. I agree that should be the default and the environment variable should be a workaround, thus I suggest to load this automatically with our best effort.

Cc @merelcht

[noklam]

This issue is partially overlap to #3591

• Automatic enable logging.yml and kedro run -v is slightly overlap
• logging.yml affect beyonds kedro run, do we need to have -v for all kedro command?
• The default of the logging.yml is more independent, as I am challenging the idea of having too many LEVEL default there. This video seems to support that everything should propagate to root unless necessary. Only for rare case you need non-root level logger filtering. (info-handler)

I am particularly interested in your opinion about this @antonymilne.

[astrojuanlu]

Some extra reading material ahead of a TD session:

• https://www.structlog.org/en/stable/logging-best-practices.html
• https://12factor.net/logs

[antonymilne]

I'm a bit out of the loop here and will talk to @noklam to share my thoughts on it all in more detail, but some rough points:

• the current (I think?) environment variable solution was added after a lot of thought and consideration with @idanov. You should definitely pick his brains on the topic too since he will definitely have some useful things to say here
• the kedro handling of logging is indeed a direct departure from the 12 factor app guidance. I believe this was a conscious decision made in the early days of kedro since users wouldn't know how to e.g. show INFO level messages or direct their logs to a file easily. Hence the logging functionality was built into kedro out of the box for better UX. Possibly expertise of users and logging tools has changed since then, but the handling of logging in kedro has always been a matter of "practicality beats purity". 12FA is useful guidance but not gospel on kedro, e.g. our handling of run environments does exactly what 12FA says not to do
• personally I don't care much about the file-based logging, but being able to expose INFO level messages by default I think is important. There is an annoying conflict here where the default Python log level of WARNING contradicts with the fact that it's assumed to be good UX to update the user with INFO on progress as their pipeline runs. Of course this assumption could be questioned with some user research since AFAIK we've always just taken it for granted. Remember that kedro pipelines can take a loooong time, and unless there's some other way of demonstrating progress (e.g. a progress bar, which we did also try in a hackathon and I liked) then I feel like kedro should display INFO messages by default. So maybe a better way to handle it would instead be a --quiet option that changes the level from INFO to WARNING?
• generally agree that kedro should interfere as little as possible with Python's default logging system, e.g. by delegating as much as possible to the root logger. But with the caveat that again this is a question of purity vs. practicality, and Python default's logging level of WARNING doesn't seem suitable on kedro, and the default format of log messages is also pretty gross.

[astrojuanlu]

Quick 2 cents

• +1 on setting the default level of the Kedro logger to INFO (if just for backwards compatibility), and then having -q set it to WARNING and -qq to ERROR
  • I don't think the current logging.yml logic is the only way to achieve that though
  • I'm also ambivalent on whether we should change the global logging level to INFO
• -1 on keeping the current logging.yml logic - whoever wants fine grained control of logs, file logging, rotation etc should be using journald, supervisor, Datadog, or whatever other solution. this is not Kedro's responsibility
• +1 on directing users to current best-practices for logging, including creating official integrations and docs for Sentry, OpenTelemetry

[antonymilne]

• +1 on setting the default level of the Kedro logger to INFO (if just for backwards compatibility), and then having -q set it to WARNING and -qq to ERROR

And -v to DEBUG?

• I don't think the current logging.yml logic is the only way to achieve that though
• I'm also ambivalent on whether we should change the global logging level to INFO

Not sure whether @noklam filled you in on the chat we had about #3657 (there's a brief summary there but not all the details). But I think we should definitely not set the global logging level to INFO. This is actually what used to be done on kedro and I think it was wrong - see #1732 for more.

Agree that the current logging.yml isn't the only way to achieve setting kedro logger to INFO. Also discussed in #1732, kedro could just add a Python setLevel call manually rather than configuring logging from a file config.

[noklam]

I notice the discussion in this thread are mainly related to #3591, though they are related and maybe overlapping.

I have this comment in #3657 #3657 (comment)

Automatic loading logging is fine, but there are edge cases with CONF_SOURCE. I think it's still an improvement and it doesn't make anything worse in the current states.

The idea is almost the same what @DimedS suggested, which we try to locate the logging.yml in the project directory. There are some complication of whether CONF_SOURCE will be available. In any case, having a non-default CONF_SOURCE is not the majority case, having logging.yml not inside the conf folder is even rarer. This is a 90% solution and maybe good enough unless we come up with something better.

[noklam]

The solution is basically this: be3d28d in #3577. It should be handles the pattern slightly better, and try to access CONF_SOURCE if possible (may be not).

[sheldontsen-qb]

(Not adding to the solution but more sharing my view...)

As a user for several years (+5 years) of kedro since the early days before it was even called kedro, I was a bit thrown off that logging was removed by default in 0.19. Personally would still prefer it out of the box enabled by default (instead of having to read the docs to find out how to enable it, I would rather read the docs on how to disable if not needed). Also appreciate my view is one of many out there.

To counter Juan's views:
(counter) +1 on keeping the current logging.yml logic - whoever wants fine grained control of logs, file logging, rotation etc should be using journald, supervisor, Datadog, or whatever other solution. this is not Kedro's responsibility.

This is super helpful to just get people setup with something basic.

(for) +1 on directing users to current best-practices for logging, including creating official integrations and docs for Sentry, OpenTelemetry

More docs on how to integrate is always helpful

Both would be good in my case but it was really nice that logging was configured out of the box for a standard project, just like how we can choose spark to be part of the starter.

Last resort: add documentation on how to configure logging.yaml to replicate old behaviour as we know after_context_created hook would have the caveat of not logging anything before context is created. It's a one-time setup per new project which is fine but right now there's no advice on the best place to achieve this without the caveat above.

Thankssss!

[noklam]

Quick summary:

• ✅There are at least one obvious user problem - which is logging get chosen as a tool but has no effect, and the consensus is that we should fix it. - How to enable logging automatically #3446
  • One possible solution is hard coding the path to conf/logging.yml with option to override with KEDRO_LOGGINGI_CONFIG, very similar to be3d28d
• It's unclear whether we should do more for logging, such as adding a new -v flag
• It's mentioned logging in development and logging in production are very different, and users probably have different needs:
  • It was mentioned we may want to de-couple rich as a plugin, pip-uninstallable to deactivate it or at least provide an easy enough way to opt out from it.
• We may consider improving the messaging of logging and people should seek for production grade logging.
• It was mentioned even though there are users need, Kedro can still say NO as we had for many other things.

Antony shared the old user research here in case you miss it at the end: #2281 (comment)

[noklam]

I have closed #3591 and move it to discussion as it's unclear whether user want it given the current logging status.

[noklam]

From @antonymilne

I thought a bit more about @DimedS ’s excellent question about why we have an environment variable for point to conf/logging.yml at all and think there’s at least 3 reasons for this (but again, definitely worth following up with @idanov :

1. Minor historical reason: it gives users the ability to change logging configuration when they change from dev to prod and similar - like how used to be possible with kedro run environments. Basically no one used this functionality with run environments but since removing the possibility of doing it at all was kind of a “hard blow”, we softened it by enabling the environment variable mechanism that still lets the very few who want to do this to have some way to achieve it
2. The “correct” path to the logging.yml file should not actually be hardcoded to conf/logging.yml be would be settings.CONF_SOURCE / logging.yml. But I am pretty sure you’ll find there’s no good way to access settings at the point you setup logging and hence you can’t do the “correct” solution. What this means is that there’s no choice but to harcdode conf/logging.yml  but this will not work for the 0.0001% of people who annoyingly decided to change their CONF_SOURCE in settings.py. Hence these people need an environment variable to point to the right file*
3. With the --conf-source option on kedro run, this gets even trickier: where do you point to in order to find the right logging.yml file now? What you really want to do is point to <conf-source picked up from CLI>/logging.yml but I don’t think there’s a good way of doing this. This was a known problem at the time, since --conf-source was added at about the same time as these logging changes, and it was regarded by Ivan as the first (only?) great conflict within kedro where there is no good solution:

a. when user packages conf it includeslogging.yml (alternatively it could live in the project root to avoid this, but he didn’t like that any better overall)
b. user puts conf somewhere else/renames it/whatever
c. user specifies --conf-source that can point to this
d. files in --conf-source are used in the kedro run
e. BUT the logging.yml inside that --conf-source doesn’t get used! The only way to use it is to set the environment variable to point to it
f. this is extremely confusing and horrible behaviour. If there were a better solution then we should absolutely go for it, but we couldn’t come up with anything at the time. Ultimately maybe configuration of logging just needs to be moved later on in the kedro process, but that has its own drawbacks. Or if kedro didn’t look for a logging.yml file at all then that would completely eradicate this problem also, which would be very nice.

*possibly this CONF_SOURCE option should never have existed or should have been removed when the --conf-source option was added to kedro run. I’m not sure why it’s there tbh. If it didn’t exist then this complication wouldn’t be here which would be nice… but the bigger problem of point 3 remains (edited)

[astrojuanlu]

@noklam shall we copy-paste it instead 🙏🏽

[noklam]

@astrojuanlu Updated, you have caught me lazily screenshot because Github doesn't preserve the Slack message format 🥲

[astrojuanlu]

Yeah that's a bummer 😅 thanks!

[noklam]

As a follow up, I have created #3801 to fix this issue.

[noklam]

This is now implemented in #3831