Skip to content

Commit

Permalink
Ignore traces on incoming request on / and /ping
Browse files Browse the repository at this point in the history
Add `deployment.environment=local` as default env variable.

Default diagLogLevel to ERROR.
  • Loading branch information
wcalderipe committed Nov 8, 2024
1 parent 0e0afeb commit 07a433b
Show file tree
Hide file tree
Showing 18 changed files with 171 additions and 38 deletions.
2 changes: 1 addition & 1 deletion .github/workflows/vault.yml
Original file line number Diff line number Diff line change
Expand Up @@ -185,4 +185,4 @@ jobs:
status: ${{ job.status }}
fields: message,commit,author
env:
SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK }}
SLACK_WEBHOOK_URL: ${{ secrets.SLACK_WEBHOOK }}
7 changes: 4 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -3,12 +3,13 @@ include ./apps/devtool/Makefile
include ./apps/docs/Makefile
include ./apps/policy-engine/Makefile
include ./apps/vault/Makefile
include ./packages/armory-e2e-testing/Makefile
include ./packages/armory-sdk/Makefile
include ./packages/nestjs-shared/Makefile
include ./packages/open-telemetry/Makefile
include ./packages/policy-engine-shared/Makefile
include ./packages/transaction-request-intent/Makefile
include ./packages/signature/Makefile
include ./packages/armory-sdk/Makefile
include ./packages/armory-e2e-testing/Makefile
include ./packages/transaction-request-intent/Makefile

# For more terminal color codes, head over to
# https://opensource.com/article/19/9/linux-terminal-colors
Expand Down
6 changes: 3 additions & 3 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ below to generate a project of your choice.

```bash
# Generate an standard JavaScript library.
npx nx g @nrwl/js:lib
npx nx g @nrwl/js:lib
# Generate an NestJS library.
npx nx g @nx/nest:library
# Generate an NestJS application.
Expand Down Expand Up @@ -166,7 +166,7 @@ OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4318
```

> [!NOTE]
> OTEL is disabled by default in development.
> OTEL is disabled by default in development.
3. Restart the application.

Expand All @@ -175,7 +175,7 @@ OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4318
### Naming Conventions

- Traces: `operation.entity.action` (e.g., `policyStore.policies.update`)
- Metrics: `system_entity_unit_state` (e.g., `policy_store_updates_total`)
- Metrics: `system_entity_unit_state` (e.g., `policy_store_updates_total`)

