-
Notifications
You must be signed in to change notification settings - Fork 1.2k
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
Option to create directory of specified log files #120
Comments
I'd rather not add this option, sorry. The config file is already quite verbose and this option may not provide much benefit over |
For the record, we would also appreciate this feature. We keep our log files in a path that is periodically purged of files and directories that haven't been touched in some amount of time (and there's nothing I can do about this). This means that if we shut down a process for a month, its log directory will be gone when we try to restart it, and so it would be nice for that directory to be automatically recreated. |
+1 for this option |
+1 |
As a related note, this diminishes the reliability of supervisor. I don't know if that's been solved somehow in the master branch, but in version 3.1.1, someone accidentally removed a log directory in our server, and supervisor simply stopped working, taking all other programs with it, not just the one with the log directory missing. I think the scalability of supervisor is also compromised because of this, since one problem in one of the programs can end up taking all others down. I'd be happy to contribute with a pull request, I just need some pointers as to where I should start in the code. Also, I don't know if I should create another issue for this... |
+1...very reasonable request, why not honoring it? |
Missing this too.... |
Is there a workaround that will at least let us create the directory from within the config file? Maybe run |
+1 We docker users should do this
This is a shell script for
Is there any good idea? |
+1
|
Here are more details about why this would be useful in my case. @mnaberez why sticking to your refusal? I have seen more solid arguments in favor of this than in your rejection.... (but I may be missing something, so more explanations welcome ;-) |
This continues to be a problem, specifically for users deploying supervisor within Docker in an ElasticBeanstalk environment. If you specify logging in Dockerrun, the service wipes the specified directories during deployment, deleting anything created while Docker is spinning up. This in turn breaks supervisor, because it expects files where none exist. If supervisor simply created directories and logs when none existed, it would completely solve the issue. As it stands, I had to disable discrete log files (and use syslog instead) in order to get my app to deploy. |
same here, I used the following workaround : |
I am also running into this now. I have a Raspberry Pi which has /var/log mounted on tmpfs to prevent writes to the SD card. After a reboot of the device, the /var/log/supervisor folder is gone and supervisor won't start. Auto creating this directory would be a much welcome feature. @mnaberez would you mind reconsidering? There have been offers of PR's for this feature as well. For context, I've spent about 4 hours debugging this issue today, and could not find any useful information or error messages informing me that a missing log directory causes supervisor to not start at all. That should really not be happening, and I don't know of any other software that doesn't know how to create it's own log folders, let alone refuses to run if they don't exist 😢 |
👍 I just ran into this as well. |
+1 👍 Would be very helpful for me as well. |
A kindly remind here. |
I just edited
and fixed for me. And I was the excatly the same situation with @adamreisnz :) |
docker wasn't around when this issue created. so, we use supervisord in docker nowadays. thanks it is nice tool. however, if we want to redirect the outputs then we have to add new line (and it creates new layer) to dockerfile such as if supervisord creates the directory then it would be good. thanks anyway. |
Bump |
+1 |
@mnaberez First of all, supervisor is a great tool. Due to the fact that supervisor is the first one to run (before the log directory can be created by the program it is starting) it is intuitive that it can create the log directory by itself, if it does not exists yet. Is there any other logic explanation not to add this option besides the comment "...the config file is already quite verbose..."? |
This comment has been minimized.
This comment has been minimized.
👍 +1 to this request |
In my opinion supervisor daemon itself should be super reliable and start even if there is any issue with configuration of particular programs. In my case I am deploying my application in 2 steps:
I run 1st step first time when I am bootstrapping new machine or when I do any changes to software or system configuration. 2nd step is running in order to release new version of application code. So after very first running of chef I am ending up with broken supervisor (service is dead) because it can't find directory. Then run capistrano in order to deploy application 1st time (it creates all the required directories) and tries to restart services using supervisorctl but fails. So I need to run chef 1 more time which is inconvenience. I do believe if something is wrong with single program configuration then supervisor daemon itself should start but program should be in failed state. |
+1 |
If I need some wrapper scripts for the supervisor itself, it kind of defeats the very purpose of the program. I think it should automatically create the whole directory branch by default. There could be an option to disable this behavior, not the other way around. |
There are actually two major problems with the behavior of the supervisor right now:
|
@mnaberez wrote:
The hypothetical problem of accidentally creating log files in a slightly wrong location seems like a much lesser trouble to me than the very real and frequent (it should be) problem of bringing down the entire server just because the administrator forgot to create the folder for a log file. And what if you mix up the name of the file and not the folder? What if you create a log in a wrong location that nevertheless exists? What if you specify a completely nonsensical name in an existing folder? After all, you know, garbage in, garbage out. Another aspect of this question is that one can conceptually see full file paths as fully qualified file names. That is, one can think of the file system as of a collection of files with names that happen to have slashes in them and not as of a tree of folders. Yet this collection can also be viewed as a tree of folders in another representation. Which brings us to:
mkdir -p, of course. Purely from a practical standpoint of the user not having to deal with some wrapper scripts or dependent command sequences.
How about they would correspond to the chown and chmod of the created log file? You create the log file itself, don't you? Why not also create the directory?
...which they don't do at first, but they find out about this problem after their server is brought down unexpectedly. If supervisord requires a wrapper script or dependent command sequences to deal with these things, it greatly reduces its usefulness as an application, as what the supervisor does is not particularly rocket science, so when you have to fix these kinds of problems yourself, you start thinking about the necessity of using the supervisord at all. Also makes you think about the general approach of the authors and what other traps they might have prepared for you.
Well, this sounds like it is out of scope for the current issue.
No. Different situations require different handling. Logs are different from e.g. critical files whose consistency is vital to the system. Logs are important, but aren't critical or vital. |
This opinion is application-specific. We will not guess about how important log files might be for someone else's application. If the user provides a |
Well, sure, I am certain there are people out there who have designed a system in which log files and their locations are vital for the functioning of that system, and they would prefer an outage of the entire server to a possibly wrong name of a folder for a single log file. But are those the majority of use cases? How typical is that situation? Do you generally prefer to optimize for the most common use case or the most uncommon use case?
But you did guess that an outage of the entire system is less critical than a possibility that the name of the folder for a single log file might be wrong. Essentially, if we have to choose between these two evils (service outage vs. possibly wrong folder name for a log file), some guessing seems unavoidable. |
The While it's entirely possible that some option to create log directories could be added at some point as discussed here, it's unlikely that the fail-fast behavior will change. It is unlikely that we are going to add special cases to ignore only some configuration mistakes as you are requesting. It would mean that users could no longer have confidence that a running |
As I mentioned in my original comment, there are circumstances where the directory literally cannot pre-exist (namely, docker within Elastic Beanstalk) because the deploying service wipes the master log directory during any new deployment. This behavior causes supervisord to fail every time, without any fault on the user's behalf. I don't find the counter-arguments against automatic directory creation terribly compelling. For example:
OK, let's assume there is a config mistake. What is the ultimate consequence of said mistake? Completely valid logs are generated and stored in the wrong place. When someone goes looking for the logs, they don't find them where they expected, so they go to the config file, see that the log destination has been incorrectly configured... and correct it. Meanwhile, they have all of the previous logs, and nothing has broken in the interim. If something did break (let's say there was an aggregator running and it wasn't collecting the logs because they weren't in the right place), then only one of two things could occur:
I don't see the pidfiles argument as analogous because logs aren't mission-critical, but PIDs are. You can't run processes at all without generating a PID, but you can run any application without generating logs. I don't think anyone here is arguing against the general fail-fast behavior here: instead, we're pointing out that the current fail-fast behavior for log files is a bad design pattern. But it strikes me that there's a simple solution: include a configuration parameter called "Automatically create log directory if absent?" and, when set to true, have it execute mkdir -p (as @dmitry-akimov-dXgjvQjNiMuVYecJ suggests) with the same permissions/ownership as the created logfile, and move on. You can have the config param default to "false" so that the error procs the first time a non-existent log directory is passed, so the user is forced to take a closer look at log configuration before making a decision. |
Most of the discussion of the last couple of days, including my own replies, has been a rehash of things already stated in this thread. I don't think it's worth going around again because I do not think new points are being made.
This issue is currently open. I reopened it because I think it is worth reconsidering. It is possible that we may add some sort of optional feature to create directories. I stated this above also. I am currently working on other issues and I am not able to work on this particular issue right now. The other committers on this project may be willing to work on it, or it can stay open. |
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
+1 |
1 similar comment
+1 |
Such a stupid bug and yet unfixed. Our |
I really don't understand why there is so much pushback on this idea. Tbh I think this behavior should be available and enabled by default for logs (but I understand the concern about changing default behavior). I don't see any argument about having it available and disabled by default however. If I ever get a minute I might look into submitting a pull request, but in case someone else has a desire to figure it out: Here's where they throw an error if the directory doesn't exist supervisor/supervisor/datatypes.py Lines 94 to 105 in b6b762d
supervisor/supervisor/datatypes.py Lines 342 to 351 in b6b762d
So a start of a fix could be: # https://github.com/Supervisor/supervisor/blob/3e7c082a71fef3860ecf06727edd091ddb3cc282/supervisor/options.py#L635
self.create_log_directories = boolean(
get('supervisord', 'create_log_directories', 'true')) # +++
...
section.logfile = existing_dirpath(
get('logfile', 'supervisord.log'),
create=self.create_log_directories) # +-
# https://github.com/Supervisor/supervisor/blob/b6b762d809e8c5966208b0836a9403294c3294c4/supervisor/datatypes.py#L342
def existing_dirpath(v, create=False):
nv = os.path.expanduser(v)
dir = os.path.dirname(nv)
if not dir:
# relative pathname with no directory component
return nv
if create: # +++
os.makedirs(dir, exist_ok=True)
if os.path.isdir(dir):
return nv
raise ValueError('The directory named as part of the path %s '
'does not exist' % v)
# https://github.com/Supervisor/supervisor/blob/b6b762d809e8c5966208b0836a9403294c3294c4/supervisor/datatypes.py#L94
def logfile_name(val, create=False):
coerced = val.lower() if hasattr(val, 'lower') else val
if coerced in LOGFILE_NONES:
return None
elif coerced in LOGFILE_AUTOS:
return Automatic
return existing_dirpath(val, create=create) # +-
# https://github.com/Supervisor/supervisor/blob/3e7c082a71fef3860ecf06727edd091ddb3cc282/supervisor/options.py#L970
lf_val = logfile_name(lf_val, create=self.create_log_directories) # +-
# https://github.com/Supervisor/supervisor/blob/3e7c082a71fef3860ecf06727edd091ddb3cc282/supervisor/options.py#L426
self.add("logfile", "supervisord.logfile", "l:", "logfile=",
lambda v: existing_dirpath(v, create=self.create_log_directories),
default="supervisord.log") |
I have created the log path myself ~~ help |
We probably would not change the default (at least not without bumping the major version). This issue has been open for a long time and there have been a lot of comments asking for it, so at this point we would consider adding it as an option (this was stated above) but nobody is currently working on it. On this issue tracker, issues that are rejected are closed. Issues that are still open are considered unresolved. |
This thread has been silent for a minute, but as the issue is still open I assume comments are still welcome. I agree with @mnaberez and others who have said that changing default behavior is not the thing to do. And the fail-fast approach is a good one - highly unambiguous - but situations demonstrably exist where it is a problem for non-existent directories specifically, as there is occasionally no way around it. Docker environments using supervisor have been mentioned as an example, and that situation is what brought me here today. The suggestion @willmcgugan made is a good one in my opinion; it doesn't change any existing or default behaviors so is compatible with any config files that may exist today. Not sure whether I think there needs to be a recursive option or if it should simply assume mkdir -p if it's creating directories. I'm leaning toward the latter. As for questions about whether supervisor should also then create directories for pids and the like - I think it could be thought of as a separate issue, although whatever decision is made here certainly could be applied elsewhere. Perhaps even it simply becomes a global option; will supervisor be creating directories where needed, or not? All that to say - another late vote in favor of this feature. |
I support this feature request as well. Having an opt-in option ( #1400 ) will allow the user to decide if:
The existing behavior can be kept as the default. Having the option would allow for some flexibility. Currently, there is no flexibility. |
+1 10+ years of stability to do not create log directory if not exists :D |
I tried to set the logfile to a file in a directory that doesn't exist. Shouldn't there be an option to automatically create these directories?
The text was updated successfully, but these errors were encountered: