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

Bumping packages to fix build and security issues #275

Draft
wants to merge 18 commits into
base: main
Choose a base branch
from

Conversation

jakub-racek-swi
Copy link
Contributor

@jakub-racek-swi jakub-racek-swi commented Sep 12, 2024

Description: Bumping packages to fix build and security issues - this is accompanied by a PR to the build env, which must be merged first.

Bumping packages to fix build and security issues

Link to tracking Issue:

Testing:

Documentation:

jakub-racek-swi and others added 3 commits September 11, 2024 14:15
Signed-off-by: jakub-racek-swi <jakub.racek@solarwinds.com>
@jakub-racek-swi jakub-racek-swi marked this pull request as ready for review September 12, 2024 09:04
jakub-racek-swi and others added 8 commits September 12, 2024 11:52
Signed-off-by: jakub-racek-swi <jakub.racek@solarwinds.com>
Signed-off-by: jakub-racek-swi <jakub.racek@solarwinds.com>
Signed-off-by: jakub-racek-swi <jakub.racek@solarwinds.com>
Signed-off-by: jakub-racek-swi <jakub.racek@solarwinds.com>
Signed-off-by: jakub-racek-swi <jakub.racek@solarwinds.com>
@@ -11,7 +11,7 @@ on:
paths:

env:
BENV_IMAGE: quay.io/splunko11ytest/network-explorer-debug/build-env
BENV_IMAGE: public.ecr.aws/u7d6c4a3/solarwinds-opentelemetry-network:build-env

Choose a reason for hiding this comment

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

I believe you should move PR to draft if you are still experimenting with the changes. We shouldn't merge to upstream use of image from solarwinds owned registry

@jakub-racek-swi jakub-racek-swi marked this pull request as draft September 23, 2024 07:01
jakub-racek-swi and others added 7 commits September 23, 2024 09:11
Signed-off-by: jakub-racek-swi <jakub.racek@solarwinds.com>
Signed-off-by: jakub-racek-swi <jakub.racek@solarwinds.com>
Signed-off-by: jakub-racek-swi <jakub.racek@solarwinds.com>
@jakub-racek-swi
Copy link
Contributor Author

Hey @yonch, I was taking another look at the failing unit tests in my otel-network build.

I've made some progress with investigating the abseil failures, which is most likely related to some old issue that happens when asan is not applied to the entire build (abseil/abseil-cpp#1524).

While investigating the 'render_test', I was trying to start the unit tests locally, but got wildly different test results.

The following tests FAILED:
          1 - render_test (Failed)
          9 - json_test (SEGFAULT)
         10 - fixed_hash_test (SEGFAULT)
         13 - log_modifiers_test (SEGFAULT)
         19 - bits_test (SEGFAULT)
         20 - counter_test (SEGFAULT)
         21 - counter_to_rate_test (SEGFAULT)
         25 - cgroup_handler_test (SEGFAULT)
         26 - kernel_symbols_test (Failed)

I am wondering how come the unit tests produce different results here in the github actions and when built locally on my machine. This is only with asan, however, as all tests do pass in the regular build.

I also wanted to ask how to handle debugging, since there is a --debug option in the build script. How do I set it up? Couldn't find any info in the docs sadly (though I could just be bad at navigating them :))

@yonch
Copy link
Contributor

yonch commented Sep 23, 2024

Hi @jakub-racek-swi !

I'm not sure regarding the ASAN failures. What CPU are you running on?

Let me check the debug builds. I remember we used to have a script to run under gdb..

@yonch
Copy link
Contributor

yonch commented Sep 23, 2024

So for debugging the kernel collector:

  • Here is the docker build clause for kernel-collector debug builds.
  • Setting the environment variable EBPF_NET_RUN_UNDER_GDB=gdb might get you what you want (from entrypoint.sh ).
  • If you're using the devbox scripts, the kernel collector's supports a --gdb flag

Hope this is what you're looking for..

@yonch
Copy link
Contributor

yonch commented Oct 9, 2024

Verified that this PR compiles with the new build image. 👍

My test run with ./build.sh --asan --debug test yields:

The following tests FAILED:
	  1 - render_test (Failed)
	 10 - fixed_hash_test (Failed)
	 26 - kernel_symbols_test (Failed)

I didn't get the SEGFAULTs you got -- maybe you changed something since that earlier post?

Without ASAN tests seem to pass.

@yonch
Copy link
Contributor

yonch commented Nov 26, 2024

@jakub-racek-swi the new build image is docker.io/otel/opentelemetry-network-build-tools. It is built via GitHub Actions on the build tools repo.

Are you comfortable changing .github/workflows/build-and-test.yaml to use that image, and mark the PR ready for review?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants