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

chore: use nx commands #2493

Merged
merged 26 commits into from
Nov 5, 2024
Merged
Show file tree
Hide file tree
Changes from 16 commits
Commits
Show all changes
26 commits
Select commit Hold shift + click to select a range
22e58cd
chore: use nx commands
david-luna Oct 18, 2024
d71f1cc
chore: rever unit tests workflow
david-luna Oct 21, 2024
8fc8034
chore: fix test:changed script
david-luna Oct 21, 2024
8c9ec9a
chore: add compile cache to unit tests
david-luna Oct 21, 2024
858fa77
chore: fix compile task in unit tests
david-luna Oct 21, 2024
e5c6bb7
chore: setup cache in test jobs
david-luna Oct 21, 2024
97e7695
chore: rever cache in unit-test workflow
david-luna Oct 21, 2024
615401d
Merge branch 'main' into dluna/update-scripts
david-luna Oct 21, 2024
83781d0
chore: use actions to upload/download artifacts
david-luna Oct 21, 2024
95a4557
Merge branch 'dluna/update-scripts' of github.com:david-luna/opentele…
david-luna Oct 21, 2024
16122d5
chore: change cache name
david-luna Oct 21, 2024
f062d99
chore: fix cache name
david-luna Oct 21, 2024
6acd051
chore: add cache to TAV workflow
david-luna Oct 21, 2024
b181935
chore: use different cache names between unti tests and TAV
david-luna Oct 21, 2024
6b5df30
chore: add dependency between jobs in TAV workflow
david-luna Oct 21, 2024
6cec554
chore: remove old scripts
david-luna Oct 21, 2024
437637e
chore: restore script
david-luna Oct 22, 2024
6aa6084
Merge branch 'main' into dluna/update-scripts
david-luna Oct 22, 2024
c7e6520
Merge branch 'main' into dluna/update-scripts
david-luna Oct 29, 2024
cef0847
chore: add missing dev dependencies
david-luna Oct 31, 2024
7ab5103
Merge branch 'main' into dluna/update-scripts
david-luna Oct 31, 2024
59fb12c
Merge branch 'main' into dluna/update-scripts
david-luna Nov 4, 2024
3ad305c
chore: update node versions in test workflows
david-luna Nov 5, 2024
ff866c0
Merge branch 'dluna/update-scripts' of github.com:david-luna/opentele…
david-luna Nov 5, 2024
530fdbb
Merge branch 'main' into dluna/update-scripts
david-luna Nov 5, 2024
07e998a
Merge branch 'main' into dluna/update-scripts
david-luna Nov 5, 2024
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
32 changes: 32 additions & 0 deletions .github/workflows/test-all-versions.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,8 +13,35 @@ on:
type: string

jobs:
build:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note for reviewer: this is the build step that is going to be cached. github.run_number is unique for the PR so each time the workflow is run this tasks overwrites the artifact saving space. retention-days is set to 1 so the artifact will be removed the next day after the last run of this workflow.

david-luna marked this conversation as resolved.
Show resolved Hide resolved
strategy:
fail-fast: false
runs-on: ubuntu-latest
env:
NPM_CONFIG_UNSAFE_PERM: true
NODE_OPTIONS: --max-old-space-size=4096
steps:
- name: Checkout
uses: actions/checkout@v4
with:
fetch-depth: 0
- uses: actions/setup-node@v4
with:
node-version: 16
- name: Install
run: npm ci
- name: Build
run: npm run compile
- name: Upload Build Artifacts
uses: actions/upload-artifact@v4
with:
name: tav-build-cache-${{ github.run_number }}
path: node_modules/.cache/nx
retention-days: 1

tav:
name: Run test-all-versions
needs: build
strategy:
fail-fast: false
matrix:
Expand Down Expand Up @@ -120,6 +147,11 @@ jobs:
run: npm install -g npm@9 # npm@9 supports node >=14.17.0
- name: Install
run: npm ci
- name: Download Build Artifacts
uses: actions/download-artifact@v4
with:
name: tav-build-cache-${{ github.run_number }}
path: node_modules/.cache/nx
- name: Build
run: npm run compile
- name: Run test-all-versions
Expand Down
38 changes: 38 additions & 0 deletions .github/workflows/unit-test.yml
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,34 @@ on:
pull_request:

