Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

NH-89406: improve control for building debug version of the extension and added documentation #155

Merged
merged 15 commits into from
Sep 17, 2024

Conversation

xuan-cao-swi
Copy link
Contributor

@xuan-cao-swi xuan-cao-swi commented Sep 11, 2024

Description

  1. remove debug flag to reduce the size of apm
  2. add instruction for debugging with core dump file
  3. add extconf_test.rb to test the workflow of extconf (makefile construction) if OBOE_DEBUG, OBOE_DEV, OBOE_STAGING appears. The test case require deep stub on each different function to avoid c lib dependency.
  4. OBOE_DEBUG will make the fetch of production release with relwithdebug directly.

Test (if applicable)

@xuan-cao-swi xuan-cao-swi marked this pull request as ready for review September 11, 2024 22:33
@xuan-cao-swi xuan-cao-swi requested a review from a team as a code owner September 11, 2024 22:33
Copy link
Contributor

@cheempz cheempz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks @xuan-cao-swi! This is really great info, much appreciated. I left a bunch of comments based on understanding below (which may be wrong):

  • in order to get extended info out of coredump, the crash needs to be reproduced using special version of solarwinds_apm
  • this special version can be achieved purely at install time by setting env var OBOE_DEBUG which enables more info in the c-extension binding code AND uses a version of liboboe that comes with extended debug info.

Rakefile Outdated Show resolved Hide resolved
puts 'Fetching finished.'
end

def fetch_file_from_cloud(files, oboe_dir, dest_dir, folder = '')
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nice DRYing :)

ext/oboe_metal/extconf.rb Show resolved Hide resolved
ext/oboe_metal/README.md Outdated Show resolved Hide resolved
ext/oboe_metal/README.md Show resolved Hide resolved
ext/oboe_metal/README.md Outdated Show resolved Hide resolved
ext/oboe_metal/README.md Outdated Show resolved Hide resolved
ext/oboe_metal/README.md Outdated Show resolved Hide resolved
solarwinds_apm.gemspec Show resolved Hide resolved
ext/oboe_metal/extconf.rb Show resolved Hide resolved
ext/oboe_metal/extconf.rb Outdated Show resolved Hide resolved
@xuan-cao-swi
Copy link
Contributor Author

in order to get extended info out of coredump, the crash needs to be reproduced using special version of solarwinds_apm

For now yes, here is the difference log info

# without .debug symbol file
#5  0x0000ffff92c1fd04 in oboe_ssl_reporter::eventSender() () from /root/.rbenv/versions/3.1.0/lib/ruby/gems/3.1.0/gems/solarwinds_apm-6.0.6/lib/../ext/oboe_metal/lib/liboboe.so

# with .debug symbol file
#5  0x0000ffff9841c6ac in oboe_ssl_reporter::eventSender (this=0xffff9927c010) at /__w/solarwinds-apm-liboboe/solarwinds-apm-liboboe/liboboe/reporter/ssl.cpp:1677

# with ReleaseAndDebug and .debug file
#5  0x0000ffff9841c6ac in oboe_ssl_reporter::eventSender (this=0xffff9927c010) at /__w/solarwinds-apm-liboboe/solarwinds-apm-liboboe/liboboe/reporter/ssl.cpp:1677
    ptr2 = 0x0
        grpcRequest = {< ...

this special version can be achieved purely at install time by setting env var OBOE_DEBUG which enables more info in the c-extension binding code AND uses a version of liboboe that comes with extended debug info.

Yes, and also need the liboboe build with ReleaseAndDebug

ext/oboe_metal/README.md Outdated Show resolved Hide resolved
@cheempz cheempz changed the title NH-89406: remove debug flag for release version of apm NH-89406: improve control for building debug version of the extension and added documentation Sep 16, 2024
Copy link
Contributor

@cheempz cheempz left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM! really appreciate the revisits and added tests @xuan-cao-swi. We'll need to bump the liboboe version to 15.0.2 for debug install to work, but feel free to put that in as part of the main release prep.

@xuan-cao-swi xuan-cao-swi merged commit 39ecef4 into main Sep 17, 2024
14 checks passed
@xuan-cao-swi xuan-cao-swi deleted the NH-89406 branch September 17, 2024 20:04
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

2 participants