Skip to content
This repository has been archived by the owner on Jan 23, 2024. It is now read-only.

fix: use npm v8 in expressapp container to workaround slow npm install from github #1484

Merged
merged 1 commit into from
May 16, 2022
Merged

fix: use npm v8 in expressapp container to workaround slow npm install from github #1484

merged 1 commit into from
May 16, 2022

Conversation

trentm
Copy link
Member

@trentm trentm commented May 16, 2022

The switch to node v16 gets use npm v8, to workaround an issue with
slow 'npm install '. See:
npm/cli#4896

In our case the github repo dependency was the command given to docker
run this container:
bash -c "npm install elastic-apm-node#SOME-COMMIT-SHA && node app.js"

This also adds a package.json to more explicitly declare we are working
with a node project workspace. Also avoid generating a package-lock file
we won't use.

Fixes: #1483

@trentm
fix: use npm v8 in expressapp container to workaround slow npm instal…
7f990a6 
…l from github

The switch to node v16 gets use npm v8, to workaround an issue with
slow 'npm install <any github repo dependency>'. See:
    npm/cli#4896

In our case the github repo dependency was the command given to docker
run this container:
    bash -c "npm install elastic-apm-node#SOME-COMMIT-SHA && node app.js"

This also adds a package.json to more explicitly declare we are working
with a node project workspace. Also avoid generating a package-lock file
we won't use.

Fixes: #1483
@trentm trentm requested review from astorm and kuisathaverat May 16, 2022 19:14
@trentm trentm self-assigned this May 16, 2022
@apmmachine
Copy link
Collaborator

💔 Tests Failed

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-05-16T19:14:23.355+0000

  • Duration: 33 min 17 sec

Test stats 🧪

Test Results
Failed 1
Passed 715
Skipped 1
Total 717

Test errors 1

Expand to view the tests failures

Tests / Sanity checks / Fix End of Files – pre_commit.lint
    Expand to view the error details

     error

    Expand to view the stacktrace

     ![CDATA[- hook id: end-of-file-fixer
- exit code: 1
- files were modified by this hook

Fixing docker/nodejs/express/Dockerfile

]]

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@trentm
Copy link
Member Author

trentm commented May 16, 2022 •
edited
Loading

AFAICT that "apm-ci/pr-merge — This commit has test failures" is a warning (!) in "Sanity checks" for which the logs are:

Archive JUnit-formatted test results1s

[2022-05-16T19:20:41.944Z] Recording test results

[2022-05-16T19:20:43.357Z] [Checks API] No suitable checks publisher found.

I am confused by that, but I'm ignoring them. The pipeline otherwise looks green to me: https://apm-ci.elastic.co/blue/organizations/jenkins/apm-integration-tests/detail/PR-1484/1/pipeline/116

@trentm trentm merged commit 1bc4005 into main May 16, 2022
@trentm trentm deleted the trentm/fix-nodejs-it-test-fail branch May 16, 2022 20:15
@v1v
Copy link
Member

v1v commented May 17, 2022

There is a missing end of file in docker/nodejs/express/Dockerfile

image

Those checks can be validated locally, with the pre-commit tool, though in this case is more a cosmetic check

@trentm
Copy link
Member Author

trentm commented May 17, 2022

@v1v Thanks. Some Qs: When I run pre-commit run --all-files manually, I get a LOT of warnings and changes:

% pre-commit run --all-files

[WARNING] The 'rev' field of repo 'https://github.com/detailyang/pre-commit-shell.git' appears to be a mutable reference (moving tag / branch).  Mutable references are never updated after first install and are not supported.  See https://pre-commit.com/#using-the-latest-version-for-a-repository for more details.  Hint: `pre-commit autoupdate` often fixes this.
[WARNING] The 'rev' field of repo 'https://github.com/elastic/apm-pipeline-library' appears to be a mutable reference (moving tag / branch).  Mutable references are never updated after first install and are not supported.  See https://pre-commit.com/#using-the-latest-version-for-a-repository for more details.  Hint: `pre-commit autoupdate` often fixes this.
Check for case conflicts.................................................Passed
Check that executables have shebangs.....................................Failed
- hook id: check-executables-have-shebangs
- exit code: 1

scripts/kibana/validate-ts-interfaces-against-apm-server-sample-docs/scripts/setup.sh: marked executable but has no (or invalid) shebang!
  If it isn't supposed to be executable, try: chmod -x scripts/kibana/validate-ts-interfaces-against-apm-server-sample-docs/scripts/setup.sh
  If it is supposed to be executable, double-check its shebang.

