common: enable log to journald by huww98 · Pull Request #39738 · ceph/ceph

huww98 · 2021-02-27T18:02:52Z

Enable ceph daemons to directly send logs to journald via unix domain socket.

While sending logs, metadata like priority, thread, timestamp is sent as structured data. And can be queried by journalctl.

Note that I don't use libsystemd because I want the implementation to be as efficient as possible.

We can work around the issue stated in #39547 if we let daemon talk to journald directly

Checklist

References tracker ticket
Updates documentation if necessary
Includes tests for new functionality or reproducer for bug

I have used vstart.sh to verify that this works on my PC. Since this depends on external service journald, could you comment on whether I need to write tests? If so, how?

Show available Jenkins commands

jenkins retest this please
jenkins test classic perf
jenkins test crimson perf
jenkins test signed
jenkins test make check
jenkins test make check arm64
jenkins test submodules
jenkins test dashboard
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox

batrick

comments so far

src/common/legacy_config_opts.h

src/common/ceph_context.cc

src/common/Journald.cc

src/common/Journald.h

src/log/Log.cc

sebastian-philipp · 2021-03-01T11:09:22Z

Speaking of cephadm here. In the new world™ of systemd, daemons are supposed to write to stdout. And stdout is then supposed to end up in journald. For cephadm, I'd like to keep in this way. I mean, to some degree, this PR is a workaround for containers/conmon#242 + https://tracker.ceph.com/issues/49551 .

On the other hand, I can see that this log driver might be useful for folks in general. Especially the added metadata looks interesting. Just out of curiosity, where are the benefits for cephadm users? Maybe you want to add this beneath https://docs.ceph.com/en/latest/cephadm/operations/#logging-to-stdout ?

huww98 · 2021-03-01T11:32:04Z

@sebastian-philipp For traditional daemons, they can detect whether the stdout is connected to journald. If yes, then send additional metadata. However, it can be hard for daemons in containers to detect whether they should send additional metadata. Take podman as an example. The stdout of daemons is connected to conmon, the conmon then send the content to log files / journald and the stdout of the podman process (if not detached).

For cephadm users, I think we can mount journald socket into the container, then enable the new "logging to journald" feature in this PR.

huww98 · 2021-03-03T06:37:12Z

Apart from the above comments, I also made some other changes:

Add fall back for kernels that do not support memfd_create (< 3.17)
Add a small benchmark program
Add some unit tests.

src/common/Journald.cc

batrick · 2021-03-04T23:17:07Z

src/common/Journald.cc

+    return LOG_NOTICE;
+  if (ceph_prio < 10)
+    return LOG_INFO;
+  return LOG_DEBUG;


I've been undecided on the wisdom of spewing Ceph debug logs to the systemd journal (with cephadm deployed containers or otherwise). Is this really recommended practice?

The mapping you've done is interesting but not necessarily wrong. I think if we have debug_ms=5 you will get some amusing spam in the journal which pretends to be important.

I've been undecided on the wisdom of spewing Ceph debug logs to the systemd journal

Why not. journald itself can do filtering both on collecting time and viewing time based on its structured metadata. And we are already doing this with the default cephadm deployment.

I think if we have debug_ms=5 you will get some amusing spam in the journal which pretends to be important.

prio=5 is mapped to LOG_INFO, which is the default level if we just connect the stderr to journald. I think it is OK. Anyway, if someone don’t like some logs, they can do the filtering themselves. This can be easier compared with logging to files.

By the way, the journald driver of conmon log everything at LOG_ERROR level. I think this is bad.

@liewegas and @sebastian-philipp what do you think? i think this change is a nice improvement. but i am not sure regarding @batrick 's question:

Is this really recommended practice?

As we log to journald instead of a file, we need to be able to write anything (including debugs) to journald that we would write to the files.

Btw we have a similar function in Python already:

ceph/src/pybind/mgr/mgr_module.py

Lines 663 to 679 in 3ba59e9

def _ceph_log_level_to_python(self, log_level: str) -> str:

if log_level:

try:

ceph_log_level = int(log_level.split("/", 1)[0])

except ValueError:

ceph_log_level = 0

else:

ceph_log_level = 0

log_level = "DEBUG"

if ceph_log_level <= 0:

log_level = "CRITICAL"

elif ceph_log_level <= 1:

log_level = "WARNING"

elif ceph_log_level <= 4:

log_level = "INFO"

return log_level

As we log to journald instead of a file, we need to be able to write anything (including debugs) to journald that we would write to the files.

