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

Great cleanup #463

Open
wants to merge 15 commits into
base: next
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
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
1 change: 1 addition & 0 deletions .github/RELEASE.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

- `opentelemetry/api {{api}}`
- `opentelemetry/core {{core}}`
- `opentelemetry/instrumentation {{instrumentation}}`

## Breaking changes

Expand Down
6 changes: 0 additions & 6 deletions .github/dependabot.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,7 @@ updates:
patterns:
- "turbo"
- "typescript"
- "tslib"
- "zig-build"
- "rollup"
- "@rollup/*"
- "rollup-*"
update-types:
- minor
- patch
Expand All @@ -51,8 +47,6 @@ updates:
- "typescript-eslint"
- "@eslint/*"
- "eslint-*"
- "@typescript-eslint/*"
- "globals"
update-types:
- minor
- patch
Expand Down
68 changes: 51 additions & 17 deletions .github/workflows/checks.yml
Original file line number Diff line number Diff line change
Expand Up @@ -35,8 +35,8 @@ jobs:
- run: yarn install --immutable

- run: yarn build
- run: yarn lint
- run: yarn test
- run: yarn lint --affected
- run: yarn test --affected
env:
SW_APM_COLLECTOR: ${{ secrets.SW_APM_COLLECTOR }}
SW_APM_SERVICE_KEY: ${{ secrets.SW_APM_SERVICE_KEY }}
Expand All @@ -53,6 +53,7 @@ jobs:
path: |
.root
packages/**/dist/
packages/solarwinds-apm/src/version.ts
packages/bindings/npm/*/*.node
packages/bindings/npm/*/liboboe.so
retention-days: 1
Expand Down Expand Up @@ -82,24 +83,19 @@ jobs:

- run: yarn version check

tests:
linux:
if: github.event_name == 'pull_request'
needs:
- checks
runs-on: ${{ matrix.arch == 'arm64' && fromJSON('{"group":"apm-arm-runner"}') || 'ubuntu-latest' }}
strategy:
matrix:
image:
- 16-alpine3.16
- 16-amazonlinux2023
- 16-debian10
- 16-ubi8
- 16-ubuntu20.04
- 18-alpine
- 18-amazonlinux
- 18-debian
- 18-ubi
- 18-ubuntu
- 18-alpine3.17
- 18-amazonlinux2023
- 18-debian10
- 18-ubi8
- 18-ubuntu20.04
- 20-alpine
- 20-amazonlinux
- 20-debian
Expand All @@ -115,9 +111,7 @@ jobs:
- arm64
# https://github.com/actions/runner/issues/801
exclude:
- image: 16-alpine3.16
arch: arm64
- image: 18-alpine
- image: 18-alpine3.17
arch: arm64
- image: 20-alpine
arch: arm64
Expand All @@ -136,7 +130,6 @@ jobs:
- uses: actions/checkout@v4
with:
lfs: true
submodules: true
- uses: actions/download-artifact@v4
with:
name: build
Expand All @@ -154,3 +147,44 @@ jobs:
env:
SW_APM_COLLECTOR: ${{ secrets.SW_APM_COLLECTOR }}
SW_APM_SERVICE_KEY: ${{ secrets.SW_APM_SERVICE_KEY }}

windows:
if: github.event_name == 'pull_request'
needs:
- checks
runs-on: windows-latest
strategy:
matrix:
node:
- 18
- 20
- 22
fail-fast: false
env:
YARN_IGNORE_NODE: 1

steps:
- uses: actions/checkout@v4
with:
lfs: true
- uses: actions/download-artifact@v4
with:
name: build
path: ./
- uses: actions/setup-node@v4
with:
node-version: ${{ matrix.node }}
- run: corepack enable

- uses: actions/cache@v4
with:
path: .yarn/cache
key: yarn-${{ hashFiles('yarn.lock') }}
restore-keys: |
yarn-

- run: yarn install --immutable
- run: yarn test --only
env:
SW_APM_COLLECTOR: ${{ secrets.SW_APM_COLLECTOR }}
SW_APM_SERVICE_KEY: ${{ secrets.SW_APM_SERVICE_KEY }}
15 changes: 5 additions & 10 deletions .github/workflows/images.yml
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,11 @@ jobs:
strategy:
matrix:
image:
- 16-alpine3.16
- 16-amazonlinux2023
- 16-debian10
- 16-ubi8
- 16-ubuntu20.04
- 18-alpine
- 18-amazonlinux
- 18-debian
- 18-ubi
- 18-ubuntu
- 18-alpine3.17
- 18-amazonlinux2023
- 18-debian10
- 18-ubi8
- 18-ubuntu20.04
- 20-alpine
- 20-amazonlinux
- 20-debian
Expand Down
2 changes: 2 additions & 0 deletions .github/workflows/release.yml
Original file line number Diff line number Diff line change
Expand Up @@ -56,9 +56,11 @@ jobs:
run: |
API=$(cat packages/solarwinds-apm/package.json | jq -r '.peerDependencies."@opentelemetry/api"')
CORE=$(cat packages/solarwinds-apm/package.json | jq -r '.dependencies."@opentelemetry/core"')
INSTRUMENTATION=$(cat packages/solarwinds-apm/package.json | jq -r '.dependencies."@opentelemetry/instrumentation"')
sed -i 's/{{arn}}/${{ needs.lambda.outputs.arn }}/' .github/RELEASE.md
sed -i "s/{{api}}/${API}/" .github/RELEASE.md
sed -i "s/{{core}}/${CORE}/" .github/RELEASE.md
sed -i "s/{{instrumentation}}/${INSTRUMENTATION}/" .github/RELEASE.md

