Skip to content

Commit

Permalink
feat: implement secret scrubbing in express instrumentation (#160)
Browse files Browse the repository at this point in the history
  • Loading branch information
Michele Mancioppi authored Feb 16, 2023
1 parent adb4c75 commit 4d5d2ed
Show file tree
Hide file tree
Showing 22 changed files with 299 additions and 309 deletions.
2 changes: 1 addition & 1 deletion jest.component.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,5 +2,5 @@ module.exports = {
testEnvironment: 'node',
testMatch: ['**/component/**/*.test.js'],
roots: ['./test'],
setupFilesAfterEnv: ['./jest.component.setup.js', "jest-json"],
setupFilesAfterEnv: ['./jest.component.setup.js', 'jest-json'],
};
6 changes: 3 additions & 3 deletions jest.component.setup.js
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
require("jest-json");
require("jest-chain");
require('jest-json');
require('jest-chain');

const oldEnv = Object.assign({}, process.env);

Expand All @@ -13,5 +13,5 @@ afterEach(() => {

beforeAll(() => {
global.console = require('console');
require( 'console-stamp' )( global.console )
require('console-stamp')(global.console)
});
6 changes: 3 additions & 3 deletions jest.integration.config.js
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,9 @@ module.exports = {
testEnvironment: 'node',
testMatch: ['**/integration/**/*.test.js'],
roots: ['./test'],
setupFilesAfterEnv: ['./jest.integration.setup.js', "jest-json"],
setupFilesAfterEnv: ['./jest.integration.setup.js', 'jest-json'],
reporters: [
"default",
"jest-summarizing-reporter"
'default',
'jest-summarizing-reporter'
]
};
28 changes: 14 additions & 14 deletions jest.integration.setup.js
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
require("jest-json");
require("jest-chain");
const {instrumentationsVersionManager} = require("./test/helpers/InstrumentationsVersionManager");
const fs = require("fs");
require('jest-json');
require('jest-chain');
const { instrumentationsVersionManager } = require('./test/helpers/InstrumentationsVersionManager');
const fs = require('fs');
const semver = require('semver');

const oldEnv = Object.assign({}, process.env);
Expand All @@ -15,30 +15,30 @@ afterEach(() => {

beforeAll(() => {
global.console = require('console');
require( 'console-stamp' )( global.console )
require('console-stamp')(global.console)
});

afterAll(() => {
console.info("Starting afterAll...");
console.info('Starting afterAll...');
const versions = instrumentationsVersionManager.getInstrumantaionsVersions();
const versions_keys = Object.keys(versions);
if (versions_keys.length){
if (versions_keys.length) {
versions_keys.forEach((lib) => {
// updated supported versions file
const TESTED_VERSIONS_PATH = `./src/instrumentations/${lib}/tested_versions`;
if (!fs.existsSync(TESTED_VERSIONS_PATH)) {
fs.mkdirSync(TESTED_VERSIONS_PATH);
}
const versionStrings = versions[lib].unsupported
.map((v) => `!${v}`)
.concat(versions[lib].supported)
.sort((v1, v2) => semver.compare(v1.replace('!', ''), v2.replace('!', '')))
.join('\n');
.map((v) => `!${v}`)
.concat(versions[lib].supported)
.sort((v1, v2) => semver.compare(v1.replace('!', ''), v2.replace('!', '')))
.join('\n');
fs.writeFileSync(`${TESTED_VERSIONS_PATH}/${lib}`, versionStrings);
console.info("Finish afterAll, supported version files were updated.");
console.info('Finish afterAll, supported version files were updated.');
});
}
else{
console.info("Finish afterAll, no versions to update.");
else {
console.info('Finish afterAll, no versions to update.');
}
});
2 changes: 1 addition & 1 deletion jest.unit.config.js
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
module.exports = {
testEnvironment: 'node',
testMatch: ['**/**/*.test.ts','**/**/*.test.js'],
testMatch: ['**/**/*.test.ts', '**/**/*.test.js'],
roots: ['./src'],
coverageDirectory: './coverage/',
collectCoverage: true,
Expand Down
6 changes: 3 additions & 3 deletions jest.unit.setup.js
Original file line number Diff line number Diff line change
@@ -1,9 +1,9 @@
const oldEnv = Object.assign({}, process.env);

beforeEach(() => {
process.env = { ...oldEnv };
process.env = { ...oldEnv };
});

afterEach(() => {
process.env = { ...oldEnv };
});
process.env = { ...oldEnv };
});
11 changes: 6 additions & 5 deletions src/instrumentations/express/express.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,16 +4,17 @@ import { Span } from '@opentelemetry/api';

import { safeExecute } from '../../utils';
import { InstrumentationIfc } from '../hooksIfc';
import { CommonUtils } from '@lumigo/node-core';

type ExpressRequestType = { req: express.Request; res: express.Response };

export const ExpressHooks: InstrumentationIfc<ExpressRequestType, any> = {
requestHook(span: Span, { req, res }: ExpressRequestType): void {
const oldResEnd = res.end;
const oldResSend = res.send;
if (req.query) span.setAttribute('http.request.query', JSON.stringify(req.query, undefined, 0));
if (req.query) span.setAttribute('http.request.query', CommonUtils.payloadStringify(req.query));
if (req.headers)
span.setAttribute('http.request.headers', JSON.stringify(req.headers, undefined, 0));
span.setAttribute('http.request.headers', CommonUtils.payloadStringify(req.headers));
let response;
res.send = function (data: any) {
response = data;
Expand All @@ -28,12 +29,12 @@ export const ExpressHooks: InstrumentationIfc<ExpressRequestType, any> = {
if (res.getHeaders())
span.setAttribute(
'http.response.headers',
JSON.stringify(res.getHeaders(), undefined, 0)
CommonUtils.payloadStringify(res.getHeaders())
); // TODO This is not compliant with the HTTP semantic conventions
if (response)
span.setAttribute('http.response.body', JSON.stringify(response, undefined, 0));
span.setAttribute('http.response.body', CommonUtils.payloadStringify(response));
if (req.body)
span.setAttribute('http.request.body', JSON.stringify(req.body, undefined, 0));
span.setAttribute('http.request.body', CommonUtils.payloadStringify(req.body));
res.end = oldResEnd;
return origRes;
})();
Expand Down
3 changes: 0 additions & 3 deletions src/instrumentations/https/HttpInstrumentation.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,6 @@ import { HttpInstrumentation } from '@opentelemetry/instrumentation-http';

import { HttpHooks } from './http';
import { Instrumentor } from '../instrumentor';
import { logger } from '../../logging';

export default class LumigoHttpInstrumentation extends Instrumentor<HttpInstrumentation> {
private readonly ignoredHostnames: string[];
Expand Down Expand Up @@ -39,8 +38,6 @@ export default class LumigoHttpInstrumentation extends Instrumentor<HttpInstrume
// Unroutable addresses, used by metadata and credential services on all clouds
/169\.254\.\d+\.\d+.*/gm.test(requestHostname);

logger.debug(`Ignoring request: ${JSON.stringify(request)}`);

return isRequestIgnored;
},
requestHook: HttpHooks.requestHook,
Expand Down
10 changes: 5 additions & 5 deletions test/component/http/http.test.js
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ const {
internalSpanAttributes, expectedClientAttributes
} = require("./httpTestUtils");
const {getSpanByKind} = require("../../testUtils/spanUtils");
const {getAppPort, callContainer, getStartedApp} = require("../../testUtils/utils");
const {getAppPort, callContainer, startTestApp} = require("../../testUtils/utils");

const SPANS_DIR = `${__dirname}/spans`;
const TEST_TIMEOUT = 20_000;
Expand Down Expand Up @@ -50,7 +50,7 @@ describe(`Component compatibility tests for HTTP`, function () {
const fileExporterName = `${SPANS_DIR}/spans-${COMPONENT_NAME}-basic.json`;

// start server
app = getStartedApp(EXEC_SERVER_FOLDER, COMPONENT_NAME, fileExporterName, {LUMIGO_ENDPOINT: "https://walle-edge-app-us-west-2.walle.golumigo.com",
app = startTestApp(EXEC_SERVER_FOLDER, COMPONENT_NAME, fileExporterName, {LUMIGO_ENDPOINT: "https://walle-edge-app-us-west-2.walle.golumigo.com",
LUMIGO_TRACER_TOKEN: 't_123321'});
const port = await getPort(app);

Expand Down Expand Up @@ -125,7 +125,7 @@ describe(`Component compatibility tests for HTTP`, function () {
const fileExporterName = `${SPANS_DIR}/spans-${COMPONENT_NAME}-span-attr.json`;

// start server
app = getStartedApp(EXEC_SERVER_FOLDER, COMPONENT_NAME, fileExporterName, {OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT: "1"});
app = startTestApp(EXEC_SERVER_FOLDER, COMPONENT_NAME, fileExporterName, {OTEL_SPAN_ATTRIBUTE_VALUE_LENGTH_LIMIT: "1"});
const port = await getPort(app);

const waited = new Promise((resolve, reject) => {
Expand Down Expand Up @@ -209,7 +209,7 @@ describe(`Component compatibility tests for HTTP`, function () {
const fileExporterName = `${SPANS_DIR}/spans-${COMPONENT_NAME}-otel-attr.json`;

// start server
app = getStartedApp(EXEC_SERVER_FOLDER, COMPONENT_NAME, fileExporterName, {OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT: "3"});
app = startTestApp(EXEC_SERVER_FOLDER, COMPONENT_NAME, fileExporterName, {OTEL_ATTRIBUTE_VALUE_LENGTH_LIMIT: "3"});
const port = await getPort(app);

const waited = new Promise((resolve, reject) => {
Expand Down Expand Up @@ -292,7 +292,7 @@ describe(`Component compatibility tests for HTTP`, function () {
const fileExporterName = `${SPANS_DIR}/spans-${COMPONENT_NAME}-default.json`;

// start server
app = getStartedApp(EXEC_SERVER_FOLDER, COMPONENT_NAME, fileExporterName);
app = startTestApp(EXEC_SERVER_FOLDER, COMPONENT_NAME, fileExporterName);
const port = await getPort(app);

const waited = new Promise((resolve, reject) => {
Expand Down
1 change: 1 addition & 0 deletions test/integration/express/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
spans
1 change: 1 addition & 0 deletions test/integration/express/app/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
package-lock.json
15 changes: 8 additions & 7 deletions test/integration/express/app/app.js
Original file line number Diff line number Diff line change
Expand Up @@ -10,17 +10,18 @@ app.use(bodyParser.urlencoded({ extended: false }));
// parse application/json
app.use(bodyParser.json());

app.get('/test-scrubbing', async (req, res) => {
res.send({
"Authorization": "SECRET"
}, 200);
});

app.get('/', async (req, res) => {
res.send("server is ready").status(200);
});

app.get('/invoke-requests', async (req, res) => {
const response = await axios.get('https://api.chucknorris.io/jokes/categories', {
headers: {
header: 'a',
},
});
res.send(response.data).status(200);
app.get('/basic', async (req, res) => {
res.send('Hello world').status(200);
});

const server = app.listen(0, () => {
Expand Down
21 changes: 1 addition & 20 deletions test/integration/express/app/installDependencies.js
Original file line number Diff line number Diff line change
@@ -1,20 +1 @@
const fs = require('fs');
const { spawnSync } = require('child_process');
const {writeDependencyVersionsFile, installDependencyInTempLocation, moveDependencyFromTempToActiveModule} = require("../../commonInstallDependencies");

console.log(`Installing supported dependency versions...`);

let packageJson = require('./package.json');
let { supportedDependencies } = packageJson.lumigo;

supportedDependencies = writeDependencyVersionsFile(supportedDependencies);

spawnSync('npm', ['install']);

fs.mkdirSync('./node_modules/.tmp', { recursive: true });

for (const dependency in supportedDependencies) {
installDependencyInTempLocation(supportedDependencies, dependency);
moveDependencyFromTempToActiveModule(supportedDependencies, dependency);
}
console.log(`Supported dependency versions installed successfully.`);
// No longer needed!
10 changes: 2 additions & 8 deletions test/integration/express/app/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,13 +10,7 @@
"license": "ISC",
"dependencies": {
"@lumigo/opentelemetry": "file:../../../../wrapper.tgz",
"body-parser": "^1.19.1"
},
"lumigo": {
"supportedDependencies": {
"express": {
"satisfies": "^4.9.0"
}
}
"body-parser": "^1.19.1",
"express": "^4.9.0"
}
}
Loading

0 comments on commit 4d5d2ed

Please sign in to comment.