This is not necessarily true. We could have a debug log for true "debug" information and a status/error/audit log go to systemd journal. Merging them is convenient for programmers but not necessarily a good thing for users.

Sending log messages to different targets based on the level is something that none of the logging code currently supports. I'm not sure if it makes sense or not, but I think even if we decide to go down that road it's an orthogonal change to this one

tchaikov

this change does not compile. see https://jenkins.ceph.com/job/ceph-dev-new-build/ARCH=x86_64,AVAILABLE_ARCH=x86_64,AVAILABLE_DIST=bionic,DIST=bionic,MACHINE_SIZE=gigantic/52590//consoleFull

Enable ceph daemons to directly send logs to journald via unix domain socket. While sending logs, metadata like priority, thread, timestamp is sent as structured data. And can be queried by journalctl. Note that I don't use libsystemd because I want the implementation to be as efficient as possible. Signed-off-by: 胡玮文 <[email protected]>

Use the same infrastructure used for local logs. Also with structured metadata. Signed-off-by: 胡玮文 <[email protected]>

Simple program to send a lot log messages to journald. On my PC, it runs at about 35000 log entries per second. Signed-off-by: 胡玮文 <[email protected]>

These tests are skipped if journald is not run. Signed-off-by: 胡玮文 <[email protected]>

tchaikov · 2021-03-12T12:09:42Z

huww98 · 2021-04-05T08:14:29Z

Hi, what else can I do? As we already sending every log to journald via stderr when deployed with cephadm, I think we don't need to worry about sending too many logs to journald. Besides, this feature is opt-in.

If this PR and #40025 get merged, I will plan another PR to enable this by default when deployed with cephadm. (Seems #40025 is nessesary for these logs to be associated with the corresponding systemd unit.

ivancich · 2021-04-05T21:28:08Z

src/common/Journald.h is breaking our Windows builds. For example, see: https://jenkins.ceph.com/job/ceph-dev-new-build/ARCH=x86_64,AVAILABLE_ARCH=x86_64,AVAILABLE_DIST=windows,DIST=windows,MACHINE_SIZE=gigantic/53543/consoleFull#-62692307e4bfde06-44c1-4b3c-8379-d9ee175fb257 .

tchaikov · 2021-04-06T04:59:54Z

@ivancich thanks Eric, i am working on a fix to address the FTBFS.

tchaikov · 2021-04-06T05:19:40Z

#40607

ivancich · 2021-04-06T17:55:27Z

@tchaikov Thanks for the quick fix, Kefu.

wjwithagen · 2021-04-07T19:49:10Z

@huww98 @tchaikov
What would be the easiest way to "undo" this journald stuff on platforms that do not have journald and all the memfd stuff.
Now this block building on at least FreeBSD.

Perhaps the easiest way is to stub-out all routines in Journald.cc ??

ivancich · 2021-04-07T20:20:55Z

@huww98 @tchaikov
What would be the easiest way to "undo" this journald stuff on platforms that do not have journald and all the memfd stuff.
Now this block building on at least FreeBSD.

Perhaps the easiest way is to stub-out all routines in Journald.cc ??

@wjwithagen: Have you seen @tchaikov's Windows fix in PR #40607? I would guess could be adapted for your case.

wjwithagen · 2021-04-07T21:56:43Z

@ivancich
I had not, but it is a good suggestion...
Had found HAVE_MSGHDR, but onfortunately that is also available in the FreeBSD includes.
But thanx for the useful hint.

github-actions bot added build/ops common core mon labels Feb 27, 2021

huww98 force-pushed the journald branch from 41993cd to 9cb558d Compare February 28, 2021 04:33

huww98 requested a review from a team as a code owner February 28, 2021 04:33

github-actions bot added the crimson label Feb 28, 2021

tchaikov requested review from tchaikov and removed request for a team February 28, 2021 04:45

tchaikov removed the crimson label Feb 28, 2021

huww98 force-pushed the journald branch from 9cb558d to 4c8c9b1 Compare February 28, 2021 16:05

github-actions bot added the crimson label Feb 28, 2021

batrick self-requested a review February 28, 2021 21:59

batrick requested changes Feb 28, 2021

View reviewed changes

src/common/legacy_config_opts.h Outdated Show resolved Hide resolved

src/common/ceph_context.cc Outdated Show resolved Hide resolved

tchaikov reviewed Mar 1, 2021

View reviewed changes

src/common/Journald.cc Outdated Show resolved Hide resolved

tchaikov reviewed Mar 1, 2021

View reviewed changes

tchaikov removed the crimson label Mar 3, 2021