Check JSON...............................................................Passed
Check for merge conflicts................................................Passed
Check Xml................................................................Passed
Fix End of Files.........................................................Passed
Shellscript: lint........................................................Failed
- hook id: shell-lint
- exit code: 1
Yaml: lint...............................................................Failed
- hook id: yamllint
- exit code: 1

.ci/jobs/apm-integration-tests-selector-mbp.yml
  9:5       warning  wrong indentation: expected 6 but found 4  (indentation)
  19:11     warning  wrong indentation: expected 12 but found 10  (indentation)

docker/kibana_src/kibana_src.yml
  1:1       warning  missing document start "---"  (document-start)

.ci/jobs/apm-it-ec.yml
  11:5      warning  wrong indentation: expected 6 but found 4  (indentation)
  21:11     warning  wrong indentation: expected 12 but found 10  (indentation)

docker/kibana/kibana.yml
  1:2       warning  missing starting space in comment  (comments)
  2:1       warning  missing document start "---"  (document-start)
  3:2       warning  missing starting space in comment  (comments)
  12:7      warning  wrong indentation: expected 4 but found 6  (indentation)

tests/versions/nodejs.yml
  1:1       warning  missing document start "---"  (document-start)

docker/dyno/tests/files/docker_inspect.yml
  1:1       warning  missing document start "---"  (document-start)
  3:1       warning  wrong indentation: expected 2 but found 0  (indentation)
  10:3      warning  wrong indentation: expected 4 but found 2  (indentation)
  14:3      warning  wrong indentation: expected 4 but found 2  (indentation)
  16:3      warning  wrong indentation: expected 4 but found 2  (indentation)
  52:121    warning  line too long (778 > 120 characters)  (line-length)
  72:3      warning  wrong indentation: expected 4 but found 2  (indentation)
  106:3     warning  wrong indentation: expected 4 but found 2  (indentation)
  129:5     warning  wrong indentation: expected 6 but found 4  (indentation)
  134:3     warning  wrong indentation: expected 4 but found 2  (indentation)
  155:121   warning  line too long (174 > 120 characters)  (line-length)
  175:7     warning  wrong indentation: expected 8 but found 6  (indentation)
  191:5     warning  wrong indentation: expected 6 but found 4  (indentation)

.ci/jobs/apm-integration-test-downstream.yml
  10:5      warning  wrong indentation: expected 6 but found 4  (indentation)
  19:11     warning  wrong indentation: expected 12 but found 10  (indentation)

.ci/jobs/apm-it-eck.yml
  11:5      warning  wrong indentation: expected 6 but found 4  (indentation)
  21:11     warning  wrong indentation: expected 12 but found 10  (indentation)

.ci/jobs/apm-integration-tests.yml
  9:5       warning  wrong indentation: expected 6 but found 4  (indentation)
  23:9      warning  wrong indentation: expected 10 but found 8  (indentation)

tests/versions/go.yml
  1:1       warning  missing document start "---"  (document-start)

tests/versions/ruby.yml
  1:1       warning  missing document start "---"  (document-start)

.mergify.yml
  1:1       warning  missing document start "---"  (document-start)
  16:9      warning  wrong indentation: expected 6 but found 8  (indentation)
  19:121    warning  line too long (157 > 120 characters)  (line-length)
  68:9      warning  wrong indentation: expected 10 but found 8  (indentation)
  71:9      warning  wrong indentation: expected 10 but found 8  (indentation)
  85:121    warning  line too long (122 > 120 characters)  (line-length)

docker/ruby/rails/testapp/config/database.yml
  7:1       warning  missing document start "---"  (document-start)

scripts/tests/config/test_apm_managed_build.yml
  1:1       warning  missing document start "---"  (document-start)

docker/elasticsearch/roles.yml
  12:121    warning  line too long (126 > 120 characters)  (line-length)

tests/versions/java.yml
  1:1       warning  missing document start "---"  (document-start)

docker/ruby/rails/testapp/config/secrets.yml
  3:1       warning  missing document start "---"  (document-start)

.ci/jobs/defaults.yml
  22:5      warning  wrong indentation: expected 6 but found 4  (indentation)

scripts/tests/config/test_start_main_default.yml
  172:121   warning  line too long (184 > 120 characters)  (line-length)

