Breakpad Crash Handler by irq0 · Pull Request #61264 · ceph/ceph

irq0 · 2025-01-08T13:43:58Z

This adds Google Breakpad as another option to handle crashes.

In terms of crash dump fidelity it is close to coredumps+system metadata, but at a fraction of the size. Typically 10s of MB.

Why Breakpad?

The generated minidumps are smaller, but still contain valuable data like all thread backtraces
It is widely adopted by OpenTTD, Chromium, Firefox, and many more
There is great tooling around processing minidumps ranging from Sentry to the https://github.com/rust-minidump suite of command line tools

Demo

https://asciinema.org/a/KMiIt0XKkgsE2s1kaRiuqMeUm

Demos the whole process from symbol file generation, crashing a daemon, to looking at the minidump.

Tools: dump_syms minidump-stackwalk
https://github.com/rust-minidump https://github.com/mozilla/dump_syms

References

https://chromium.googlesource.com/breakpad/breakpad/+/HEAD/docs/getting_started_with_breakpad.md

https://chromium.googlesource.com/breakpad/breakpad

Checklist

Tracker (select at least one)
- References tracker ticket
- Very recent bug; references commit where it was introduced
- New feature (ticket optional)
- Doc update (no ticket needed)
- Code cleanup (no ticket needed)
Component impact
- Affects Dashboard, opened tracker ticket
- Affects Orchestrator, opened tracker ticket
- No impact that needs to be tracked
Documentation (select at least one)
- Updates relevant documentation
- No doc update is appropriate
Tests (select at least one)
- Includes unit test(s)
- Includes integration test(s)
- Includes bug reproducer
- No tests

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 dashboard cephadm
jenkins test api
jenkins test docs
jenkins render docs
jenkins test ceph-volume all
jenkins test ceph-volume tox
jenkins test windows
jenkins test rook e2e

src/CMakeLists.txt

adamemerson · 2025-01-08T16:17:53Z

I think you have to put the word 'submodule' somewhere in the commit message where you update the submodule.

adamemerson · 2025-01-08T16:18:50Z

jenkins test make check

adamemerson · 2025-01-08T16:59:20Z

Overall I'm very much in favor of this once you get it compiling everywhere.

irq0 · 2025-01-13T12:09:45Z

jenkins test make check

irq0 · 2025-01-14T08:36:34Z

jenkins test make check

irq0 · 2025-01-16T16:13:51Z

Overall I'm very much in favor of this once you get it compiling everywhere.

Tests just passed 🥳. The last push added a build option and ifdefs.
The default is set to not build on windows.

adamemerson · 2025-01-17T19:06:40Z

@mattbenjamin @chardan I like this idea and pulling in a better crash handler was something I'd been thinking of off and on. Would you take a look?

chardan

This is a useful facility to have! Thanks for your work on it. LGTM.

chardan · 2025-01-17T21:01:24Z

src/global/global_init.cc

