Skip to content
This repository has been archived by the owner. It is now read-only.

[dev.icinga.com #4519] add support for gzip compressed logs #1326

Closed
icinga-migration opened this issue Aug 6, 2013 · 17 comments
Closed

[dev.icinga.com #4519] add support for gzip compressed logs #1326

icinga-migration opened this issue Aug 6, 2013 · 17 comments
Milestone

Comments

@icinga-migration
Copy link
Member

@icinga-migration icinga-migration commented Aug 6, 2013

This issue has been migrated from Redmine: https://dev.icinga.com/issues/4519

Created by formorer on 2013-08-06 13:21:04 +00:00

Assignee: ricardo
Status: Resolved (closed on 2014-02-19 12:05:10 +00:00)
Target Version: 1.11
Last Update: 2014-12-08 09:15:31 +00:00 (in Redmine)


It would be nice if icinga cgi would be able to read compressed logs.
That whould save me/us some space on disk.

Changesets

2013-12-26 01:23:11 +00:00 by ricardo cf5f403

classic-ui: add support for gzip compressed logs #4519

This commit adds support to read gzip compressed log files
in cgis. It also adds a new cgi.cfg option "read_gzip_logs"
which is disabled by default for now. If read_gzip_logs is
switched on and your compressed log files are still quite
big, it could have a negative impact on the performance of
systems under havy load as every file has to be decompressed.

If you have a small environment, you can switch on
"read_gzip_logs" and compress your logs under archives with
a cronjob every night and save some space.

You can also disable the reading of compressed logs via
configure with "--disable-compressed-logs".

This is my little xmas preset to the icinga community ;-)

refs: #4519
whatthecommit: Well, it's doing something.

2013-12-26 10:16:27 +00:00 by ricardo d6698cf

classic-ui: fixed add support for gzip compressed logs #4519

well it was late, should work now.

refs: #4519
whatthecommit: Nothing to see here, move along

2014-01-03 22:17:28 +00:00 by ricardo bb2b0b3

classic-ui: added config option read_gzip_logs to config.cgi #4519

refs: #4519

2014-02-18 22:46:07 +00:00 by ricardo 991e84c

Merge branch 'feature/support-compressed-logs-4519' into next

Fixes: #4519
Conflicts:
	Changelog
@icinga-migration
Copy link
Member Author

@icinga-migration icinga-migration commented Aug 6, 2013

Updated by formorer on 2013-08-06 13:21:18 +00:00

  • Priority changed from Normal to Low
@icinga-migration
Copy link
Member Author

@icinga-migration icinga-migration commented Sep 11, 2013

Updated by ricardo on 2013-09-11 21:24:53 +00:00

  • Status changed from New to Feedback

hi formorer,

I thought about this as well. But I fear it will take muuuch longer to view logs if the searched time span gets bigger. Especially in big environments. And then the users would start to complain again, that all cgis using historical data are too slow.

Or how do you see it?

@icinga-migration
Copy link
Member Author

@icinga-migration icinga-migration commented Sep 13, 2013

Updated by tk on 2013-09-13 06:28:13 +00:00

I would highly appreciate if the cgi would be able to read compressed logs.

Maybe it would make sense to add a check like "the next log file is compressed, so make the 'Latest Archive'/'Earlier Archive'/'More Recent Archive' arrow yellow instead of green, and if the user moves the mouse over it, display a warning 'Archive is compressed, reading may be slow'".
Actually, if you now move the mouse on the arrow, it displays 'Latest Archive'/'Earlier Archive'/'More Recent Archive', so just add the warning to this ALT text.

@icinga-migration
Copy link
Member Author

@icinga-migration icinga-migration commented Sep 14, 2013

Updated by mfriedrich on 2013-09-14 18:04:10 +00:00

i fear the performance impact too on large log files (imagine external commands / passive check results being logged) and a large timespan.

and the code making this happen isn't elasticsearch, or comparable. imho one could hack a prototype and testdrive that longterm in larger environments. but not for 1.10 where feature freeze happens in ~2 weeks.

@icinga-migration
Copy link
Member Author

@icinga-migration icinga-migration commented Sep 16, 2013

Updated by tk on 2013-09-16 07:02:16 +00:00

dnsmichi wrote:

i fear the performance impact too on large log files (imagine external commands / passive check results being logged) and a large timespan.

Why not let the user decide whether he can live with the performance loss?

and the code making this happen isn't elasticsearch, or comparable. imho one could hack a prototype and testdrive that longterm in larger environments. but not for 1.10 where feature freeze happens in ~2 weeks.

Not having had a look at the source code so far my understanding may be wrong, but isn't the difference "instead of open(filename, ...), just use 'if (-f filename) {open(...)} else { /* add .gz suffix to filename, then use zlib's gzopen(...) */ }'"? Might still be too much work to do it before the feature freeze because of other priorities though.

@icinga-migration
Copy link
Member Author

@icinga-migration icinga-migration commented Sep 16, 2013

Updated by mfriedrich on 2013-09-16 08:28:58 +00:00

tk wrote:

dnsmichi wrote:
> i fear the performance impact too on large log files (imagine external commands / passive check results being logged) and a large timespan.

Why not let the user decide whether he can live with the performance loss?

been there, done that. reverts/additional config options are the result of that. trust me, new features are fun. until they generate bugs and issues. so keeping the contra in the first place on the discussion is my part of the story. feel free to convince me ;-)

> and the code making this happen isn't elasticsearch, or comparable. imho one could hack a prototype and testdrive that longterm in larger environments. but not for 1.10 where feature freeze happens in ~2 weeks.

Not having had a look at the source code so far my understanding may be wrong, but isn't the difference "instead of open(filename, ...), just use 'if (-f filename) {open(...)} else { /* add .gz suffix to filename, then use zlib's gzopen(...) */ }'"? Might still be too much work to do it before the feature freeze because of other priorities though.

Sure, why not. If you got a patch, send it for prototyping. I am just saying that feature freeze happens soon, and our quality assurance guidelines don't allow us to add experimental features afterwards (unless they remain on seperated feature/* branches for the next release, i.e. 1.11).
Afterall I would've done it on the weekend, but I was too stuffed with fixing the rest for the roadmap for 1.10 ...

@icinga-migration
Copy link
Member Author

@icinga-migration icinga-migration commented Oct 4, 2013

Updated by ricardo on 2013-10-04 09:43:13 +00:00

  • Target Version set to 1.11

will add a cgi.cfg option with a warning on the performance impact. Then the users can't say they didn't know about it.

@icinga-migration
Copy link
Member Author

@icinga-migration icinga-migration commented Oct 4, 2013

Updated by formorer on 2013-10-04 09:45:49 +00:00

Perfect!

Thanks

@icinga-migration
Copy link
Member Author

@icinga-migration icinga-migration commented Oct 11, 2013

Updated by tk on 2013-10-11 09:08:24 +00:00

Cool!

Thanks from me as well!

@icinga-migration
Copy link
Member Author

@icinga-migration icinga-migration commented Dec 26, 2013

Updated by ricardo on 2013-12-26 01:30:34 +00:00

  • Subject changed from Support gzip compressed logs to add support for gzip compressed logs
  • Assigned to set to ricardo
  • Priority changed from Low to Normal
  • Done % changed from 0 to 80

in current "feature/support-compressed-logs-4519"

please test everyone!!

@icinga-migration
Copy link
Member Author

@icinga-migration icinga-migration commented Jan 3, 2014

Updated by ricardo on 2014-01-03 23:02:40 +00:00

testers? any testers?

@icinga-migration
Copy link
Member Author

@icinga-migration icinga-migration commented Jan 24, 2014

Updated by tk on 2014-01-24 15:36:41 +00:00

Well, back from "killing backlog after vacation". I'd like to test, but I think I'll need some assistance / pointers.
which repositories to check out, maybe a few pointers on how to test... Can you help me with this?

@icinga-migration
Copy link
Member Author

@icinga-migration icinga-migration commented Jan 24, 2014

Updated by mfriedrich on 2014-01-24 15:39:31 +00:00

https://github.com/Icinga/icinga-core/tree/feature/support-compressed-logs-4519

@icinga-migration
Copy link
Member Author

@icinga-migration icinga-migration commented Feb 1, 2014

Updated by tk on 2014-02-01 19:16:53 +00:00

OK, it took a few days, but now I'm back from testing.
Short report: Everything worked like a charm for me. Except for the fact that it was a bad idea to just "use an existing SLES VM", as it had not all requirements for compiling icinga (and they were also missing in the repository, encluding the SLES SDK). So I created a new (debian) VM, installed all the necessary prerequisites and compiled the whole thing. Apart from me messing up --prefix, all went fine. Compiling, installing, running.
Then I needed to run the VM for a few days in order to see the logs rotate ;-)
Now I just tested

  • uncompressed logs: runs fine, with both "read_gzip_logs" set to "0" and to "1"
  • compressed logs: runs as expected (can be read with "read_gzip_logs" set to "1" but can't with the default "0")

So thanks for implementing this :-)

@icinga-migration
Copy link
Member Author

@icinga-migration icinga-migration commented Feb 18, 2014

Updated by ricardo on 2014-02-18 22:51:42 +00:00

  • Done % changed from 80 to 90

Thanks you and Carl for testing.

Merged it to next branch.

@icinga-migration
Copy link
Member Author

@icinga-migration icinga-migration commented Feb 19, 2014

Updated by ricardo on 2014-02-19 12:05:10 +00:00

  • Status changed from Feedback to Resolved
  • Done % changed from 90 to 100

Applied in changeset icinga-core:991e84c2e47dc3dede7506ffcf946b012c10971a.

@icinga-migration
Copy link
Member Author

@icinga-migration icinga-migration commented Dec 8, 2014

Updated by mfriedrich on 2014-12-08 09:15:31 +00:00

  • Project changed from 19 to Core, Classic UI, IDOUtils
  • Category set to Classic UI
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Linked pull requests

Successfully merging a pull request may close this issue.

None yet
1 participant
You can’t perform that action at this time.