scripts/tests/config/test_apm_managed.yml
  1:1       warning  missing document start "---"  (document-start)
  21:5      warning  wrong indentation: expected 6 but found 4  (indentation)
  24:3      warning  wrong indentation: expected 4 but found 2  (indentation)
  31:3      warning  wrong indentation: expected 4 but found 2  (indentation)
  34:3      warning  wrong indentation: expected 4 but found 2  (indentation)

.ci/jobs/apm-integration-tests-upgrade-mbp.yml
  9:5       warning  wrong indentation: expected 6 but found 4  (indentation)
  19:11     warning  wrong indentation: expected 12 but found 10  (indentation)

tests/versions/rum.yml
  1:1       warning  missing document start "---"  (document-start)

tests/versions/php.yml
  1:1       warning  missing document start "---"  (document-start)

tests/server/.exclude_versions.yml
  1:1       warning  missing document start "---"  (document-start)

docker/dyno/app/range.yml
  1:1       error    too many blank lines (1 > 0)  (empty-lines)
  83:6      warning  too few spaces before comment  (comments)
  84:6      warning  comment not indented like content  (comments-indentation)
  89:6      warning  too few spaces before comment  (comments)
  92:5      warning  too few spaces before comment  (comments)
  94:11     warning  too few spaces before comment  (comments)
  95:5      warning  too few spaces before comment  (comments)
  105:11    warning  too few spaces before comment  (comments)
  110:10    warning  too few spaces before comment  (comments)

docker/kibana/kibana-8.yml
  1:1       warning  missing document start "---"  (document-start)

tests/versions/dotnet.yml
  1:1       warning  missing document start "---"  (document-start)

docker/packetbeat/packetbeat.yml
  6:1       warning  wrong indentation: expected 2 but found 0  (indentation)

docker/packetbeat/packetbeat.6.x-compat.yml
  1:1       warning  missing document start "---"  (document-start)
  5:1       warning  wrong indentation: expected 2 but found 0  (indentation)
  32:8      warning  missing starting space in comment  (comments)

docker/filebeat/filebeat.yml
  42:15     warning  wrong indentation: expected 16 but found 14  (indentation)
  71:15     warning  wrong indentation: expected 16 but found 14  (indentation)
  141:15    warning  wrong indentation: expected 16 but found 14  (indentation)
  143:20    warning  wrong indentation: expected 20 but found 19  (indentation)

tests/versions/apm_server.yml
  1:1       warning  missing document start "---"  (document-start)

tests/versions/python.yml
  1:1       warning  missing document start "---"  (document-start)

check-bash-syntax........................................................Passed
check-abstract-classes-and-trait.....................(no files to check)Skipped
check-jsonslurper-class..................................................Passed
check-jenkins-pipelines..................................................Passed
- hook id: check-jenkins-pipelines
- duration: 2.79s
check-unicode-non-breaking-spaces........................................Passed
remove-unicode-non-breaking-spaces.......................................Passed
check-en-dashes..........................................................Passed
remove-en-dashes.........................................................Passed
check-jjbb...............................................................Passed
- hook id: check-jjbb
- duration: 13.32s

Transform JJBB to JJB
2022-05-17 16:44:31,030 [jjbb.main] [INFO] No templates found in .ci/templates, continuing without them.
2022-05-17 16:44:31,073 [jjbb.main] [INFO] No views found at .ci/views/views.yml, continuing without them.
Validate JJB
Transform JJBB to JJB
2022-05-17 16:44:31,048 [jjbb.main] [INFO] No templates found in .ci/templates, continuing without them.
2022-05-17 16:44:31,085 [jjbb.main] [INFO] No views found at .ci/views/views.yml, continuing without them.
Validate JJB


% git status
On branch main
Your branch is up to date with 'origin/main'.