jobs:
build:
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note for reviewer: same here for unit tests. I guess a single compile step can be done for both workflows but I think this will add complexity to them. This way both workflows are independent and still we reduce the resource usage

strategy:
fail-fast: false
runs-on: ubuntu-latest
env:
NPM_CONFIG_UNSAFE_PERM: true
NODE_OPTIONS: --max-old-space-size=4096
steps:
- name: Checkout
uses: actions/checkout@v4
with:
fetch-depth: 0
- uses: actions/setup-node@v4
with:
node-version: 16
- name: Install
run: npm ci
- name: Build
run: npm run compile
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm a little scared of the "if npm ci and npm run compile layout is the same for all node versions". :)
Have you done a basic check comparing two separate git clones that do those steps, one with node 16 and one with node 22, say?

  • I think I'm reasonably confident that for non-binary packages that the node_modules/... tree should be the same between versions -- otherwise the idea of package-lock.json is broken.
  • I'm not sure if we have binary deps -- i.e. ones that are built at install time via a gyp file or install or post-install npm scripts.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm a little scared of the "if npm ci and npm run compile layout is the same for all node versions". :)

We can do even another twist on this because:

  • ts-node compiles src & tests folders so "in theory" is not necessary to compile, just to have the version.ts file available
  • but some packages depend on another in the same monorepo like @opentelemetry/resouce-detector-container and these dependencies need to be build.

Since the published packages are compiled using nodejs v16 maybe is better to have a "production" build as tests assets instead of a different build with each nodejs version (14, 18, 20, ...)?

Copy link
Member

Choose a reason for hiding this comment

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

I'm not sure if we have binary deps -- i.e. ones that are built at install time via a gyp file or install or post-install npm scripts.

We don't have them anymore AFAICT. The last was removed with grpc-cencus-binary-propagator (#2276) because it always built on macOS (it had pre-built binaries for linux+glibc so building sometimes does not happen based on which system one is using). I think if there was another one left we'd have noticed it by now as it tends to take a lot of time on install. 🙂

Since the published packages are compiled using nodejs v16 maybe is better to have a "production" build as tests assets instead of a different build with each nodejs version (14, 18, 20, ...)?

I think having the Node.js version matched with the published packages makes sense from a testing standpoint. This way we can preempt surprises if there are any. @david-luna would you mind putting a comment in the publish workflow and here that the versions should always be updated together? 🙂

- name: Upload build artifact
uses: actions/upload-artifact@v4
with:
name: tests-build-cache-${{ github.run_number }}
path: node_modules/.cache/nx
retention-days: 1

unit-test:
needs: build
strategy:
fail-fast: false
matrix:
Expand Down Expand Up @@ -128,6 +155,11 @@ jobs:
run: npm install -g npm@9 # npm@9 supports node >=14.17.0
- name: Install
run: npm ci
- name: Download Build Artifacts
uses: actions/download-artifact@v4
with:
name: tests-build-cache-${{ github.run_number }}
path: node_modules/.cache/nx
- name: Build
run: npm run compile
- name: Unit tests (Full)
Expand All @@ -145,6 +177,7 @@ jobs:
verbose: true

browser-test:
needs: build
strategy:
fail-fast: false
matrix:
Expand All @@ -166,6 +199,11 @@ jobs:
run: npm install -g npm@9 # npm@9 supports node >=14.17.0
- name: Install
run: npm ci
- name: Dowload Artifacts
david-luna marked this conversation as resolved.
Show resolved Hide resolved
uses: actions/download-artifact@v4
with:
name: tests-build-cache-${{ github.run_number }}
path: node_modules/.cache/nx
- name: Build
run: npm run compile
- name: Unit tests
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
"compile": "tsc -p .",
"lint": "eslint . --ext .ts",
"lint:fix": "eslint . --ext .ts --fix",
"precompile": "tsc --version && lerna run version:update --scope @opentelemetry/resource-detector-alibaba-cloud --include-dependencies",
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note for reviewer: not necessary since nx.json has defined the task dependencies. Same applies for other packages