huww98 force-pushed the journald branch from 4c8c9b1 to b9295a8 Compare March 3, 2021 06:20

github-actions bot added crimson tests labels Mar 3, 2021

huww98 force-pushed the journald branch from b9295a8 to dd69497 Compare March 3, 2021 07:17

huww98 requested review from batrick and tchaikov March 3, 2021 08:16

batrick reviewed Mar 4, 2021

View reviewed changes

huww98 force-pushed the journald branch from dd69497 to 3d6dd13 Compare March 5, 2021 09:55

huww98 requested a review from batrick March 5, 2021 12:20

tchaikov approved these changes Mar 8, 2021

View reviewed changes

tchaikov added needs-qa wip-kefu-testing and removed needs-qa wip-kefu-testing labels Mar 8, 2021

tchaikov requested changes Mar 9, 2021

View reviewed changes

huww98 added 4 commits March 9, 2021 20:14

common: enable sending cluster log to journald

e05900b

Use the same infrastructure used for local logs. Also with structured metadata. Signed-off-by: 胡玮文 <[email protected]>

test: benchmark journald logger

eadfcd1

Simple program to send a lot log messages to journald. On my PC, it runs at about 35000 log entries per second. Signed-off-by: 胡玮文 <[email protected]>

test/common: unit tests for journald logger

1e109e8

These tests are skipped if journald is not run. Signed-off-by: 胡玮文 <[email protected]>

huww98 force-pushed the journald branch from 3d6dd13 to 1e109e8 Compare March 9, 2021 12:34

huww98 requested a review from tchaikov March 10, 2021 00:37

tchaikov added the wip-kefu-testing label Mar 11, 2021

tchaikov requested a review from liewegas March 12, 2021 12:10

tchaikov removed the wip-kefu-testing label Mar 12, 2021

tchaikov approved these changes Mar 12, 2021

View reviewed changes

liewegas approved these changes Apr 5, 2021

View reviewed changes

batrick approved these changes Apr 5, 2021

View reviewed changes

tchaikov merged commit 9ef8055 into ceph:master Apr 5, 2021

huww98 deleted the journald branch April 6, 2021 15:21

huww98 mentioned this pull request Apr 7, 2021

cephadm: enable log to journald by default #40640

Merged

3 tasks

josh mentioned this pull request Sep 4, 2025

ceph: compile with systemd support flag NixOS/nixpkgs#440226

Merged

13 tasks

	def _ceph_log_level_to_python(self, log_level: str) -> str:
	if log_level:
	try:
	ceph_log_level = int(log_level.split("/", 1)[0])
	except ValueError:
	ceph_log_level = 0
	else:
	ceph_log_level = 0

	log_level = "DEBUG"
	if ceph_log_level <= 0:
	log_level = "CRITICAL"
	elif ceph_log_level <= 1:
	log_level = "WARNING"
	elif ceph_log_level <= 4:
	log_level = "INFO"
	return log_level

Conversation

huww98 commented Feb 27, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Checklist

Uh oh!

batrick left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

sebastian-philipp commented Mar 1, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

huww98 commented Mar 1, 2021

Uh oh!

huww98 commented Mar 3, 2021

Uh oh!

Uh oh!

Uh oh!

batrick Mar 4, 2021

Choose a reason for hiding this comment

Uh oh!

huww98 Mar 5, 2021

Choose a reason for hiding this comment

Uh oh!

tchaikov Mar 12, 2021

Choose a reason for hiding this comment

Uh oh!

sebastian-philipp Mar 12, 2021

Choose a reason for hiding this comment

Uh oh!

batrick Mar 12, 2021

Choose a reason for hiding this comment

Uh oh!

liewegas Apr 5, 2021

Choose a reason for hiding this comment

Uh oh!

tchaikov left a comment

Choose a reason for hiding this comment

Uh oh!

tchaikov commented Mar 12, 2021

Uh oh!

huww98 commented Apr 5, 2021 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ivancich commented Apr 5, 2021

Uh oh!

tchaikov commented Apr 6, 2021

Uh oh!

tchaikov commented Apr 6, 2021

Uh oh!

ivancich commented Apr 6, 2021

Uh oh!

wjwithagen commented Apr 7, 2021

Uh oh!

ivancich commented Apr 7, 2021

Uh oh!

wjwithagen commented Apr 7, 2021

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

7 participants

huww98 commented Feb 27, 2021 •

edited

Loading

sebastian-philipp commented Mar 1, 2021 •

edited

Loading

huww98 commented Apr 5, 2021 •

edited

Loading