+#ifdef HAVE_BREAKPAD
+  if (g_conf()->breakpad) {
+    google_breakpad::MinidumpDescriptor descriptor(g_conf()->crash_dir);
+    g_ceph_context->_ex_handler.reset(


This is fine, but also consider:
_ex_handler = std::make_unique<google_breakpad::ExceptionHandler>(...your args...);

chardan · 2025-01-17T21:26:08Z

One question I do have: should this have any documentation? An update to -D options listed somewhere?

irq0 · 2025-01-20T15:25:11Z

One question I do have: should this have any documentation? An update to -D options listed somewhere?

What kind of docs do you have in mind?

I could do a writeup of the demo linked above. This is however just one way to use the feature. A Sentry-based workflow would look quite different.

chardan · 2025-01-20T15:42:55Z

@irq0 I don't think we want to repeat the docs that come with the tool itself so much, so much as the install guide in README.md if there seemed to be an appropriate section to mention it. I didn't have anything too extensive in mind; and, lots of things are AFAIK currently discovered by looking through the build scripts.

chardan · 2025-01-20T16:13:09Z

@adamemerson @mattbenjamin Any other thoughts?

chardan · 2025-01-22T20:33:16Z

@irq0 I'm trying to get this into the test lab, but there are some technical issues. Thanks for your patience!

irq0 · 2025-01-23T15:43:26Z

@irq0 I'm trying to get this into the test lab, but there are some technical issues. Thanks for your patience!

Sure, let me know if I can help.

chardan · 2025-01-28T20:56:59Z

Current Shaman build fails with breakpad annotations:
https://shaman.ceph.com/builds/ceph/wip-irq0-breakpad+/91a9b194ca2eeb62671c10eef1f6aedaebfe4a07/default/421581/

adamemerson · 2025-01-28T21:15:35Z

Some of them are just vanilla failures.

g++ -DHAVE_CONFIG_H -I. -I./src  -I./src   -Wmissing-braces -Wnon-virtual-dtor -Woverloaded-virtual -Wreorder -Wsign-compare -Wunused-local-typedefs -Wunused-variable -Wvla -Werror -fPIC -O2 -flto=auto -ffat-lto-objects -fexceptions -g -grecord-gcc-switches -pipe -Wall -Werror=format-security -Wp,-D_FORTIFY_SOURCE=2 -Wp,-D_GLIBCXX_ASSERTIONS -specs=/usr/lib/rpm/redhat/redhat-hardened-cc1 -fstack-protector-strong   -m64 -march=x86-64-v2 -mtune=generic -fasynchronous-unwind-tables -fstack-clash-protection -fcf-protection -MT src/common/string_conversion.o -MD -MP -MF $depbase.Tpo -c -o src/common/string_conversion.o src/common/string_conversion.cc &&\
mv -f $depbase.Tpo $depbase.Po
src/common/string_conversion.cc: In function ‘google_breakpad::UTF32ToUTF16Char(wchar_t, unsigned short*)’:
src/common/string_conversion.cc:107:16: error: array subscript 2 is outside array bounds of ‘wchar_t[1]’ [-Werror=array-bounds]
  107 |   const UTF32* source_end_ptr = source_ptr + 1;
      |                ^~~~~~~~~~~~~~
src/common/string_conversion.cc:105:31: note: while referencing ‘in’
  105 | void UTF32ToUTF16Char(wchar_t in, uint16_t out[2]) {
      |                       ~~~~~~~~^~
cc1plus: all warnings being treated as errors

Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com>

Add option WITH_BREAKPAD defaulting to TRUE on non-windows platforms. Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com>

Only build breakpad code if we build with breakpad support. Print warning if breakpad enabled in config, but not built. Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com>

Explicitly disable incompatible `-Warray-bounds` and `-Wmaybe-uninitialized`. These aren't normally enabled as compile-failing errors, except when building RPMs on CentOS9. Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com>

Test that with enabled Breakpad crash handling Ceph creates correct minidumps. Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com>

chardan · 2025-05-13T18:26:06Z

jenkins test api

irq0 · 2025-06-02T14:15:06Z

Is this good to merge?

chardan · 2025-06-02T16:13:08Z

@irq0 This looks good to me at this point; if you'd like to add a Death Test, I should point out that Catch2 is now supported, so you can do that if you would like to! (See cmake/modules/AddCephTest.cmake and test/rgw/CMakeLists.txt for examples).

irq0 · 2025-06-02T16:16:24Z

@irq0 This looks good to me at this point; if you'd like to add a Death Test, I should point out that Catch2 is now supported, so you can do that if you would like to!

The last push added the death test using gtest :)

chardan · 2025-06-02T16:18:05Z

@irq0 Merged! Thank you for all of your work on this, and also for your considerable patience!

irq0 · 2025-06-02T16:24:01Z

@irq0 Merged! Thank you for all of your work on this, and also for your considerable patience!

Thanks! Any chance to get this into tentacle?

chardan · 2025-06-02T17:14:07Z

I don't think so, unfortunately.

cbodley · 2025-06-04T16:58:20Z

ubuntu builds on https://shaman.ceph.com/builds/ceph/main/ have been failing for a couple days with:

In file included from /build/ceph-20.3.0-762-gc71d7eeb/src/common/ceph_context.h:38,
                 from /build/ceph-20.3.0-762-gc71d7eeb/src/common/dout.h:29,
                 from /build/ceph-20.3.0-762-gc71d7eeb/src/java/native/libcephfs_jni.cc:33:
/build/ceph-20.3.0-762-gc71d7eeb/src/breakpad/src/client/linux/handler/exception_handler.h:39:10: fatal error: client/linux/crash_generation/crash_generation_client.h: No such file or directory
   39 | #include "client/linux/crash_generation/crash_generation_client.h"
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

example logs:
https://jenkins.ceph.com/job/ceph-dev-build/ARCH=x86_64,AVAILABLE_ARCH=x86_64,AVAILABLE_DIST=jammy,DIST=jammy,MACHINE_SIZE=gigantic/57721//consoleFull
https://jenkins.ceph.com/job/ceph-dev-build/ARCH=x86_64,AVAILABLE_ARCH=x86_64,AVAILABLE_DIST=jammy,DIST=jammy,MACHINE_SIZE=gigantic/57737//consoleFull

cbodley · 2025-06-04T17:06:15Z

Thanks for your patience! Shaman output here: https://shaman.ceph.com/builds/ceph/wip-irq0-breakpad/f7070a71447ebc6f51be180e811756db4e341dc7/default/427571/

@chardan note that this build was from 97 days ago, and https://shaman.ceph.com/builds/ceph/wip-irq0-breakpad/f7070a71447ebc6f51be180e811756db4e341dc7/ shows the same ubuntu failure

chardan · 2025-06-04T17:22:50Z

@cbodley This is my fault, I didn't notice that and shouldn't have merged-- apologies for the confusion here, I'll work on being more thorough when the merge button is involved!

ivancich · 2025-06-04T18:36:24Z

@chardan and @cbodley : Given that there were modifications to global_init.cc, shouldn't this have gone through teuthology prior to merge also? (I don't even know what suites are used for common code.)

cbodley · 2025-06-04T18:44:42Z

@chardan and @cbodley : Given that there were modifications to global_init.cc, shouldn't this have gone through teuthology prior to merge also?

agreed, yes

I don't even know what suites are used for common code.

the rados suite is a good default for common changes - you can add the 'core' and 'needs-qa' labels to get picked up for testing/merging

athanatos · 2025-06-04T20:47:02Z

This PR re-added an extra line to .gitmodules.

	shallow = true

This breaks the nvmeof submodule on fresh checkouts generally.

ceph#61264 reintroduced https://tracker.ceph.com/issues/67640 fixed by 383091e. Setting shallow=true for the nvmeof/gateway submodule is problematic because the ceph.git submodule sha1 is only very rarely the head sha1 of the default branch. Fixes: https://tracker.ceph.com/issues/71568 Signed-off-by: Samuel Just <sjust@redhat.com>

ivancich · 2025-06-05T15:34:11Z

@chardan Did you see comment about re: need for teuthology run in this case?

chardan · 2025-06-05T18:34:34Z

@ivancich I've submitted it to shaman for build, then will run through teuthology. Might take a bit, things are backed up...

ceph#61264 reintroduced https://tracker.ceph.com/issues/67640 fixed by 383091e. Setting shallow=true for the nvmeof/gateway submodule is problematic because the ceph.git submodule sha1 is only very rarely the head sha1 of the default branch. Fixes: https://tracker.ceph.com/issues/71568 Signed-off-by: Samuel Just <sjust@redhat.com>

irq0 requested a review from a team as a code owner January 8, 2025 13:43

github-actions bot added build/ops common rgw labels Jan 8, 2025

irq0 commented Jan 8, 2025

View reviewed changes

src/CMakeLists.txt Show resolved Hide resolved

irq0 force-pushed the wip/breakpad branch from 9a2fd9f to 1fd1e99 Compare January 8, 2025 15:06

irq0 force-pushed the wip/breakpad branch from 1fd1e99 to 0691bbb Compare January 8, 2025 16:30

irq0 mentioned this pull request Jan 10, 2025

RGW SSE-KMS secrets cache #61256

Open

14 tasks

irq0 force-pushed the wip/breakpad branch from 0691bbb to 17f91e1 Compare January 13, 2025 15:02

irq0 requested a review from a team as a code owner January 13, 2025 15:02

github-actions bot added the crimson label Jan 13, 2025

irq0 force-pushed the wip/breakpad branch from 17f91e1 to e955210 Compare January 16, 2025 09:28

adamemerson requested review from chardan and mattbenjamin January 17, 2025 19:05

chardan approved these changes Jan 17, 2025

View reviewed changes

Marcel Lauhoff and others added 6 commits May 9, 2025 17:06

cmake: build and link breakpad

c0e9791

Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com>

global: set up breakpad crash reporting

5729a4b

Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com>

cmake: Make breakpad build conditional (WITH_BREAKPAD)

a451047

Add option WITH_BREAKPAD defaulting to TRUE on non-windows platforms. Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com>

global: Make breakpad code compile-time conditional on HAVE_BREAKPAD

cf7d43a

Only build breakpad code if we build with breakpad support. Print warning if breakpad enabled in config, but not built. Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com>

cmake: Disable incompatible warning flags for Breakpad

92e115f

Explicitly disable incompatible `-Warray-bounds` and `-Wmaybe-uninitialized`. These aren't normally enabled as compile-failing errors, except when building RPMs on CentOS9. Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com>

test: Breakpad Death Tests

d2467ba

Test that with enabled Breakpad crash handling Ceph creates correct minidumps. Signed-off-by: Marcel Lauhoff <marcel.lauhoff@clyso.com>

irq0 force-pushed the wip/breakpad branch from 1e685e2 to d2467ba Compare May 13, 2025 11:36

github-actions bot added tests and removed needs-rebase labels May 13, 2025

chardan merged commit 5611721 into ceph:main Jun 2, 2025
12 checks passed

chardan mentioned this pull request Jun 2, 2025

revert breakpad support #63654

Closed

14 tasks

athanatos mentioned this pull request Jun 4, 2025

.gitmodules: remove shallow=true config from nvmeof/gateway #63729

Merged

Conversation

irq0 commented Jan 8, 2025 • edited by chardan Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Why Breakpad?

Demo

References

Checklist

Uh oh!

Uh oh!

adamemerson commented Jan 8, 2025

Uh oh!

adamemerson commented Jan 8, 2025

Uh oh!

adamemerson commented Jan 8, 2025

Uh oh!

irq0 commented Jan 13, 2025

Uh oh!

irq0 commented Jan 14, 2025

Uh oh!

irq0 commented Jan 16, 2025

Uh oh!

adamemerson commented Jan 17, 2025

Uh oh!

chardan left a comment

Choose a reason for hiding this comment

Uh oh!

chardan Jan 17, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

chardan commented Jan 17, 2025

Uh oh!

irq0 commented Jan 20, 2025

Uh oh!

chardan commented Jan 20, 2025

Uh oh!

chardan commented Jan 20, 2025

Uh oh!

chardan commented Jan 22, 2025

Uh oh!

irq0 commented Jan 23, 2025

Uh oh!

chardan commented Jan 28, 2025

Uh oh!

adamemerson commented Jan 28, 2025

Uh oh!

chardan commented May 13, 2025

Uh oh!

irq0 commented Jun 2, 2025

Uh oh!

chardan commented Jun 2, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

irq0 commented Jun 2, 2025

Uh oh!

Uh oh!

chardan commented Jun 2, 2025

Uh oh!

irq0 commented Jun 2, 2025

Uh oh!

chardan commented Jun 2, 2025

Uh oh!

cbodley commented Jun 4, 2025

Uh oh!

cbodley commented Jun 4, 2025

Uh oh!

chardan commented Jun 4, 2025

Uh oh!

ivancich commented Jun 4, 2025

Uh oh!

cbodley commented Jun 4, 2025

Uh oh!

athanatos commented Jun 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

ivancich commented Jun 5, 2025

Uh oh!

chardan commented Jun 5, 2025

Uh oh!

Reviewers

irq0 commented Jan 8, 2025 •

edited by chardan

Loading

chardan Jan 17, 2025 •

edited

Loading

chardan commented Jun 2, 2025 •

edited

Loading

athanatos commented Jun 4, 2025 •

edited

Loading