- name: Create release draft
env:
Expand Down
8 changes: 2 additions & 6 deletions CONTRIBUTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ This project aims to support all currently maintained and future LTS Node versio

When a new future LTS is released (any even-numbered Node versions) the project should be updated to support it so that by the time it becomes an LTS it has already been well-tested. In most cases this doesn't require any change apart from updating the Docker images. It is however possible, although unlikely, that a future release may break native code or a dependency, which should be kept in mind.

When a current LTS goes out of maintenance, support for it should be removed. This project should not support its dependents in using unsupported, potentially insecure Node versions. The version of `@types/node` depended on by all packages should be updated to the next LTS version, for instance going from `^14.0.0` to `^16.0.0`. The `target` in the [base tsconfig](./tsconfig.base.json) should be updated to the highest standard supported by the next LTS version (check [`node.green`](https://node.green) for this) and the same should be done for the `languageOptions` in the ESLint configuration package. The Docker images for the old LTS should be removed, and the distro versions used by the Docker images for the next LTS should be changed to the lowest supported versions if necessary, for instance removing `14-alpine3.12` and moving from `16-alpine` to `16-alpine:3.12`. All `package.json` `engines` fields should also be updated to the next LTS version.
When a version has been EOL for over a year, support for it should be removed. This project should not encourage customers to use unsupported, potentially insecure Node versions. The version of `@types/node` depended on by all packages should be updated to the next LTS version, for instance going from `^14.0.0` to `^16.0.0`. The `target` in the [base tsconfig](./tsconfig.base.json) should be updated to the highest standard supported by the next LTS version (check [`node.green`](https://node.green) for this). The Docker images for the old LTS should be removed, and the distro versions used by the Docker images for the next LTS should be changed to the lowest supported versions if necessary, for instance removing `14-alpine3.12` and moving from `16-alpine` to `16-alpine:3.12`. All `package.json` `engines` fields should also be updated to the next LTS version.

## Upgrading dependencies

Expand Down Expand Up @@ -99,11 +99,7 @@ This is especially important given this repo uses Turborepo to run commands in a

## ES Modules vs CommonJS

We explicitly aim to support ESM

Packages should optimally target both ESM and CJS unless they have a good reason not to, in which case they should target CJS only for compatibility. At the moment the two CJS-only packages are the SDK because it's internal and has a lot of stateful components, and the bindings because they are internal and `.node` files can't be imported from ESM.

Dual targeting is made simple by the project's [Rollup configuration](./packages/rollup-config/), which should work without being extended pretty much all the time. Files ending in `.es.{ts, js}` will only be included for ESM `.cjs.{ts, js}` only for CJS. We don't use `.m{ts, js}` and `.c{ts, js}` because they have poor TypeScript support (and I, the original author, think they are Ugly and Bad). Projects should always specify `"type": "module"` when dual targeting (or an explicit `"commonjs"`) in their `package.json`.
We target ESM wherever possible, but all public facing API functions must be callable from either ESM or CommonJS.

Internal code (like scripts and examples) should attempt to use ESM as much as possible unless they are meant to demonstrate usage of the library from CJS.

Expand Down
2 changes: 1 addition & 1 deletion README.md
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ This project contains a few examples of how to use the library in the [`examples

## Node.js Version Support

The packages in this project support all currently maintained LTS versions. At the moment this means Node.js 16, 18 and 20 are supported.
The packages in this project support all LTS Node.js versions up until their End Of Life plus 1 year. At the moment this means Node.js 18, 20 and 22 are supported.

## License

Expand Down
17 changes: 0 additions & 17 deletions docker/16-amazonlinux2023.Dockerfile

This file was deleted.

18 changes: 0 additions & 18 deletions docker/16-ubuntu20.04.Dockerfile

This file was deleted.

15 changes: 0 additions & 15 deletions docker/18-alpine.Dockerfile

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM node:16-alpine3.16
FROM node:18-alpine3.17

RUN apk add --no-cache \
curl \
Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM amazonlinux
FROM amazonlinux:2023

RUN dnf install -y \
curl-minimal \
Expand Down
13 changes: 0 additions & 13 deletions docker/18-debian.Dockerfile

This file was deleted.

Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM node:16-buster-slim
FROM node:18-buster-slim

RUN apt-get update && apt-get install -y \
curl \
Expand Down
20 changes: 0 additions & 20 deletions docker/18-ubi.Dockerfile

This file was deleted.

2 changes: 1 addition & 1 deletion docker/16-ubi8.Dockerfile → docker/18-ubi8.Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@ RUN dnf install -y \
xz

RUN dnf module disable -y nodejs && \
curl -fsSL https://rpm.nodesource.com/setup_16.x | bash - && \
curl -fsSL https://rpm.nodesource.com/setup_18.x | bash - && \
dnf install -y nodejs && \
dnf clean -y all

Expand Down
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
FROM ubuntu
FROM ubuntu:20.04

RUN apt-get update && apt-get install -y \
ca-certificates \
Expand Down
Loading
Loading