Changes not staged for commit:
  (use "git add <file>..." to update what will be committed)
  (use "git restore <file>..." to discard changes in working directory)
	modified:   docker/dyno/package.json
	modified:   docker/dyno/tests/__init__.py
	modified:   docker/dyno/tests/unit/__init__.py
	modified:   docker/dyno/tests/unit/index.test.js
	modified:   docker/nodejs/express/Dockerfile
	modified:   docker/opbeans/sql/opbeans.sql
	modified:   scripts/kibana/validate-ts-interfaces-against-apm-server-sample-docs/README.md
	modified:   tests/server/6.x/error/python.json
	modified:   tests/server/6.x/error/rum.json
	modified:   tests/server/6.x/span/python.json
	modified:   tests/server/6.x/span/rum_1.json
	modified:   tests/server/6.x/span/rum_2.json
	modified:   tests/server/6.x/span/rum_3.json
	modified:   tests/server/6.x/span/rum_4.json
	modified:   tests/server/6.x/span/rum_5.json
	modified:   tests/server/6.x/span/rum_6.json
	modified:   tests/server/6.x/span/rum_7.json
	modified:   tests/server/6.x/transaction/python.json
	modified:   tests/server/6.x/transaction/rum.json
	modified:   tests/server/7.0.0/error/python.json
	modified:   tests/server/7.0.0/error/rum.json
	modified:   tests/server/7.0.0/span/nodejs.json
	modified:   tests/server/7.0.0/span/python.json
	modified:   tests/server/7.0.0/span/rum.json
	modified:   tests/server/7.0.0/span/rum_2.json
	modified:   tests/server/7.0.0/span/rum_3.json
	modified:   tests/server/7.0.0/span/rum_4.json
	modified:   tests/server/7.0.0/span/rum_5.json
	modified:   tests/server/7.0.0/span/rum_6.json
	modified:   tests/server/7.0.0/span/rum_7.json
	modified:   tests/server/7.0.0/transaction/python.json
	modified:   tests/server/7.0.0/transaction/rum.json

However, if I just pre-commit run, then I get no warnings or errors or changes:

% pre-commit run
[WARNING] The 'rev' field of repo 'https://github.com/detailyang/pre-commit-shell.git' appears to be a mutable reference (moving tag / branch).  Mutable references are never updated after first install and are not supported.  See https://pre-commit.com/#using-the-latest-version-for-a-repository for more details.  Hint: `pre-commit autoupdate` often fixes this.
[WARNING] The 'rev' field of repo 'https://github.com/elastic/apm-pipeline-library' appears to be a mutable reference (moving tag / branch).  Mutable references are never updated after first install and are not supported.  See https://pre-commit.com/#using-the-latest-version-for-a-repository for more details.  Hint: `pre-commit autoupdate` often fixes this.
Check for case conflicts.............................(no files to check)Skipped
Check that executables have shebangs.................(no files to check)Skipped
Check JSON...........................................(no files to check)Skipped
Check for merge conflicts............................(no files to check)Skipped
Check Xml............................................(no files to check)Skipped
Fix End of Files.....................................(no files to check)Skipped
Shellscript: lint....................................(no files to check)Skipped
Yaml: lint...........................................(no files to check)Skipped
check-bash-syntax....................................(no files to check)Skipped
check-abstract-classes-and-trait.....................(no files to check)Skipped
check-jsonslurper-class..............................(no files to check)Skipped
check-jenkins-pipelines..............................(no files to check)Skipped
- hook id: check-jenkins-pipelines
check-unicode-non-breaking-spaces....................(no files to check)Skipped
remove-unicode-non-breaking-spaces...................(no files to check)Skipped
check-en-dashes......................................(no files to check)Skipped
remove-en-dashes.....................................(no files to check)Skipped
check-jjbb...........................................(no files to check)Skipped
- hook id: check-jjbb

What am I supposed to run in a working copy to know what is going to fail in CI?

@trentm trentm mentioned this pull request May 17, 2022
Merged
@v1v
Copy link
Member

v1v commented May 17, 2022

What am I supposed to run in a working copy to know what is going to fail in CI?

pre-commit was added recently in compared to when this project started. Sometimes I run pre-commit <file> or similar command before doing any changes to apply the fixes in the linting, then I apply my changes.

Though, it's not always the case...

the existing integration in the CI does validate the change-set, so it will not run pre-commit run --all-files but the pre-commit for each file in the PR

@v1v
Copy link
Member

v1v commented May 17, 2022

I also forgot to mention, once the pre-commit has been configured and installed in the workspace, you do nothing but git commit, if there are linting errors in any of the files to be committed then it will be reported.

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@astorm astorm astorm approved these changes

@kuisathaverat kuisathaverat Awaiting requested review from kuisathaverat

Assignees

@trentm trentm

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

apm-agent-nodejs integration tests are failing because of slow npm install from github
4 participants
@trentm @apmmachine @v1v @astorm