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: Reduce dependencies and binary size, add circle ci detector #26522

Merged
merged 7 commits into from
Apr 17, 2023
Merged
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
4 changes: 1 addition & 3 deletions .circleci/workflows.yml
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ commands:
- run:
name: Check current branch to persist artifacts
command: |
if [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "release/"* && "$CIRCLE_BRANCH" != "update-v8-snapshot-cache-on-develop" ]]; then
if [[ "$CIRCLE_BRANCH" != "develop" && "$CIRCLE_BRANCH" != "release/"* && "$CIRCLE_BRANCH" != "update-v8-snapshot-cache-on-develop" && "$CIRCLE_BRANCH" != "matth/chore/add-circle-ci-detector" ]]; then
echo "Not uploading artifacts or posting install comment for this branch."
circleci-agent step halt
fi
Expand Down Expand Up @@ -493,7 +493,6 @@ commands:
# internal PR
CYPRESS_RECORD_KEY=$MAIN_RECORD_KEY \
CYPRESS_INTERNAL_ENABLE_TELEMETRY="true" \
OTEL_RESOURCE_ATTRIBUTES="ci.branch=$CIRCLE_BRANCH,ci.job=$CIRCLE_JOB,ci.node-index=$CIRCLE_NODE_INDEX,ci.circle=$CIRCLECI,ci.build-url=$CIRCLE_BUILD_URL,ci.build-number=$CIRCLE_BUILD_NUM" \
Copy link
Member Author

Choose a reason for hiding this comment

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

the circle ci detector replaces the need for this line

