Skip to content

Commit

Permalink
Merge pull request #1248 from isomerpages/release_v0.73.0
Browse files Browse the repository at this point in the history
* chore(package): use npm (#1237)

* fix: upgrade dotenv from 16.4.1 to 16.4.5 (#1233)

Snyk has created this PR to upgrade dotenv from 16.4.1 to 16.4.5.

See this package in npm:
https://www.npmjs.com/package/dotenv

See this project in Snyk:
https://app.snyk.io/org/isomer/project/676b9e26-cebf-4964-b7b3-d9843e3339ff?utm_source=github&utm_medium=referral&page=upgrade-pr

Co-authored-by: snyk-bot <[email protected]>

* build(deps): bump express from 4.17.3 to 4.19.2 (#1242)

Bumps [express](https://github.com/expressjs/express) from 4.17.3 to 4.19.2.
- [Release notes](https://github.com/expressjs/express/releases)
- [Changelog](https://github.com/expressjs/express/blob/master/History.md)
- [Commits](expressjs/express@4.17.3...4.19.2)

---
updated-dependencies:
- dependency-name: express
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>

* fix: package.json & package-lock.json to reduce vulnerabilities (#1241)

The following vulnerabilities are fixed with an upgrade:
- https://snyk.io/vuln/SNYK-JS-EXPRESS-6474509

Co-authored-by: snyk-bot <[email protected]>

* fix: upgrade @growthbook/growthbook from 0.27.0 to 0.34.0 (#1232)

Snyk has created this PR to upgrade @growthbook/growthbook from 0.27.0 to 0.34.0.

See this package in npm:
https://www.npmjs.com/package/@growthbook/growthbook

See this project in Snyk:
https://app.snyk.io/org/isomer/project/676b9e26-cebf-4964-b7b3-d9843e3339ff?utm_source=github&utm_medium=referral&page=upgrade-pr

Co-authored-by: snyk-bot <[email protected]>

* fix: upgrade @aws-sdk/client-dynamodb from 3.501.0 to 3.521.0 (#1229)

Snyk has created this PR to upgrade @aws-sdk/client-dynamodb from 3.501.0 to 3.521.0.

See this package in npm:
https://www.npmjs.com/package/@aws-sdk/client-dynamodb

See this project in Snyk:
https://app.snyk.io/org/isomer/project/676b9e26-cebf-4964-b7b3-d9843e3339ff?utm_source=github&utm_medium=referral&page=upgrade-pr

Co-authored-by: snyk-bot <[email protected]>

* fix: upgrade @aws-sdk/client-amplify from 3.501.0 to 3.521.0 (#1230)

Snyk has created this PR to upgrade @aws-sdk/client-amplify from 3.501.0 to 3.521.0.

See this package in npm:
https://www.npmjs.com/package/@aws-sdk/client-amplify

See this project in Snyk:
https://app.snyk.io/org/isomer/project/676b9e26-cebf-4964-b7b3-d9843e3339ff?utm_source=github&utm_medium=referral&page=upgrade-pr

Co-authored-by: snyk-bot <[email protected]>

* fix: upgrade @aws-sdk/client-cloudwatch-logs from 3.501.0 to 3.521.0 (#1231)

Snyk has created this PR to upgrade @aws-sdk/client-cloudwatch-logs from 3.501.0 to 3.521.0.

See this package in npm:
https://www.npmjs.com/package/@aws-sdk/client-cloudwatch-logs

See this project in Snyk:
https://app.snyk.io/org/isomer/project/676b9e26-cebf-4964-b7b3-d9843e3339ff?utm_source=github&utm_medium=referral&page=upgrade-pr

Co-authored-by: snyk-bot <[email protected]>

* fix(dockerfile): add dig to image (#1244)

* feat(dd): add traces to gitfilesysteM (#1240)

* feat(dd): adding in metrics to gitfilesystem

* feat: instrument with wrap-tracers

* feat: patch dd-trace to understand neverthrow

* fix: missing tracer import

* fix: use the clear tracer.wrap() method

* fix(dd): fix patch type

* chore: add tests for tracer compatibility with neverthrow

* chore: upgrade dd-trace to v5 and patch for neverthrow

* chore: remove unecessary change from patch

* feat: implement a blacklist for methods that should not be instrumented

* chore: remove obsolete patch

* chore: lock dd-trace to 5.9.0 strictly so patch is always applied

---------

Co-authored-by: Timothee Groleau <[email protected]>

* fix: reduce log size to just last commit (#1243)

* fix: reduce log size to just last commit

* feat: defaults maxCount in getGitLog to 1

* chore(GitFileSystemService): add validation tests for getGitLog

* chore(dep): add flag (#1247)

* chore: bump version to v0.73.0

---------

Signed-off-by: dependabot[bot] <[email protected]>
Co-authored-by: seaerchin <[email protected]>
Co-authored-by: Isomer Admin <[email protected]>
Co-authored-by: snyk-bot <[email protected]>
Co-authored-by: dependabot[bot] <49699333+dependabot[bot]@users.noreply.github.com>
Co-authored-by: Timothee Groleau <[email protected]>
Co-authored-by: Timothee Groleau <[email protected]>
  • Loading branch information
7 people authored Mar 28, 2024
2 parents a8636ef + fbac13b commit ece3934
Show file tree
Hide file tree
Showing 8 changed files with 1,108 additions and 768 deletions.
19 changes: 19 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,26 @@ All notable changes to this project will be documented in this file. Dates are d

Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog).

#### [v0.73.0](https://github.com/isomerpages/isomercms-backend/compare/v0.72.0...v0.73.0)

- chore(dep): add flag [`#1247`](https://github.com/isomerpages/isomercms-backend/pull/1247)
- fix: reduce log size to just last commit [`#1243`](https://github.com/isomerpages/isomercms-backend/pull/1243)
- feat(dd): add traces to gitfilesysteM [`#1240`](https://github.com/isomerpages/isomercms-backend/pull/1240)
- fix(dockerfile): add dig to image [`#1244`](https://github.com/isomerpages/isomercms-backend/pull/1244)
- fix: upgrade @aws-sdk/client-cloudwatch-logs from 3.501.0 to 3.521.0 [`#1231`](https://github.com/isomerpages/isomercms-backend/pull/1231)
- fix: upgrade @aws-sdk/client-amplify from 3.501.0 to 3.521.0 [`#1230`](https://github.com/isomerpages/isomercms-backend/pull/1230)
- fix: upgrade @aws-sdk/client-dynamodb from 3.501.0 to 3.521.0 [`#1229`](https://github.com/isomerpages/isomercms-backend/pull/1229)
- fix: upgrade @growthbook/growthbook from 0.27.0 to 0.34.0 [`#1232`](https://github.com/isomerpages/isomercms-backend/pull/1232)
- fix: package.json & package-lock.json to reduce vulnerabilities [`#1241`](https://github.com/isomerpages/isomercms-backend/pull/1241)
- build(deps): bump express from 4.17.3 to 4.19.2 [`#1242`](https://github.com/isomerpages/isomercms-backend/pull/1242)
- fix: upgrade dotenv from 16.4.1 to 16.4.5 [`#1233`](https://github.com/isomerpages/isomercms-backend/pull/1233)
- chore(package): use npm [`#1237`](https://github.com/isomerpages/isomercms-backend/pull/1237)
- backport v0.72.0 [`#1236`](https://github.com/isomerpages/isomercms-backend/pull/1236)

#### [v0.72.0](https://github.com/isomerpages/isomercms-backend/compare/v0.71.0...v0.72.0)

> 21 March 2024
- fix(link checker): wrong error reported [`#1227`](https://github.com/isomerpages/isomercms-backend/pull/1227)
- fix(tags): update tagging for dd [`#1222`](https://github.com/isomerpages/isomercms-backend/pull/1222)
- perf(I/O): rm blocking fs calls [`#1220`](https://github.com/isomerpages/isomercms-backend/pull/1220)
Expand All @@ -24,6 +42,7 @@ Generated by [`auto-changelog`](https://github.com/CookPete/auto-changelog).
- fix: upgrade isomorphic-dompurify from 0.24.0 to 0.27.0 [`#881`](https://github.com/isomerpages/isomercms-backend/pull/881)
- backport v0.71.0 [`#1214`](https://github.com/isomerpages/isomercms-backend/pull/1214)
- build(deps): bump @aws-sdk/client-secrets-manager [`#1218`](https://github.com/isomerpages/isomercms-backend/pull/1218)
- chore: bump version to v0.72.0 [`7b8c157`](https://github.com/isomerpages/isomercms-backend/commit/7b8c157b52bdaa120bca1113a1a0859c6a126028)

#### [v0.71.0](https://github.com/isomerpages/isomercms-backend/compare/v0.70.0...v0.71.0)

Expand Down
3 changes: 2 additions & 1 deletion Dockerfile
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,8 @@ WORKDIR /opt/isomercms-backend
RUN apk update && \
apk add --no-cache bash && \
apk add git && \
apk add openssh-client
apk add openssh-client && \
apk add bind-tools

RUN adduser -u 900 webapp -D -h /home/webapp -s /bin/sh
USER webapp
Expand Down
1,633 changes: 886 additions & 747 deletions package-lock.json

Large diffs are not rendered by default.

24 changes: 13 additions & 11 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "isomercms",
"version": "0.72.0",
"version": "0.73.0",
"private": true,
"scripts": {
"build": "tsc -p tsconfig.build.json",
Expand All @@ -11,7 +11,7 @@
"dev": "source .env && docker compose -f docker-compose.dev.yml up",
"test:docker": "docker run -d -p 54321:5432 --name postgres -e POSTGRES_USER=isomer -e POSTGRES_PASSWORD=password -e POSTGRES_DB=isomercms_test postgres:latest",
"test": "source .env.test && jest --runInBand",
"release": "npm version $npm_config_isomer_update && git push --tags",
"release": "bash scripts/release_prep.sh",
"lint": "npx eslint .",
"lint-fix": "eslint --ignore-path .gitignore . --fix",
"format": "npx prettier --check .",
Expand All @@ -23,15 +23,16 @@
"jump:staging": "source .ssh/.env.staging && ssh -L 5433:$DB_HOST:5432 $SSH_USER@$SSH_HOST -i .ssh/isomercms-staging-bastion.pem",
"db:migrate:staging": "source .ssh/.env.staging && npx sequelize-cli db:migrate",
"jump:prod": "source .ssh/.env.prod && ssh -L 5433:$DB_HOST:5432 $SSH_USER@$SSH_HOST -i .ssh/isomercms-production-bastion.pem",
"db:migrate:prod": "source .ssh/.env.prod && npx sequelize-cli db:migrate"
"db:migrate:prod": "source .ssh/.env.prod && npx sequelize-cli db:migrate",
"postinstall": "patch-package"
},
"dependencies": {
"@aws-sdk/client-amplify": "^3.382.0",
"@aws-sdk/client-dynamodb": "^3.370.0",
"@aws-sdk/client-cloudwatch-logs": "^3.370.0",
"@aws-sdk/client-dynamodb": "^3.521.0",
"@aws-sdk/client-amplify": "^3.521.0",
"@aws-sdk/client-cloudwatch-logs": "^3.521.0",
"@aws-sdk/client-secrets-manager": "^3.389.0",
"@aws-sdk/lib-dynamodb": "^3.521.0",
"@growthbook/growthbook": "^0.27.0",
"@growthbook/growthbook": "^0.34.0",
"@octokit/plugin-retry": "^6.0.0",
"@octokit/rest": "^18.12.0",
"@opengovsg/formsg-sdk": "^0.11.0",
Expand All @@ -40,8 +41,8 @@
"@types/node": "^20.11.13",
"auto-bind": "^4.0.0",
"aws-lambda": "^1.0.7",
"axios": "^1.6.5",
"aws-sdk": "^2.1565.0",
"axios": "^1.6.5",
"axios-cache-interceptor": "^0.10.7",
"base-64": "^0.1.0",
"bcryptjs": "^2.4.3",
Expand All @@ -55,13 +56,13 @@
"cors": "^2.8.5",
"cross-fetch": "^4.0.0",
"crypto-js": "^4.2.0",
"dd-trace": "^4.11.0",
"dd-trace": "5.9.0",
"debug": "~2.6.9",
"dompurify": "^3.0.9",
"dotenv": "^16.3.1",
"dotenv": "^16.4.5",
"eventsource": "^2.0.2",
"exponential-backoff": "^3.1.0",
"express": "~4.17.3",
"express": "~4.19.2",
"express-rate-limit": "^6.7.0",
"express-session": "^1.17.3",
"file-type": "^16.5.4",
Expand All @@ -87,6 +88,7 @@
"node-dig-dns": "^0.3.3",
"otplib": "^12.0.1",
"papaparse": "^5.4.1",
"patch-package": "^8.0.0",
"pg": "^8.6.0",
"pg-connection-string": "^2.5.0",
"pino": "^8.19.0",
Expand Down
25 changes: 25 additions & 0 deletions patches/dd-trace+5.9.0.patch
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
diff --git a/node_modules/dd-trace/packages/dd-trace/src/tracer.js b/node_modules/dd-trace/packages/dd-trace/src/tracer.js
index 63c60e8..49b5b7e 100644
--- a/node_modules/dd-trace/packages/dd-trace/src/tracer.js
+++ b/node_modules/dd-trace/packages/dd-trace/src/tracer.js
@@ -73,7 +73,19 @@ class DatadogTracer extends Tracer {

const result = this.scope().activate(span, () => fn(span))

- if (result && typeof result.then === 'function') {
+ if (result && typeof result.andThen === 'function') {
+ return result
+ .map(value => {
+ span.finish()
+ return value
+ })
+ .mapErr(error => {
+ addError(span, error)
+ span.finish()
+ return error
+ });
+ }
+ else if (result && typeof result.then === 'function') {
return result.then(
value => {
span.finish()
63 changes: 54 additions & 9 deletions src/services/db/GitFileSystemService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@ import GitFileSystemError from "@errors/GitFileSystemError"
import GitFileSystemNeedsRollbackError from "@errors/GitFileSystemNeedsRollbackError"
import { NotFoundError } from "@errors/NotFoundError"

import tracer from "@utils/tracer"

import {
EFS_VOL_PATH_STAGING,
EFS_VOL_PATH_STAGING_LITE,
Expand All @@ -42,11 +44,46 @@ import type { IsomerCommitMessage } from "@root/types/github"
import { ALLOWED_FILE_EXTENSIONS } from "@root/utils/file-upload-utils"
import { getPaginatedDirectoryContents } from "@root/utils/files"

// methods that do not need to be wrapped for instrumentation
const METHOD_INSTRUMENTATION_BLACKLIST = [
// constructor cannot be instrumented with tracer.wrap() because it would lose invocation with 'new'
"constructor",

// these methods only check and return env vars, tracking spans for them is useless and wasteful
"getEfsVolPathFromBranch",
"getEfsVolPath",
"isStagingFromBranchName",
]

export default class GitFileSystemService {
private readonly git: SimpleGit

constructor(git: SimpleGit) {
this.git = git

// Below is a "dirty" hack to instrument ALL methods of GitFileSystemService
// all methods will create spans for visibility in traces

/* eslint-disable no-restricted-syntax */
/* eslint-disable @typescript-eslint/ban-ts-comment */
for (const methodName of Object.getOwnPropertyNames(
GitFileSystemService.prototype
)) {
// eslint-disable-next-line no-continue
if (METHOD_INSTRUMENTATION_BLACKLIST.includes(methodName)) continue

// @ts-ignore
const method = this[methodName]
if (typeof method === "function") {
// @ts-ignore
this[methodName] = tracer.wrap(
`GitFileSystem.${methodName}`,
method.bind(this)
)
}
}
/* eslint-enable @typescript-eslint/ban-ts-comment */
/* eslint-enable no-restricted-syntax */
}

private getEfsVolPathFromBranch(branchName: string): string {
Expand Down Expand Up @@ -416,13 +453,22 @@ export default class GitFileSystemService {
// Get the Git log of a particular branch
getGitLog(
repoName: string,
branchName: string
branchName: string,
maxCount = 1
): ResultAsync<LogResult<DefaultLogFields>, GitFileSystemError> {
const efsVolPath = this.getEfsVolPathFromBranch(branchName)

if (!Number.isInteger(maxCount) || maxCount < 1) {
return errAsync(
new GitFileSystemError(`Invalid maxCount value supplied: ${maxCount}`)
)
}

return ResultAsync.fromPromise(
this.git
.cwd({ path: `${efsVolPath}/${repoName}`, root: false })
.log([branchName]),
.log([`--max-count=${maxCount}`, branchName]),

(error) => {
logger.error(
`Error when getting Git log of "${branchName}" branch: ${error}, when trying to access ${efsVolPath}/${repoName} for ${branchName}`
Expand Down Expand Up @@ -1726,13 +1772,12 @@ export default class GitFileSystemService {
branchName: string
): ResultAsync<GitHubCommitData, GitFileSystemError> {
return this.isLocalBranchPresent(repoName, branchName)
.andThen((isBranchLocallyPresent) => {
if (isBranchLocallyPresent) {
return this.getGitLog(repoName, branchName)
}

return this.getGitLog(repoName, `origin/${branchName}`)
})
.andThen((isBranchLocallyPresent) =>
this.getGitLog(
repoName,
isBranchLocallyPresent ? branchName : `origin/${branchName}`
)
)
.andThen((logSummary) => {
const possibleCommit = logSummary.latest
if (this.isDefaultLogFields(possibleCommit)) {
Expand Down
28 changes: 28 additions & 0 deletions src/services/db/__tests__/GitFileSystemService.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -809,6 +809,34 @@ describe("GitFileSystemService", () => {

expect(result._unsafeUnwrapErr()).toBeInstanceOf(GitFileSystemError)
})

it("should return GitFileSystemError if maxCount is supplied as a float", async () => {
const result = await GitFileSystemService.getGitLog(
"fake-repo",
"fake-commit-sha",
1.1 // float value
)

expect(result._unsafeUnwrapErr()).toBeInstanceOf(GitFileSystemError)
})

it("should return GitFileSystemError if a maxCount less than 1 is supplied", async () => {
const result1 = await GitFileSystemService.getGitLog(
"fake-repo",
"fake-commit-sha",
0
)

expect(result1._unsafeUnwrapErr()).toBeInstanceOf(GitFileSystemError)

const result2 = await GitFileSystemService.getGitLog(
"fake-repo",
"fake-commit-sha",
-1
)

expect(result2._unsafeUnwrapErr()).toBeInstanceOf(GitFileSystemError)
})
})

describe("rollback", () => {
Expand Down
81 changes: 81 additions & 0 deletions src/utils/__tests__/tracer.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,81 @@
// @ts-nocheck

/*
this test file is not testing our own code.
Because dd-trace is not compatible with neverthrow, we introduced a patch to fix dd-trace itself.
This test file verifies that the patch we apply does what it intends to, which is to retain the neverthrow
interface when code that is traced or wrapped return a neverthrow ResultAsync result.
*/

import tracer from "dd-trace"
import { errAsync, okAsync } from "neverthrow"

describe("NeverThrow compatibility", () => {
describe("tracer.trace()", () => {
it("should maintain the neverthrow interface for ok results", async () => {
const resultAsync = tracer.trace("foo", () => okAsync(1))

expect(resultAsync.then).toBeDefined() // promise interface
expect(resultAsync.andThen).toBeDefined() // neverthrow interface
expect(resultAsync.orElse).toBeDefined()

const res = await resultAsync

expect(res.isOk()).toStrictEqual(true)
expect(res.unwrapOr(0)).toStrictEqual(1)
})

it("should maintain the neverthrow interface for error results", async () => {
const resultAsync = tracer.trace("foo", () =>
errAsync(new Error("foo error"))
)

expect(resultAsync.then).toBeDefined() // promise interface
expect(resultAsync.andThen).toBeDefined() // neverthrow interface
expect(resultAsync.orElse).toBeDefined()

const res = await resultAsync

expect(res.isOk()).toStrictEqual(false)
expect(res.isErr()).toStrictEqual(true)

expect(res.error.message).toStrictEqual("foo error")
})
})

describe("tracer.wrap()", () => {
it("should maintain the neverthrow interface for ok results", async () => {
const foo = () => okAsync(1)
const wrappedFoo = tracer.wrap("foo", foo)

const resultAsync = wrappedFoo()

expect(resultAsync.then).toBeDefined() // promise interface
expect(resultAsync.andThen).toBeDefined() // neverthrow interface
expect(resultAsync.orElse).toBeDefined()

const res = await resultAsync

expect(res.isOk()).toStrictEqual(true)
expect(res.unwrapOr(0)).toStrictEqual(1)
})

it("should maintain the neverthrow interface for error results", async () => {
const foo = () => errAsync(new Error("foo error"))
const wrappedFoo = tracer.wrap("foo", foo)

const resultAsync = wrappedFoo()

expect(resultAsync.then).toBeDefined() // promise interface
expect(resultAsync.andThen).toBeDefined() // neverthrow interface
expect(resultAsync.orElse).toBeDefined()

const res = await resultAsync

expect(res.isOk()).toStrictEqual(false)
expect(res.isErr()).toStrictEqual(true)

expect(res.error.message).toStrictEqual("foo error")
})
})
})

0 comments on commit ece3934

Please sign in to comment.