Conversation
2b16808 to
763b94e
Compare
763b94e to
4e4407d
Compare
|
For event handlers, I prefer to set a logger prop on charm, and use |
zmraul
left a comment
zmraul
left a comment
There was a problem hiding this comment.
I would either move all logging through charm.logger or none. Because right now we have "normal" logging in several places. So I would instead clearly migrate to a custom logging for all modules.
There is then the issue where all logs controlled by the libs are not routed through this, which includes our own libs. Not sure if there is a global override for this in python. Maybe we can wire something through JujuLogHandler in ops instead?
| *self.cluster.internal_user_credentials.values(), | ||
| *self.cluster.client_passwords.values(), | ||
| self.cluster.s3_credentials.get("access-key", ""), | ||
| self.cluster.s3_credentials.get("secrety-key", ""), |
There was a problem hiding this comment.
| self.cluster.s3_credentials.get("secrety-key", ""), | |
| self.cluster.s3_credentials.get("secret-key", ""), |
| self.unit_server.certificate, | ||
| self.unit_server.ca, |
There was a problem hiding this comment.
nit: I don't think these two are needed, but not a big deal
There was a problem hiding this comment.
Me neither, but the last time I asked Mykola he had a different opinion, so let's keep'em
Unfortunately this is not possible, since we only pass the charm object to event handlers, and that is the correct pattern. The core, managers and workloads should have their own logging facility imo, and not be coupled with
If a lib logs sensitive content, that's the lib's issue and it should be patched by the lib maintainer imo. In that hypothetical scenario, our mitigation would be to silence the lib's logging through standard logging methods. |
2 participants
Description
Adds a RedactionFilter to loggers, which may log sensitive content.
This PR is backed by an analysis of source code done using Claude Code.
Logging Analysis
Dynamic log messages in
srcLog call sites whose message text varies at runtime. 36 of the 65
loggercalls insrc.Group A — f-string interpolation (21)
Literal string with variables interpolated into it. Stable prefix, so greppable.
src/charm.py:305{self.unit.name} restarting...src/charm.py:348{self.unit.name} initializing...src/charm.py:382{self.unit.name} startedsrc/charm.py:412{updated_servers=}src/charm.py:440...switching to {self.state.cluster.quorum} quorumsrc/charm.py:470Skipping update of {client.component.name}...src/core/cluster.py:440Can't retrieve network binding data: {e}src/core/models.py:59Fields {list(items.keys())} were attempted...src/managers/backup.py:95Using existing bucket {bucket_name}src/managers/backup.py:100Created bucket {bucket_name}src/managers/config.py:405src/managers/config.py:415src/managers/config.py:426{self.log_level}src/managers/quorum.py:192{leader_chroots=}src/managers/quorum.py:213{sasl_acl=}src/managers/quorum.py:224CREATE CHROOT - {client.database}src/managers/quorum.py:228UPDATE CHROOT - {client.database}src/events/backup.py:73{missing_required_parameters}src/events/upgrade.py:112{self.charm.unit.name} upgrading workload...src/events/password_actions.py:65{...upgrade_stack}(msg built at :61-64)src/events/password_actions.py:72{CHARM_USERS},{username}(msg built at :71)Group B — bare variable, no literal text (15)
The whole message is an exception or attribute. No static text to grep for.
src/managers/tls.py:89e.stdoutsrc/managers/tls.py:173str(e.stdout)src/managers/tls.py:190str(e.stdout)src/managers/tls.py:273str(e.stdout)src/managers/tls.py:284str(e.stdout)src/workload.py:38str(e)(.exception)src/workload.py:45str(e)(.exception)src/workload.py:52str(e)(.exception)src/workload.py:142str(e)src/managers/quorum.py:85str(e)src/managers/quorum.py:154str(e)src/events/provider.py:76str(e)src/events/upgrade.py:130e.causesrc/managers/k8s.py:46e.status.messagesrc/core/cluster.py:468e(object passed directly, not stringified)