"prewatch": "npm run precompile",
"prepublishOnly": "npm run compile",
"test": "nyc ts-mocha -p tsconfig.json 'test/**/*.test.ts'",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
"compile": "tsc -p .",
"lint": "eslint . --ext .ts",
"lint:fix": "eslint . --ext .ts --fix",
"precompile": "tsc --version && lerna run version:update --scope @opentelemetry/resource-detector-aws --include-dependencies",
"prewatch": "npm run precompile",
"prepublishOnly": "npm run compile",
"test": "nyc ts-mocha -p tsconfig.json 'test/**/*.test.ts'",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
"compile": "tsc -p .",
"lint": "eslint . --ext .ts",
"lint:fix": "eslint . --ext .ts --fix",
"precompile": "tsc --version && lerna run version:update --scope @opentelemetry/resource-detector-azure --include-dependencies",
"prewatch": "npm run precompile",
"prepublishOnly": "npm run compile",
"test": "nyc ts-mocha -p tsconfig.json 'test/**/*.test.ts'",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
"compile": "tsc -p .",
"lint": "eslint . --ext .ts",
"lint:fix": "eslint . --ext .ts --fix",
"precompile": "tsc --version && lerna run version:update --scope @opentelemetry/resource-detector-container --include-dependencies",
"prewatch": "npm run precompile",
"prepublishOnly": "npm run compile",
"test": "nyc ts-mocha -p tsconfig.json 'test/**/*.test.ts'",
Expand All @@ -34,6 +33,7 @@
"devDependencies": {
"@opentelemetry/api": "^1.0.0",
"@opentelemetry/contrib-test-utils": "^0.41.0",
"@opentelemetry/instrumentation-fs": "*",
david-luna marked this conversation as resolved.
Show resolved Hide resolved
"@types/mocha": "8.2.3",
"@types/node": "18.18.14",
"@types/sinon": "10.0.20",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
"compile": "tsc -p .",
"lint": "eslint . --ext .ts",
"lint:fix": "eslint . --ext .ts --fix",
"precompile": "tsc --version && lerna run version:update --scope @opentelemetry/resource-detector-gcp --include-dependencies",
"prewatch": "npm run precompile",
"prepublishOnly": "npm run compile",
"test": "nyc ts-mocha -p tsconfig.json 'test/**/*.test.ts'",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
"compile": "tsc -p .",
"lint": "eslint . --ext .ts",
"lint:fix": "eslint . --ext .ts --fix",
"precompile": "tsc --version && lerna run version:update --scope @opentelemetry/resource-detector-github --include-dependencies",
"prewatch": "npm run precompile",
"prepublishOnly": "npm run compile",
"test": "nyc ts-mocha -p tsconfig.json 'test/**/*.test.ts'",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
"compile": "tsc -p .",
"lint": "eslint . --ext .ts",
"lint:fix": "eslint . --ext .ts --fix",
"precompile": "tsc --version && lerna run version:update --scope @opentelemetry/resource-detector-instana --include-dependencies",
"prewatch": "npm run precompile",
"prepublishOnly": "npm run compile",
"test": "nyc ts-mocha -p tsconfig.json 'test/**/*.test.ts'",
Expand Down
1 change: 0 additions & 1 deletion metapackages/auto-configuration-propagators/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@
"compile": "tsc -p .",
"lint": "eslint . --ext .ts",
"lint:fix": "eslint . --ext .ts --fix",
"precompile": "tsc --version && lerna run version:update --scope @opentelemetry/auto-configuration-propagators --include-dependencies",
"prewatch": "npm run precompile",
"prepublishOnly": "npm run compile",
"tdd": "npm run test -- --watch-extensions ts --watch",
Expand Down
1 change: 0 additions & 1 deletion metapackages/auto-instrumentations-node/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
"compile": "tsc -p .",
"lint": "eslint . --ext .ts",
"lint:fix": "eslint . --ext .ts --fix",
"precompile": "tsc --version && lerna run version:update --scope @opentelemetry/auto-instrumentations-node --include-dependencies",
"prewatch": "npm run precompile",
"prepublishOnly": "npm run compile",
"tdd": "yarn test -- --watch-extensions ts --watch",
Expand Down
1 change: 0 additions & 1 deletion metapackages/auto-instrumentations-web/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
"compile": "tsc --build tsconfig.json tsconfig.esm.json tsconfig.esnext.json",
"lint": "eslint . --ext .ts",
"lint:fix": "eslint . --ext .ts --fix",
"precompile": "tsc --version && lerna run version:update --scope @opentelemetry/auto-instrumentations-web --include-dependencies",
"prewatch": "npm run precompile",
"prepublishOnly": "npm run compile",
"test:browser": "nyc karma start --single-run",
Expand Down
43 changes: 43 additions & 0 deletions nx.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,43 @@
{
"tasksRunnerOptions": {
"default": {
"runner": "nx/tasks-runners/default",
"options": {
"cacheableOperations": [
"compile",
"lint",
"version:update"
]
}
}
},
"targetDefaults": {
"compile": {
"dependsOn": [
"version:update",
"^compile"
Copy link
Contributor Author

Choose a reason for hiding this comment

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

note for reviewer: this is the syntax to tell that dependencies within the monorepo should be compiled before compiling the current package

],
"inputs": [
"{projectRoot}/src",
"{projectRoot}/test"
],
"outputs": [
"{projectRoot}/build"
]
},
"lint": {
"inputs": [
"{projectRoot}/src",
"{projectRoot}/test"
]
},
"version:update": {
"inputs": [
"{projectRoot}/package.json"
],
"outputs": [
"{projectRoot}/src/version.ts"
]
}
}
}
david-luna marked this conversation as resolved.
Show resolved Hide resolved
3 changes: 3 additions & 0 deletions package-lock.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

24 changes: 11 additions & 13 deletions package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,24 +12,21 @@
"main": "build/src/index.js",
"types": "build/src/index.d.ts",
"scripts": {
"clean": "lerna run clean",
"precompile": "tsc --version && npm run version:update",
"version:update": "lerna run version:update",
"compile": "lerna run compile",
"prewatch": "npm run precompile",
"test": "lerna run test",
"test:ci:changed": "lerna run test --since origin/main",
"test:browser": "lerna run test:browser --concurrency 1",
"test-all-versions": "npm run --if-present --workspaces test-all-versions",
"bump": "lerna publish",
david-luna marked this conversation as resolved.
Show resolved Hide resolved
"clean": "nx run-many -t clean",
"version:update": "nx run-many -t version:update",
"compile": "nx run-many -t compile",
"test": "nx run-many -t test",
"test:browser": "nx run-many -t test:browser",
"test:ci:changed": "nx affected -t test --base=origin/main --head=HEAD",
"test-all-versions": "nx run-many -t test-all-versions",
"changelog": "lerna-changelog",
"lint": "lerna run lint && npm run lint:readme && npm run lint:markdown",
"lint:fix": "lerna run lint:fix && npm run lint:markdown:fix",
"lint": "nx run-many -t lint && npm run lint:readme && npm run lint:markdown",
"lint:fix": "nx run-many -t lint:fix && npm run lint:markdown:fix",
"lint:examples": "eslint ./examples/**/*.js",
"lint:examples:fix": "eslint ./examples/**/*.js --fix",
"lint:markdown": "markdownlint-cli2 $(git ls-files '*.md')",
"lint:markdown:fix": "markdownlint-cli2 --fix $(git ls-files '*.md')",
"lint:readme": "lerna run lint:readme"
"lint:readme": "nx run-many -t lint:readme"
},
"keywords": [
"opentelemetry",
Expand All @@ -56,6 +53,7 @@
"lerna-changelog": "2.2.0",
"markdownlint-cli2": "0.13.0",
"minimatch": "^9.0.3",
"nx": "15.9.7",
"prettier": "2.8.8",
"semver": "^7.6.0",
"process": "0.11.10",
Expand Down
1 change: 0 additions & 1 deletion packages/baggage-span-processor/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
"compile": "tsc -p .",
"lint": "eslint . --ext .ts",
"lint:fix": "eslint . --ext .ts --fix",
"precompile": "tsc --version && lerna run version:update --scope @opentelemetry/baggage-span-processor --include-dependencies",
"prewatch": "npm run precompile",
"prepublishOnly": "npm run compile",
"tdd": "npm run test -- --watch-extensions ts --watch",
Expand Down
1 change: 0 additions & 1 deletion packages/opentelemetry-host-metrics/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
"compile": "tsc -p .",
"lint": "eslint . --ext .ts",
"lint:fix": "eslint . --ext .ts --fix",
"precompile": "tsc --version && lerna run version:update --scope @opentelemetry/host-metrics --include-dependencies",
"prewatch": "npm run precompile",
"prepublishOnly": "npm run compile",
"tdd": "npm run test -- --watch-extensions ts --watch",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
"types": "build/src/index.d.ts",
"repository": "open-telemetry/opentelemetry-js-contrib",
"scripts": {
"precompile": "tsc --version && lerna run version:update --scope @opentelemetry/id-generator-aws-xray --include-dependencies",
"prewatch": "npm run precompile",
"compile": "tsc --build tsconfig.json tsconfig.esm.json",
"clean": "tsc --build --clean tsconfig.json tsconfig.esm.json",
Expand Down
1 change: 0 additions & 1 deletion packages/opentelemetry-propagation-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
"compile": "tsc --build tsconfig.json tsconfig.esm.json",
"lint": "eslint . --ext .ts",
"lint:fix": "eslint . --ext .ts --fix",
"precompile": "tsc --version && lerna run version:update --scope @opentelemetry/propagation-utils --include-dependencies",
"prepublishOnly": "npm run compile",
"prewatch": "npm run precompile",
"tdd": "npm run test -- --watch-extensions ts --watch",
Expand Down
1 change: 0 additions & 1 deletion packages/opentelemetry-redis-common/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
"lint": "eslint . --ext .ts",
"lint:fix": "eslint . --ext .ts --fix",
"compile": "tsc --build tsconfig.json",
"precompile": "tsc --version && lerna run version:update --scope @opentelemetry/redis-common --include-dependencies",
"prewatch": "npm run precompile",
"prepublishOnly": "npm run compile",
"test": "nyc ts-mocha -p tsconfig.json 'test/**/*.test.ts'",
Expand Down
1 change: 0 additions & 1 deletion packages/opentelemetry-sql-common/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
"lint": "eslint . --ext .ts",
"lint:fix": "eslint . --ext .ts --fix",
"compile": "tsc --build tsconfig.json",
"precompile": "tsc --version && lerna run version:update --scope @opentelemetry/sql-common --include-dependencies",
"prewatch": "npm run precompile",
"prepublishOnly": "npm run compile",
"test": "nyc ts-mocha -p tsconfig.json 'test/**/*.test.ts'",
Expand Down
1 change: 0 additions & 1 deletion packages/opentelemetry-test-utils/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
"lint": "eslint . --ext .ts",
"lint:fix": "eslint . --ext .ts --fix",
"compile": "tsc -p .",
"precompile": "tsc --version && lerna run version:update --scope @opentelemetry/contrib-test-utils --include-dependencies",
"prewatch": "npm run precompile",
"prepublishOnly": "npm run compile",
"watch": "tsc -w"
Expand Down
1 change: 0 additions & 1 deletion packages/winston-transport/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@
"clean": "rimraf build/*",
"lint": "eslint . --ext .ts",
"lint:fix": "eslint . --ext .ts --fix",
"precompile": "tsc --version && lerna run version:update --scope @opentelemetry/winston-transport --include-dependencies",
"prewatch": "npm run precompile",
"prepublishOnly": "npm run compile",
"version:update": "node ../../scripts/version-update.js",
Expand Down
1 change: 0 additions & 1 deletion plugins/node/instrumentation-amqplib/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
"lint": "eslint . --ext .ts",
"lint:fix": "eslint . --ext .ts --fix",
"lint:readme": "node ../../../scripts/lint-readme.js",
"precompile": "tsc --version && lerna run version:update --scope @opentelemetry/instrumentation-amqplib --include-dependencies",
"prewatch": "npm run precompile",
"prepublishOnly": "npm run compile",
"tdd": "npm run test -- --watch-extensions ts --watch",
Expand Down
1 change: 0 additions & 1 deletion plugins/node/instrumentation-cucumber/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
"lint": "eslint . --ext .ts",
"lint:fix": "eslint . --ext .ts --fix",
"lint:readme": "node ../../../scripts/lint-readme.js",
"precompile": "tsc --version && lerna run version:update --scope @opentelemetry/instrumentation-cucumber --include-dependencies",
"prewatch": "npm run precompile",
"prepublishOnly": "npm run compile",
"version:update": "node ../../../scripts/version-update.js",
Expand Down
Loading
Loading