Skip to content
New issue

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

#

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

Already on GitHub? # to your account

Jump to bottom

fix(ci): fixed deploy-pages job needs. #1929

Merged
merged 4 commits into from
Jun 21, 2024
Merged

fix(ci): fixed deploy-pages job needs. #1929

merged 4 commits into from
Jun 21, 2024

Conversation

FedeDP
Copy link
Contributor

@FedeDP FedeDP commented Jun 21, 2024

What type of PR is this?

/kind bug

Any specific area of the project related to this PR?

/area CI

Does this PR require a change in the driver versions?

What this PR does / why we need it:

Small fix.
Also, i updated running linux kernel on our cncf x64 node to 5.19.0 to incorporate linux perf patch: torvalds/linux@be8ecc5
See: https://eighty-twenty.org/2021/09/09/perf-addr2line-speed-improvement; basically without the patch, perf script is basically unusable on large files.

Which issue(s) this PR fixes:

Fixes #

Special notes for your reviewer:

Does this PR introduce a user-facing change?:

NONE

FedeDP added 2 commits June 21, 2024 10:35
@FedeDP
fix(ci): fixed deploy-pages job needs.
2417c1c 
Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
@FedeDP
chore(ci): install linux perf too in composite perf action.
958c2e0 
Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
@FedeDP
Copy link
Contributor Author

FedeDP commented Jun 21, 2024

/milestone 0.18.0

@poiana poiana added this to the 0.18.0 milestone Jun 21, 2024
@poiana poiana added the size/XS label Jun 21, 2024
@poiana poiana requested review from hbrueckner and LucaGuerra June 21, 2024 08:46
@FedeDP
fix(ci): fixed path to generate_inline_svg_md.py script.
88b21fa 
Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
@github-actions GitHub Actions
Copy link

Perf diff from master - unit tests

LucaGuerra
LucaGuerra previously approved these changes Jun 21, 2024
View reviewed changes
@poiana poiana added the lgtm label Jun 21, 2024
Andreagit97
Andreagit97 previously approved these changes Jun 21, 2024
View reviewed changes
Copy link
Member

@Andreagit97 Andreagit97 left a comment

Choose a reason for hiding this comment

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

/approve

@FedeDP
chore(ci): store PR info before checking threshold to avoid empty com…
76d8897 
…ments.

Signed-off-by: Federico Di Pierro <nierro92@gmail.com>
@FedeDP FedeDP dismissed stale reviews from Andreagit97 and LucaGuerra via 76d8897 June 21, 2024 09:17
@poiana poiana added size/M and removed size/XS labels Jun 21, 2024
@github-actions GitHub Actions
Copy link

Perf diff from master - unit tests

FedeDP
FedeDP commented Jun 21, 2024
View reviewed changes
.github/actions/composite-perf/action.yml
@@ -15,7 +15,7 @@ runs:
- name: Install deps ⛓️
shell: bash
run: |
sudo apt update && sudo apt install -y --no-install-recommends ca-certificates cmake build-essential git clang llvm pkg-config autoconf automake libtool libelf-dev wget libc-ares-dev libcurl4-openssl-dev libssl-dev libtbb-dev libjq-dev libjsoncpp-dev libgrpc++-dev protobuf-compiler-grpc libgtest-dev libprotobuf-dev
sudo apt update && sudo apt install -y --no-install-recommends ca-certificates cmake build-essential git clang llvm pkg-config autoconf automake libtool libelf-dev wget libc-ares-dev libcurl4-openssl-dev libssl-dev libtbb-dev libjq-dev libjsoncpp-dev libgrpc++-dev protobuf-compiler-grpc libgtest-dev libprotobuf-dev linux-tools-common linux-tools-generic linux-tools-`uname -r`
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Even if perf is already installed on our cncf node, add it to the deps.

FedeDP
FedeDP commented Jun 21, 2024
View reviewed changes
.github/workflows/pages.yml
@@ -104,10 +104,10 @@ jobs:

- name: Generate perf pages
run: |
python3 ./github/generate_inline_svg_md.py trace_tests.svg
python3 .github/generate_inline_svg_md.py trace_tests.svg
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed path to python script.

FedeDP
FedeDP commented Jun 21, 2024
View reviewed changes
.github/workflows/perf.yml
Copy link
Contributor Author

Choose a reason for hiding this comment

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

To avoid if: always() that would trigger even when something fails, leading to weird empty PR comments, save PR info immediately, and then finally check thresholds to make the job fail.

@github-actions GitHub Actions
Copy link

Perf diff from master - unit tests

     9.14%     -1.54%  [.] sinsp_thread_manager::find_thread
     7.07%     -1.23%  [.] next
     3.29%     +1.03%  [.] sinsp_evt::load_params
     4.04%     +0.99%  [.] sinsp_thread_manager::get_thread_ref
     0.54%     +0.97%  [.] scap_event_encode_params_v
     2.36%     -0.82%  [.] libsinsp::sinsp_suppress::process_event
     0.67%     +0.74%  [.] scap_event_decode_params
     0.95%     +0.73%  [.] sinsp_fdtable::find_ref
    12.88%     -0.67%  [.] sinsp_parser::reset
     1.21%     -0.60%  [.] std::__shared_count<(__gnu_cxx::_Lock_policy)2>::__shared_count

Perf diff from master - scap file

    17.22%     -7.94%  [.] libsinsp::runc::match_container_id
    12.76%     +5.20%  [.] sinsp_evt_formatter::tostring_withformat
    13.26%     -2.89%  [.] sinsp_filter_check_event::extract_single
     8.76%     -2.73%  [.] sinsp_filter_check::rawval_to_string
    12.86%     -2.66%  [.] sinsp_filter_check_thread::extract_single
     4.43%     +2.51%  [.] sinsp_parser::process_event
     4.44%     +2.51%  [.] sinsp_filter_check::tostring
    17.44%     +2.36%  [.] sinsp_filter_check::extract
     4.45%     -0.14%  [.] sinsp_threadinfo::~sinsp_threadinfo
     4.38%     -0.06%  [.] sinsp_thread_manager::get_thread_ref

Andreagit97
Andreagit97 approved these changes Jun 21, 2024
View reviewed changes
Copy link
Member

@Andreagit97 Andreagit97 left a comment

Choose a reason for hiding this comment

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

/approve

@poiana
Copy link
Contributor

poiana commented Jun 21, 2024

[APPROVALNOTIFIER] This PR is APPROVED

This pull-request has been approved by: Andreagit97, FedeDP, LucaGuerra

The full list of commands accepted by this bot can be found here.

The pull request process is described here

Needs approval from an approver in each of these files:
  • OWNERS [Andreagit97,FedeDP,LucaGuerra]

Approvers can indicate their approval by writing /approve in a comment
Approvers can cancel approval by writing /approve cancel in a comment

@poiana poiana merged commit 5902d14 into master Jun 21, 2024
40 of 41 checks passed
@poiana poiana deleted the fix/deploy-pages-needs branch June 21, 2024 09:45
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@LucaGuerra LucaGuerra LucaGuerra approved these changes

@Andreagit97 Andreagit97 Andreagit97 approved these changes

@hbrueckner hbrueckner Awaiting requested review from hbrueckner

Assignees

@LucaGuerra LucaGuerra

@Andreagit97 Andreagit97

Labels
approved area/CI dco-signoff: yes kind/bug Something isn't working lgtm release-note-none size/M
Projects
None yet
Milestone
0.18.0
Development

Successfully merging this pull request may close these issues.
4 participants
@FedeDP @poiana @LucaGuerra @Andreagit97