stub42
commented
3 years ago OS command line length limits are being violated, per https://bugs.launchpad.net/bugs/1274460
Closed mkalcok closed 3 years ago
stub42
commented
3 years ago OS command line length limits are being violated, per https://bugs.launchpad.net/bugs/1274460
pengale
commented
3 years ago It looks like the Juju core bug is pretty old, and has been deprioritized.
Should we just truncate overly long lines in the operator framework? That might make the code a bit less pretty (we'll have to separate the construction of the logging string from the actual call out to the system), but I think that it might be the right thing to do.
pengale
commented
3 years ago Researching this a bit, it looks like the problem may be less with the maximum length of a line in bash (getconfig ARG_MAX), and more with the max length of a single argument passed to bash, which is often 131071. (Painstaking discussion here: https://www.in-ulm.de/~mascheck/various/argmax/)
I'm a little bit tempted to make a change like this:
--- a/ops/model.py
+++ b/ops/model.py
@@ -38,6 +38,8 @@ from ops.jujuversion import JujuVersion
logger = logging.getLogger(__name__)
+MAX_STR_LEN = 131071
+
class Model:
"""Represents the Juju Model as seen from this unit.
@@ -1589,7 +1591,7 @@ class _ModelBackend:
self._run('application-version-set', '--', version)
def juju_log(self, level, message):
- self._run('juju-log', '--log-level', level, "--", message)
+ self._run('juju-log', '--log-level', level, "--", message[:MAX_STR_LEN])
def network_get(self, binding_name, relation_id=None):
"""Return network info provided by network-get for a given binding.Basically, we just truncate any log messages that are longer than 131071. (And hope that the addition of quotes or whatever doesn't break things -- we could lop off an additional ten characters or so if we were paranoid.)
What say the rest of you? Sound like a decent plan?
mkalcok
commented
3 years ago How about splitting the message into multiple MAX_STR_LEN-sized strings and calling juju-log multiple times? Maybe prefacing this split output with one more message (with same log-level?) indicating that following message was too long and was split to multiple parts?
pengale
commented
3 years ago @mkalcok Splitting into multiple lines makes sense. It's strangely less satisfying -- I guess that I just like tossing extra stuff away, when I can get away with it. But much more friendly to the person who might need the data from a big log dump :-)
mkalcok
commented
3 years ago @petevg yup..gigantic logs are obviously not ideal but sometimes there's no other way. Especially if the logs come from underlying library (like it was in my case) and charm author has no control over them but needs their output to debug/solve the issue.
jameinel
commented
3 years ago I thought the actual param size was closer to 32kb which is why people trip over it. (especially with env vars, etc). But maybe that is bash and not exec. Arguably Juju shouldn't allow you to be creating log entries that are 100s of KB in size in the first place. (stack traces come to mind as something that is fairly unbounded and can easily cause a problem). It wouldn't be hard on the juju side to support "echo foo | juju-log -l INFO", though you then run into compatibility questions (I'm using juju 2.8 vs 2.9.X that happens to support stdin.)
On Wed, Sep 29, 2021, 17:49 Pete Vander Giessen @.***> wrote:
Researching this a bit, it looks like the problem may be less with the maximum length of a line in bash (getconfig ARG_MAX), and more with the max length of a single argument passed to bash, which is often 131071. (Painstaking discussion here: https://www.in-ulm.de/~mascheck/various/argmax/)
I'm a little bit tempted to make a change like this:
--- a/ops/model.py +++ b/ops/model.py @@ -38,6 +38,8 @@ from ops.jujuversion import JujuVersion
logger = logging.getLogger(name)
+MAX_STR_LEN = 131071 +
class Model: """Represents the Juju Model as seen from this unit. @@ -1589,7 +1591,7 @@ class _ModelBackend: self._run('application-version-set', '--', version)
def juju_log(self, level, message):
- self._run('juju-log', '--log-level', level, "--", message)
self._run('juju-log', '--log-level', level, "--", message[:MAX_STR_LEN])
def network_get(self, binding_name, relation_id=None): """Return network info provided by network-get for a given binding.
Basically, we just truncate any log messages that are longer than 131071. (And hope that the addition of quotes or whatever doesn't break things -- we could lop off an additional ten characters or so if we were paranoid.)
What say the rest of you? Sound like a decent plan?
— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/canonical/operator/issues/531#issuecomment-930570203, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABRQ7N2SOQNZN6FWBINIHLUEOCWBANCNFSM442CMBYQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.
I encountered problem where
_ModelBackend.juju_log()threw an exception while logging message from underlying python modules. This happened to me when I was usingkubernetespython client to create CRDs from yaml.For reference: this is the CRD file I was deploying. After setting loglevel for
kubernetesmodule toINFO, to avoid thatlogger.debug()call, everything started to work again.