See [Metric Naming](./packages/nestjs-shared/src/lib/module/open-telemetry/service/metric.service.ts)
and [Trace/Span
Expand Down
2 changes: 2 additions & 0 deletions apps/armory/.env.default
Original file line number Diff line number Diff line change
Expand Up @@ -36,4 +36,6 @@ MANAGED_DATASTORE_BASE_URL=http://localhost:3005/v1/data
# OpenTelemetry configuration.
OTEL_SDK_DISABLED=true
OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4318
OTEL_EXPORTER_OTLP_PROTOCOL=http/protobuf
OTEL_LOGS_EXPORTER=otlp
OTEL_RESOURCE_ATTRIBUTES=deployment.environment=local
8 changes: 6 additions & 2 deletions apps/armory/src/main.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
import { instrumentOpenTelemetry } from '@narval/open-telemetry'
import { instrumentTelemetry } from '@narval/open-telemetry'
import { DiagLogLevel } from '@opentelemetry/api'

// IMPORTANT: OpenTelemetry SDK must be registered before any other imports to
// ensure proper instrumentation. The instrumentation packages patches Node.js
// runtime - if NestFactory or other dependencies load first, they'll use the
// unpatched runtime and won't be instrumented correctly.
instrumentOpenTelemetry({ serviceName: 'auth' })
instrumentTelemetry({
serviceName: 'auth',
diagLogLevel: DiagLogLevel.ERROR
})

import { ConfigService } from '@narval/config-module'
import { LoggerService, withApiVersion, withCors, withLogger, withSwagger } from '@narval/nestjs-shared'
Expand Down
2 changes: 2 additions & 0 deletions apps/policy-engine/.env.default
Original file line number Diff line number Diff line change
Expand Up @@ -45,4 +45,6 @@ TSM_PLAYER_COUNT=
# OpenTelemetry configuration.
OTEL_SDK_DISABLED=true
OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4318
OTEL_EXPORTER_OTLP_PROTOCOL=http/protobuf
OTEL_LOGS_EXPORTER=otlp
OTEL_RESOURCE_ATTRIBUTES=deployment.environment=local
8 changes: 6 additions & 2 deletions apps/policy-engine/src/main.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
import { instrumentOpenTelemetry } from '@narval/open-telemetry'
import { instrumentTelemetry } from '@narval/open-telemetry'
import { DiagLogLevel } from '@opentelemetry/api'

// IMPORTANT: OpenTelemetry SDK must be registered before any other imports to
// ensure proper instrumentation. The instrumentation packages patches Node.js
// runtime - if NestFactory or other dependencies load first, they'll use the
// unpatched runtime and won't be instrumented correctly.
instrumentOpenTelemetry({ serviceName: 'policy-engine' })
instrumentTelemetry({
serviceName: 'policy-engine',
diagLogLevel: DiagLogLevel.ERROR
})

import { ConfigService } from '@narval/config-module'
import { LoggerService, withApiVersion, withCors, withLogger, withSwagger } from '@narval/nestjs-shared'
Expand Down
2 changes: 2 additions & 0 deletions apps/vault/.env.default
Original file line number Diff line number Diff line change
Expand Up @@ -38,4 +38,6 @@ BASE_URL=http://localhost:3011
# OpenTelemetry configuration.
OTEL_SDK_DISABLED=true
OTEL_EXPORTER_OTLP_ENDPOINT=http://localhost:4318
OTEL_EXPORTER_OTLP_PROTOCOL=http/protobuf
OTEL_LOGS_EXPORTER=otlp
OTEL_RESOURCE_ATTRIBUTES=deployment.environment=local
8 changes: 6 additions & 2 deletions apps/vault/src/main.ts
Original file line number Diff line number Diff line change
@@ -1,10 +1,14 @@
import { instrumentOpenTelemetry } from '@narval/open-telemetry'
import { instrumentTelemetry } from '@narval/open-telemetry'
import { DiagLogLevel } from '@opentelemetry/api'

// IMPORTANT: OpenTelemetry SDK must be registered before any other imports to
// ensure proper instrumentation. The instrumentation packages patches Node.js
// runtime - if NestFactory or other dependencies load first, they'll use the
// unpatched runtime and won't be instrumented correctly.
instrumentOpenTelemetry({ serviceName: 'vault' })
instrumentTelemetry({
serviceName: 'vault',
diagLogLevel: DiagLogLevel.ERROR
})

import { ConfigService } from '@narval/config-module'
import { LoggerService, withApiVersion, withCors, withLogger, withSwagger } from '@narval/nestjs-shared'
Expand Down
29 changes: 29 additions & 0 deletions packages/open-telemetry/Makefile
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
OPEN_TELEMETRY_PROJECT_NAME := open-telemetry
OPEN_TELEMETRY_PROJECT_DIR := ./packages/open-telemetry

# == Code format ==

open-telemetry/format:
npx nx format:write --projects ${OPEN_TELEMETRY_PROJECT_NAME}

open-telemetry/lint:
npx nx lint ${OPEN_TELEMETRY_PROJECT_NAME} -- --fix

open-telemetry/format/check:
npx nx format:check --projects ${OPEN_TELEMETRY_PROJECT_NAME}

open-telemetry/lint/check:
npx nx lint ${OPEN_TELEMETRY_PROJECT_NAME}

# == Testing ==

open-telemetry/test/type:
npx tsc \
--project ${OPEN_TELEMETRY_PROJECT_DIR}/tsconfig.lib.json \
--noEmit

open-telemetry/test/unit:
npx nx test:unit ${OPEN_TELEMETRY_PROJECT_NAME} -- ${ARGS}

open-telemetry/test/unit/watch:
make open-telemetry/test/unit ARGS=--watch
9 changes: 4 additions & 5 deletions packages/open-telemetry/README.md
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# Open Telemetry
# Open Telemetry

Armory Stack shared Open Telemetry (OTEL) instrumentation configuration.

Expand All @@ -7,9 +7,8 @@ Armory Stack shared Open Telemetry (OTEL) instrumentation configuration.
OTEL registration is kept in a separate package because:

- OTEL modifies Node.js runtime behavior by patching core modules. If
we import any dependencies before registering OTEL, those imports
will use the unpatched runtime and won't be instrumented correctly.
we import any dependencies before registering OTEL, those imports
will use the unpatched runtime and won't be instrumented correctly.

- Having it separate ensures OTELis registered first, before any
other code runs, guaranteeing proper instrumentation of all dependencies.

other code runs, guaranteeing proper instrumentation of all dependencies.
19 changes: 13 additions & 6 deletions packages/open-telemetry/jest.config.ts
Original file line number Diff line number Diff line change
@@ -1,11 +1,18 @@
/* eslint-disable */
export default {
import type { Config } from 'jest'

const config: Config = {
displayName: 'open-telemetry',
moduleFileExtensions: ['ts', 'tsx', 'js', 'jsx'],
preset: '../../jest.preset.js',
testEnvironment: 'node',
transform: {
'^.+\\.[tj]s$': ['ts-jest', { tsconfig: '<rootDir>/tsconfig.spec.json' }]
},
moduleFileExtensions: ['ts', 'js', 'html'],
coverageDirectory: '../../coverage/packages/open-telemetry'
'^.+\\.[tj]sx?$': [
'ts-jest',
{
tsconfig: '<rootDir>/tsconfig.spec.json'
}
]
}
}

export default config
9 changes: 9 additions & 0 deletions packages/open-telemetry/jest.unit.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,9 @@
import type { Config } from 'jest'
import sharedConfig from './jest.config'

const config: Config = {
...sharedConfig,
testMatch: ['<rootDir>/**/__test__/unit/**/*.spec.ts']
}

export default config
11 changes: 9 additions & 2 deletions packages/open-telemetry/project.json
Original file line number Diff line number Diff line change
Expand Up @@ -8,11 +8,18 @@
"lint": {
"executor": "@nx/eslint:lint"
},
"test": {
"test:type": {
"executor": "nx:run-commands",
"options": {
"command": "make open-telemetry/test/type"
}
},
"test:unit": {
"executor": "@nx/jest:jest",
"outputs": ["{workspaceRoot}/coverage/{projectRoot}"],
"options": {
"jestConfig": "packages/open-telemetry/jest.config.ts"
"jestConfig": "packages/open-telemetry/jest.unit.ts",
"verbose": true
}
}
}
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
import { IncomingMessage } from 'http'
import { Socket } from 'net'
import { ignoreIncomingRequestHook } from '../../open-telemetry'

const buildIncomingMessage = (url: string, method = 'GET') => {
const socket = new Socket()
const message = new IncomingMessage(socket)

message.url = url
message.method = method
message.headers = {
host: 'localhost:3000',
'user-agent': 'Mozilla/5.0',
accept: '*/*'
}

return message
}

describe('ignoreIncomingRequestHook', () => {
const rootTestCases = [
buildIncomingMessage('/'),
buildIncomingMessage('/?foo='),
buildIncomingMessage('/?foo=bar'),
buildIncomingMessage('/?'),
buildIncomingMessage('/&')
]

const pingTestCases = [
buildIncomingMessage('/ping'),
buildIncomingMessage('/ping?foo='),
buildIncomingMessage('/ping?foo=bar'),
buildIncomingMessage('/ping?'),
buildIncomingMessage('/ping&')
]

const ignoreTestCases = [...rootTestCases, ...pingTestCases]

for (const request of ignoreTestCases) {
it(`ignores ${request.url}`, () => {
expect(ignoreIncomingRequestHook(request)).toEqual(true)
})
}

const doNotIgnoreTestCases = [
buildIncomingMessage('/foo'),
buildIncomingMessage('/ready'),
buildIncomingMessage('/bar'),
buildIncomingMessage('/client')
]

for (const request of doNotIgnoreTestCases) {
it(`does not ignore ${request.url}`, () => {
expect(ignoreIncomingRequestHook(request)).toEqual(false)
})
}
})
17 changes: 15 additions & 2 deletions packages/open-telemetry/src/lib/open-telemetry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -20,17 +20,30 @@ import { Resource } from '@opentelemetry/resources'
import { PeriodicExportingMetricReader } from '@opentelemetry/sdk-metrics'
import { NodeSDK } from '@opentelemetry/sdk-node'
import { ATTR_SERVICE_NAME } from '@opentelemetry/semantic-conventions'
import { IncomingMessage } from 'http'

type OpenTelemetryOption = {
serviceName: string
diagLogLevel?: DiagLogLevel
instrumentations?: InstrumentationConfigMap
}

export const ignoreIncomingRequestHook = (req: IncomingMessage): boolean => {
const basePath = req.url?.split(/[?&]/)[0] || ''
// These URL paths are part of each NestJS application and used to determine
// if the server is running.
const ignorePaths = ['/', '/ping']

return ignorePaths.includes(basePath)
}

const getInstrumentations = (instrumentations?: InstrumentationConfigMap) => {
return getNodeAutoInstrumentations({
'@opentelemetry/instrumentation-nestjs-core': { enabled: true },
'@opentelemetry/instrumentation-http': { enabled: true },
'@opentelemetry/instrumentation-http': {
enabled: true,
ignoreIncomingRequestHook
},
'@opentelemetry/instrumentation-winston': { enabled: true },
'@opentelemetry/instrumentation-net': { enabled: true },
'@opentelemetry/instrumentation-express': { enabled: true },
Expand Down Expand Up @@ -114,7 +127,7 @@ const registerGracefulShutdownHandler = ({ sdk, event }: { sdk: NodeSDK; event:
})
}

export const instrumentOpenTelemetry = (options: OpenTelemetryOption): void => {
export const instrumentTelemetry = (options: OpenTelemetryOption): void => {
const sdk = buildOpenTelemetrySdk(options)

sdk.start()
Expand Down
Original file line number Diff line number Diff line change
@@ -1,14 +1,7 @@
import { Alg } from '@narval/signature'
import { Hex } from 'viem'
import { decode } from '../../decoders/decode'
import {
ContractRegistry,
InputType,
Intents,
PERMIT2_ADDRESS,
TransactionStatus,
WalletType
} from '../../domain'
import { ContractRegistry, InputType, Intents, PERMIT2_ADDRESS, TransactionStatus, WalletType } from '../../domain'
import { buildContractRegistry, buildTransactionKey, buildTransactionRegistry } from '../../utils'
import {
mockCancelTransaction,
Expand Down
4 changes: 2 additions & 2 deletions packages/transaction-request-intent/src/lib/utils.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,7 @@ import {
toChainAccountId
} from '@narval/policy-engine-shared'
import { SetOptional } from 'type-fest'
import { Address, fromHex, presignMessagePrefix } from 'viem'
import { Address, fromHex } from 'viem'
import {
AssetTypeAndUnknown,
ContractCallInput,
Expand All @@ -37,7 +37,7 @@ import {
WalletType
} from './domain'
import { DecoderError } from './error'
import { Permit, Permit2, SignMessage, SignTypedData } from './intent.types'
import { Permit, Permit2, SignTypedData } from './intent.types'
import { MethodsMapping, SUPPORTED_METHODS, SupportedMethodId } from './supported-methods'
import { isAssetType, isPermit, isPermit2, isSupportedMethodId } from './typeguards'

Expand Down

0 comments on commit 07a433b

Please sign in to comment.