yarn cypress:run --record --parallel --group 5x-driver-<<parameters.browser>> --browser <<parameters.browser>>
else
# external PR
Expand Down Expand Up @@ -567,7 +566,6 @@ commands:
PERCY_ENABLE=${PERCY_TOKEN:-0} \
PERCY_PARALLEL_TOTAL=-1 \
CYPRESS_INTERNAL_ENABLE_TELEMETRY="true" \
OTEL_RESOURCE_ATTRIBUTES="ci.branch=$CIRCLE_BRANCH,ci.job=$CIRCLE_JOB,ci.node-index=$CIRCLE_NODE_INDEX,ci.circle=$CIRCLECI,ci.build-url=$CIRCLE_BUILD_URL,ci.build-number=$CIRCLE_BUILD_NUM" \
$cmd yarn workspace @packages/<<parameters.package>> cypress:run:<<parameters.type>> --browser <<parameters.browser>> --record --parallel --group <<parameters.package>>-<<parameters.type>>
else
# external PR
Expand Down
15 changes: 8 additions & 7 deletions packages/telemetry/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,14 @@
},
"dependencies": {
"@opentelemetry/api": "1.4.1",
"@opentelemetry/auto-instrumentations-node": "0.36.4",
Copy link
Member Author

Choose a reason for hiding this comment

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

specifically we don't need this dep

"@opentelemetry/exporter-trace-otlp-http": "0.36.1",
"@opentelemetry/instrumentation": "0.36.1",
Copy link
Member Author

Choose a reason for hiding this comment

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

or this one

"@opentelemetry/otlp-exporter-base": "0.36.1",
"@opentelemetry/sdk-trace-base": "1.10.1",
"@opentelemetry/sdk-trace-node": "1.10.1",
"@opentelemetry/sdk-trace-web": "1.10.1"
"@opentelemetry/core": "1.12.0",
"@opentelemetry/exporter-trace-otlp-http": "0.38.0",
"@opentelemetry/otlp-exporter-base": "0.38.0",
"@opentelemetry/resources": "1.12.0",
"@opentelemetry/sdk-trace-base": "1.12.0",
"@opentelemetry/sdk-trace-node": "1.12.0",
"@opentelemetry/sdk-trace-web": "1.12.0",
"@opentelemetry/semantic-conventions": "1.12.0"
},
"devDependencies": {
"@packages/ts": "0.0.0-development",
Expand Down
35 changes: 35 additions & 0 deletions packages/telemetry/src/detectors/circleCiDetectorSync.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
import type { DetectorSync, ResourceAttributes, IResource } from '@opentelemetry/resources'
import { Resource } from '@opentelemetry/resources'

/**
* CircleCiDetectorSync can be used to detect the presence of and create a Resource
* from circle ci env variables.
*/
class CircleCiDetectorSync implements DetectorSync {
/**
* Returns a {@link Resource} populated with attributes from the
* circle ci environment variable.
*
* @param config The resource detection config -- ignored
*/
detect (): IResource {
const attributes: ResourceAttributes = {}

const { CIRCLECI, CIRCLE_BRANCH, CIRCLE_JOB, CIRCLE_NODE_INDEX, CIRCLE_BUILD_URL, CIRCLE_BUILD_NUM, CIRCLE_SHA1, CIRCLE_PR_NUMBER } = process.env

if (CIRCLECI) {
attributes['ci.circle'] = CIRCLECI
attributes['ci.branch'] = CIRCLE_BRANCH
attributes['ci.job'] = CIRCLE_JOB
attributes['ci.node'] = CIRCLE_NODE_INDEX
attributes['ci.build-url'] = CIRCLE_BUILD_URL
attributes['ci.build-number'] = CIRCLE_BUILD_NUM
attributes['SHA1'] = CIRCLE_SHA1
attributes['ci.pr-number'] = CIRCLE_PR_NUMBER
}

return new Resource(attributes)
}
}

export const circleCiDetectorSync = new CircleCiDetectorSync()
3 changes: 2 additions & 1 deletion packages/telemetry/src/node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import type { startSpanOptions, findActiveSpanOptions, contextObject } from './i
import { Telemetry as TelemetryClass, TelemetryNoop } from './index'
import { NodeTracerProvider } from '@opentelemetry/sdk-trace-node'
import { envDetectorSync, processDetectorSync, osDetectorSync, hostDetectorSync } from '@opentelemetry/resources'
import { circleCiDetectorSync } from './detectors/circleCiDetectorSync'
import { BatchSpanProcessor } from '@opentelemetry/sdk-trace-base'

import { OTLPTraceExporter as OTLPTraceExporterIpc } from './span-exporters/ipc-span-exporter'
Expand Down Expand Up @@ -50,7 +51,7 @@ const init = ({
namespace,
Provider: NodeTracerProvider,
detectors: [
envDetectorSync, processDetectorSync, osDetectorSync, hostDetectorSync,
envDetectorSync, processDetectorSync, osDetectorSync, hostDetectorSync, circleCiDetectorSync,
],
rootContextObject: context,
version,
Expand Down
107 changes: 107 additions & 0 deletions packages/telemetry/test/detectors/circleCiDetectorSync.spec.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,107 @@
import { expect } from 'chai'


import { circleCiDetectorSync } from '../../src/detectors/circleCiDetectorSync'

describe('circleCiDetectorSync', () => {
describe('undefined values', () => {
const processValues: any = {}

beforeEach(() => {
// cache values
processValues.CIRCLECI = process.env.CIRCLECI
processValues.CIRCLE_BRANCH = process.env.CIRCLE_BRANCH
processValues.CIRCLE_JOB = process.env.CIRCLE_JOB
processValues.CIRCLE_NODE_INDEX = process.env.CIRCLE_NODE_INDEX
processValues.CIRCLE_BUILD_URL = process.env.CIRCLE_BUILD_URL
processValues.CIRCLE_BUILD_NUM = process.env.CIRCLE_BUILD_NUM
processValues.CIRCLE_SHA1 = process.env.CIRCLE_SHA1
processValues.CIRCLE_PR_NUMBER = process.env.CIRCLE_PR_NUMBER

//reset values
delete process.env.CIRCLECI
delete process.env.CIRCLE_BRANCH
delete process.env.CIRCLE_JOB
delete process.env.CIRCLE_NODE_INDEX
delete process.env.CIRCLE_BUILD_URL
delete process.env.CIRCLE_BUILD_NUM
delete process.env.CIRCLE_SHA1
delete process.env.CIRCLE_PR_NUMBER
})

afterEach(() => {
// Replace values
process.env.CIRCLECI = processValues.CIRCLECI
Copy link
Contributor

Choose a reason for hiding this comment

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

Does this condition need to be reversed so we delete the circle process variables so they don't leak out to other tests in the process?

Copy link
Member Author

Choose a reason for hiding this comment

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

this is the reversing, we cache the values, delete them, then reset them to the cache

Copy link
Contributor

Choose a reason for hiding this comment

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

Ah that makes a lot of sense now 😅

process.env.CIRCLE_BRANCH = processValues.CIRCLE_BRANCH
process.env.CIRCLE_JOB = processValues.CIRCLE_JOB
process.env.CIRCLE_NODE_INDEX = processValues.CIRCLE_NODE_INDEX
process.env.CIRCLE_BUILD_URL = processValues.CIRCLE_BUILD_URL
process.env.CIRCLE_BUILD_NUM = processValues.CIRCLE_BUILD_NUM
process.env.CIRCLE_SHA1 = processValues.CIRCLE_SHA1
process.env.CIRCLE_PR_NUMBER = processValues.CIRCLE_PR_NUMBER
})

describe('detect', () => {
it('returns an empty resource', () => {
const resource = circleCiDetectorSync.detect()

expect(resource.attributes).to.be.empty
})
})
})

describe('defined values', () => {
const processValues: any = {}

beforeEach(() => {
// cache values
processValues.CIRCLECI = process.env.CIRCLECI
processValues.CIRCLE_BRANCH = process.env.CIRCLE_BRANCH
processValues.CIRCLE_JOB = process.env.CIRCLE_JOB
processValues.CIRCLE_NODE_INDEX = process.env.CIRCLE_NODE_INDEX
processValues.CIRCLE_BUILD_URL = process.env.CIRCLE_BUILD_URL
processValues.CIRCLE_BUILD_NUM = process.env.CIRCLE_BUILD_NUM
processValues.CIRCLE_SHA1 = process.env.CIRCLE_SHA1
processValues.CIRCLE_PR_NUMBER = process.env.CIRCLE_PR_NUMBER

//reset values
process.env.CIRCLECI = 'circleCi'
process.env.CIRCLE_BRANCH = 'circleBranch'
process.env.CIRCLE_JOB = 'circleJob'
process.env.CIRCLE_NODE_INDEX = 'circleNodeIndex'
process.env.CIRCLE_BUILD_URL = 'circleBuildUrl'
process.env.CIRCLE_BUILD_NUM = 'circleBuildNum'
process.env.CIRCLE_SHA1 = 'circleSha1'
process.env.CIRCLE_PR_NUMBER = 'circlePrNumber'
})

afterEach(() => {
// Replace values
process.env.CIRCLECI = processValues.CIRCLECI
process.env.CIRCLE_BRANCH = processValues.CIRCLE_BRANCH
process.env.CIRCLE_JOB = processValues.CIRCLE_JOB
process.env.CIRCLE_NODE_INDEX = processValues.CIRCLE_NODE_INDEX
process.env.CIRCLE_BUILD_URL = processValues.CIRCLE_BUILD_URL
process.env.CIRCLE_BUILD_NUM = processValues.CIRCLE_BUILD_NUM
process.env.CIRCLE_SHA1 = processValues.CIRCLE_SHA1
process.env.CIRCLE_PR_NUMBER = processValues.CIRCLE_PR_NUMBER
})

describe('detect', () => {
it('returns a resource with attributes', () => {
const resource = circleCiDetectorSync.detect()

console.log(resource.attributes)

expect(resource.attributes['ci.circle']).to.equal('circleCi')
expect(resource.attributes['ci.branch']).to.equal('circleBranch')
expect(resource.attributes['ci.job']).to.equal('circleJob')
expect(resource.attributes['ci.node']).to.equal('circleNodeIndex')
expect(resource.attributes['ci.build-url']).to.equal('circleBuildUrl')
expect(resource.attributes['ci.build-number']).to.equal('circleBuildNum')
expect(resource.attributes['SHA1']).to.equal('circleSha1')
expect(resource.attributes['ci.pr-number']).to.equal('circlePrNumber')
})
})
})
})
2 changes: 2 additions & 0 deletions scripts/binary/binary-cleanup.js
Original file line number Diff line number Diff line change
Expand Up @@ -201,6 +201,8 @@ const buildEntryPointAndCleanup = async (buildAppDir) => {
path.join(buildAppDir, '**', 'nexus', 'dist-esm'),
path.join(buildAppDir, '**', '@graphql-tools', '**', '*.mjs'),
path.join(buildAppDir, '**', 'graphql', '**', '*.mjs'),
path.join(buildAppDir, '**', '@openTelemetry', '**', 'esm'),
path.join(buildAppDir, '**', '@openTelemetry', '**', 'esnext'),
Comment on lines +204 to +205
Copy link
Member Author

Choose a reason for hiding this comment

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

these lines remove the esm and esnext folders from open telemetry, we don't use them.

// We currently do not use any map files
path.join(buildAppDir, '**', '*js.map'),
// License files need to be kept
Expand Down
Loading