diff --git a/.github/dependabot.yml b/.github/dependabot.yml index b9b15ac9b18..fddfcb60d0a 100644 --- a/.github/dependabot.yml +++ b/.github/dependabot.yml @@ -79,7 +79,8 @@ updates: - "@datadog/wasm-js-rewriter" - "@opentelemetry/api" - package-ecosystem: "npm" - directory: "/packages/dd-trace/test/plugins/versions" + directories: + - "/packages/dd-trace/test/plugins/versions" schedule: interval: "daily" open-pull-requests-limit: 1 @@ -92,3 +93,22 @@ updates: test-versions: patterns: - "*" + - package-ecosystem: "npm" + directories: + - "/integration-tests/esbuild" + schedule: + interval: "daily" + open-pull-requests-limit: 1 + labels: + - dependabot + - dependencies + - javascript + - semver-patch + ignore: + - dependency-name: "express" + # Update express manually for now due to esbuild breaking otherwise + update-types: ["version-update:semver-major"] + groups: + test-versions: + patterns: + - "*" diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index f51e47d2f82..ac8620e411d 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -5,15 +5,15 @@ stages: - single-step-instrumentation-tests - macrobenchmarks -check_config_inversion_local_file: +validate_supported_configurations_local_file: rules: - when: on_success - extends: .check_config_inversion_local_file + extends: .validate_supported_configurations_local_file variables: LOCAL_JSON_PATH: "packages/dd-trace/src/supported-configurations.json" -config-inversion-update-supported-range: - extends: .config_inversion_update_central +update_central_configurations_version_range: + extends: .update_central_configurations_version_range variables: LOCAL_REPO_NAME: "dd-trace-js" LOCAL_JSON_PATH: "packages/dd-trace/src/supported-configurations.json" diff --git a/.gitlab/one-pipeline.locked.yml b/.gitlab/one-pipeline.locked.yml index b8554f3ae89..a385ff771db 100644 --- a/.gitlab/one-pipeline.locked.yml +++ b/.gitlab/one-pipeline.locked.yml @@ -1,4 +1,4 @@ # DO NOT EDIT THIS FILE MANUALLY # This file is auto-generated by automation. include: - - remote: https://gitlab-templates.ddbuild.io/libdatadog/one-pipeline/ca/50d49f6898ce86e93326856210e8ab6526895273cb6341ac2d7d0e6c1c14e31e/one-pipeline.yml + - remote: https://gitlab-templates.ddbuild.io/libdatadog/one-pipeline/ca/d5116fedde5478649c2f0cc00457f0f658d157e1bd8b3a7468bcaf5972c442a4/one-pipeline.yml diff --git a/integration-tests/esbuild/package.json b/integration-tests/esbuild/package.json index 3c9c8a9e157..428c4a9d933 100644 --- a/integration-tests/esbuild/package.json +++ b/integration-tests/esbuild/package.json @@ -20,14 +20,14 @@ "author": "Thomas Hunter II ", "license": "ISC", "dependencies": { - "@apollo/server": "*", - "@koa/router": "*", - "aws-sdk": "*", - "axios": "*", - "esbuild": "*", - "express": "^4.16.2", - "knex": "*", - "koa": "*", - "openai": "*" + "@apollo/server": "5.0.0", + "@koa/router": "14.0.0", + "aws-sdk": "2.1692.0", + "axios": "1.11.0", + "esbuild": "0.25.9", + "express": "4.21.2", + "knex": "3.1.0", + "koa": "3.0.1", + "openai": "5.12.2" } } diff --git a/integration-tests/log_injection.spec.js b/integration-tests/log_injection.spec.js new file mode 100644 index 00000000000..5553e00bea7 --- /dev/null +++ b/integration-tests/log_injection.spec.js @@ -0,0 +1,66 @@ +'use strict' + +const { FakeAgent, createSandbox, spawnProc, curlAndAssertMessage, assertObjectContains } = require('./helpers') +const path = require('path') +const { USER_KEEP } = require('../ext/priority') + +describe('Log Injection', () => { + let agent + let proc + let sandbox + let cwd + let app + let env + + before(async () => { + sandbox = await createSandbox(['express', 'winston']) + cwd = sandbox.folder + app = path.join(cwd, 'log_injection/index.js') + }) + + after(async () => { + await sandbox.remove() + }) + + beforeEach(async () => { + agent = await new FakeAgent().start() + }) + + describe('log injection with rule based sampling', () => { + beforeEach(async () => { + agent = await new FakeAgent().start() + }) + + afterEach(async () => { + proc.kill() + await agent.stop() + }) + + it('should correctly apply rule based sampling when log injection is enabled', async () => { + env = { + AGENT_PORT: agent.port, + lOG_INJECTION: 'true' + } + proc = await spawnProc(app, { cwd, env, execArgv: [] }) + const url = proc.url + '/sampled' + return curlAndAssertMessage(agent, url, ({ headers, payload }) => { + // Bug: previously got USER_REJECT instead of USER_KEEP when log injection is enabled, + // meaning resource rules are not applied & instead global sampling is applied + // Now gets USER_KEEP because resource rules are applied + assertObjectContains(payload, [[{ metrics: { _sampling_priority_v1: USER_KEEP } }]]) + }, 20000, 1) + }) + + it('should correctly apply rule based sampling when log injection is disabled', async () => { + env = { + AGENT_PORT: agent.port, + lOG_INJECTION: 'false' + } + proc = await spawnProc(app, { cwd, env, execArgv: [] }) + const url = proc.url + '/sampled' + return curlAndAssertMessage(agent, url, ({ headers, payload }) => { + assertObjectContains(payload, [[{ metrics: { _sampling_priority_v1: USER_KEEP } }]]) + }, 20000, 1) + }) + }) +}) diff --git a/integration-tests/log_injection/index.js b/integration-tests/log_injection/index.js new file mode 100644 index 00000000000..d9b7b80cb89 --- /dev/null +++ b/integration-tests/log_injection/index.js @@ -0,0 +1,50 @@ +'use strict' + +const options = { + service: 'test-service', + sampleRate: 0.0, + samplingRules: [ + { + resource: 'GET /sampled', + sampleRate: 1.0 + } + ] +} + +if (process.env.AGENT_PORT) { + options.port = process.env.AGENT_PORT +} + +if (process.env.lOG_INJECTION) { + options.logInjection = process.env.lOG_INJECTION +} + +const tracer = require('dd-trace') +tracer.init(options) + +const express = require('express') +const winston = require('winston') + +const app = express() + +// Create winston logger +const logger = winston.createLogger({ + level: 'info', + format: winston.format.json(), + transports: [ + new winston.transports.Console({ silent: true }) + ] +}) + +// Route WITH logging (demonstrates the bug) +app.get('/sampled', (req, res) => { + // BUG: This winston.info() triggers log injection BEFORE resource.name is set + // which causes sampling decision to happen too early, bypassing the resource rule + logger.info('Processing GET /sampled request') + res.json({ message: 'logged request' }) +}) + +const server = app.listen(0, () => { + const port = server.address().port + process.send({ port }) +}) diff --git a/package.json b/package.json index 857aaf9267d..0b437c4a686 100644 --- a/package.json +++ b/package.json @@ -1,6 +1,6 @@ { "name": "dd-trace", - "version": "5.63.3", + "version": "5.63.4", "description": "Datadog APM tracing client for JavaScript", "main": "index.js", "typings": "index.d.ts", diff --git a/packages/dd-trace/src/opentracing/tracer.js b/packages/dd-trace/src/opentracing/tracer.js index 87f96d9b45a..9fb8a467a80 100644 --- a/packages/dd-trace/src/opentracing/tracer.js +++ b/packages/dd-trace/src/opentracing/tracer.js @@ -86,7 +86,7 @@ class DatadogTracer { } try { - if (format !== 'text_map_dsm') { + if (format !== 'text_map_dsm' && format !== formats.LOG) { this._prioritySampler.sample(context) } this._propagators[format].inject(context, carrier) diff --git a/packages/dd-trace/test/appsec/next/app-dir/next.config.js b/packages/dd-trace/test/appsec/next/app-dir/next.config.js index 40ee170113f..20f24204bfc 100644 --- a/packages/dd-trace/test/appsec/next/app-dir/next.config.js +++ b/packages/dd-trace/test/appsec/next/app-dir/next.config.js @@ -6,7 +6,8 @@ const nextConfig = { experimental: { appDir: true }, - output: 'standalone' + output: 'standalone', + outputFileTracingRoot: __dirname } module.exports = nextConfig diff --git a/packages/dd-trace/test/opentracing/tracer.spec.js b/packages/dd-trace/test/opentracing/tracer.spec.js index 31e3df79a33..e580e20c16e 100644 --- a/packages/dd-trace/test/opentracing/tracer.spec.js +++ b/packages/dd-trace/test/opentracing/tracer.spec.js @@ -5,6 +5,7 @@ require('../setup/tap') const opentracing = require('opentracing') const os = require('os') const SpanContext = require('../../src/opentracing/span_context') +const formats = require('../../../../ext/formats') const Reference = opentracing.Reference describe('Tracer', () => { @@ -25,6 +26,7 @@ describe('Tracer', () => { let TextMapPropagator let HttpPropagator let BinaryPropagator + let LogPropagator let propagator let config let log @@ -58,6 +60,7 @@ describe('Tracer', () => { TextMapPropagator = sinon.stub() HttpPropagator = sinon.stub() BinaryPropagator = sinon.stub() + LogPropagator = sinon.stub() propagator = { inject: sinon.stub(), extract: sinon.stub() @@ -90,6 +93,7 @@ describe('Tracer', () => { './propagation/text_map': TextMapPropagator, './propagation/http': HttpPropagator, './propagation/binary': BinaryPropagator, + './propagation/log': LogPropagator, '../log': log, '../exporter': exporter }) @@ -368,6 +372,16 @@ describe('Tracer', () => { expect(prioritySampler.sample).to.have.been.calledWith(spanContext) }) + + it('should not generate sampling priority for log injection', () => { + LogPropagator.returns(propagator) + + tracer = new Tracer(config) + tracer.inject(spanContext, formats.LOG, carrier) + + expect(prioritySampler.sample).to.not.have.been.called + expect(propagator.inject).to.have.been.calledWith(spanContext, carrier) + }) }) describe('extract', () => {