Configurable backup log file

[lhoss]

main IDEAs of the PR:

• be able to use the exporter (works quite nicely!) more independently of the barman_docker container
• full flexibility on choosing the filename of the backup file (still providing downward compatibility to the original pattern, except that the BACKUP_LOG_DIR replaces BARMAN_BARMAN_HOME, with the added benefit of avoiding any hardcoded subdirs as before (./prometheus_exporter_work)

I tested that all the cases work

ps: other behavior change: The get_backup_log_fileno longer fails (even if none of the 2 used ENV vars are set). In this case the 3 metrics related to the backup file (parsing logic), will simply stay empty (which is probably ok for many folks, that are happy to have a good number of usable base metrics)

[lhoss]

@rsilva4 I guess that PR would get a faster merge if I had kept the BARMAN_BARMAN_HOME as fallback), but don't you agree it's cleaner to use the BACKUP_LOG_DIR (that's btw set in some of the other scripts. I'ld propose to extract also those duplicate ENV vars, into a common barman_env.sh)

[rsilva4-zz]

Hi @lhoss I was going through the PR changes and it's been a while since I wrote this and so far I'm not expecting nothing to be broken at a first glance. However I have to more carefully inspect what is the potential impact for current production deployments.

Regarding your thoughts about BACKUP_LOG_DIR I agree and honestly I don't remember why I don't put it there from the beginning.

I also like your idea of a barman_env.sh with all the ENV stuff it would makes things a lot cleaner. Fell free to drop that change in another PR I would be grateful.

Regarding this one, I expect to have a final say about it tomorrow, today it's getting late :sleepy: ok?

[rsilva4-zz]

@lhoss right now the changes proposed don't cover the case where you define those variables and they are different from the previous hard-coded ones. In that case the bash scripts will still write to that previous location but the exporter will be looking for them in a new location.

Maybe we have to add the barman_env.sh and set some defaults there or in the Dockerfile before merging this to take care of that issue.

Mind sharing your use case for this project? Are you doing backups with it? If you have more ideas but don't have the time to code them just open issues and contribute to the backlog.

[lhoss]

Maybe we have to add the barman_env.sh and set some defaults there or in the Dockerfile before merging this to take care of that issue.

yeah I'm perfectly aware that this needs additional work (like introducing the proposed barman_env.sh), just that it's difficult for me because I'm not familiar with the whole solution ( the docker-barman container), but only the exporter that is highly useful for me (just required the small changes I pushed upstream)

Mind sharing your use case for this project? Are you doing backups with it? If you have more ideas but don't have the time to code them just open issues and contribute to the backlog.

Yeah, in our case we have decided to have a 3 (now 4)-in-1 (container) solution build on the mdillon/postgis image, added barman and finally 2 prometheus exporters (the existing opensource 'postgres_exporter', plus your prom_exporter ). Sure glad to add new issues if I have ideas, one I added today about logging, that I think could be really useful when doing new dev cycles 👍

[rsilva4-zz]

Ok, that made things clearer. I don't see that this PR pending merge affect you right now, right?

If so it will wait for me to have an available time slot for work on it.

[Tom-Fawcett]

@rsilva4 I would like for this PR to progress :smiley:.

I'm using prom_exporter.py independent of the rest of this repo.

I am happy to patch my local copy of the script, but it would be nice to have the fix upstreamed.

[rsilva4-zz]

@mariomourao can you please confirm that this change does not have impact on your services? Last time I took a look at it I didn't see a problem but I didn't had the time to test it properly.