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

[OGUI-1455] Logging live mode filters/criteria #2684

Open
wants to merge 6 commits into
base: dev
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 3 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
108 changes: 108 additions & 0 deletions Framework/Backend/test/mocha-ws.js
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,108 @@ process.env.NODE_TLS_REJECT_UNAUTHORIZED = '0';

let http, ws, tokenService, token; // eslint-disable-line

const filters = {
timestamp: {
since: '13:02:30',
until: '13:02:40',
$since: '2024-12-02T12:02:30.000Z',
$until: '2024-12-02T12:02:40.000Z',
},
hostname: {
match: 'aldaqecs01-v1',
exclude: '',
$match: 'aldaqecs01-v1',
$exclude: null,
},
rolename: {
match: '',
exclude: '',
$match: null,
$exclude: null,
},
pid: {
match: '50990',
exclude: '',
$match: '50990',
$exclude: null,
},
username: {
match: 'alicedaq',
exclude: '',
$match: 'alicedaq',
$exclude: null,
},
system: {
match: 'DAQ',
exclude: '',
$match: 'DAQ',
$exclude: null,
},
facility: {
match: 'runControl',
exclude: '',
$match: 'runControl',
$exclude: null,
},
detector: {
match: 'TPC',
exclude: '',
$match: 'TPC',
$exclude: null,
},
partition: {
match: '',
exclude: '',
$match: null,
$exclude: null,
},
run: {
match: '248023',
exclude: '',
$match: '248023',
$exclude: null,
},
errcode: {
match: '',
exclude: '',
$match: null,
$exclude: null,
},
errline: {
match: '',
exclude: '',
$match: null,
$exclude: null,
},
errsource: {
match: '',
exclude: '',
$match: null,
$exclude: null,
},
message: {
match: '',
exclude: '',
$match: null,
$exclude: null,
},
severity: {
in: 'I F',
$in: [
'I',
'F',
],
},
level: {
max: null,
$max: null,
},
};

const minifiedFilters = '{"timestamp":{"since":"13:02:30","until":"13:02:40"},"hostname":{"match":"aldaqecs01-v1"},' +
'"pid":{"match":"50990"},"username":{"match":"alicedaq"},"system":{"match":"DAQ"},"facility":{"match":"runControl"},' +
'"detector":{"match":"TPC"},"run":{"match":"248023"},"severity":{"in":"I F"}}';

describe('websocket', () => {
before(() => {
tokenService = new O2TokenService(config.jwt);
Expand Down Expand Up @@ -130,6 +232,12 @@ describe('websocket', () => {
});
});

it('minifyCriteria() works as expected', (done) => {
const criterias = ws.minifyCriteria(filters);
assert.strictEqual(JSON.stringify(criterias), minifiedFilters);
done();
});

it('Request message broadcast with 200', (done) => {
const connection = new WebSocketClient(`ws://localhost:${config.http.port}/?token=${token}`);

Expand Down
40 changes: 40 additions & 0 deletions Framework/Backend/websocket/server.js
Original file line number Diff line number Diff line change
Expand Up @@ -133,6 +133,42 @@
client.on('error', (err) => this.logger.error(`Connection ${err.code}`));
}

/**
* Make criteria more readable.
* This code is a close copy of InfoLogger/public/logFilter/LogFilter.js LN 101 toObject()
* @param {object} criteria - criteria to be minified
* @returns {object} minimal filter object
*/
minifyCriteria(criteria) {
Copy link
Member

Choose a reason for hiding this comment

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

@Houwie7000 as this is a utility function, I think it is better if we move it to an independent JS module in an utility folder.
As currently there is no such folder, please create one and add this function to its own module. Then call it from there in the WS server.
The tests associated to this function should match the folder structure as well

// Copy everything
const criterias = JSON.parse(JSON.stringify(criteria));
Copy link
Member

Choose a reason for hiding this comment

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

Currently this function minifyCriteria is called form the WS server directly and there is no try-catch associated to it which means if the criteria is in a bad format it will crash the server.
I would recommend you add this function in a try catch and log in case of error that the payload criteria is not a correct format. We should not log the payload (as it is user input) as it might contain an injection attack


// Clean-up the whole structure

for (const field in criterias) {
for (const operator in criterias[field]) {
// Remote parsed properties (generated with fromJSON)
if (operator.includes('$')) {
delete criterias[field][operator];
}

// Remote empty inputs
if (!criterias[field][operator]) {
delete criterias[field][operator];
} else if (operator === 'match' || operator === 'exclude') {
// Encode potential breaking characters and escape double quotes as are used by browser by default
criterias[field][operator] = encodeURI(criterias[field][operator].replace(/["]+/g, '\\"'));
}

// Remove empty fields
if (!Object.keys(criterias[field]).length) {
delete criterias[field];
}
}
}
return criterias;
}

/**
* Called when a new message arrives
* Handles connection with a client
Expand All @@ -146,6 +182,10 @@
// 2. Check if its message filter (no auth required)
if (parsed.getCommand() == 'filter' && parsed.getPayload()) {
client.filter = new Function(`return ${parsed.getPayload()}`)();
const criterias = this.minifyCriteria(client.filter(message, true));
if (criterias != false) {
this.logger.debugMessage(`New live filter applied: ${JSON.stringify(criterias)}`);

Check warning on line 187 in Framework/Backend/websocket/server.js

View check run for this annotation

Codecov / codecov/patch

Framework/Backend/websocket/server.js#L185-L187

Added lines #L185 - L187 were not covered by tests
}
}
// 3. Get reply if callback exists
this.processRequest(parsed)
Expand Down
7 changes: 6 additions & 1 deletion InfoLogger/public/logFilter/LogFilter.js
Original file line number Diff line number Diff line change
Expand Up @@ -152,12 +152,17 @@ export default class LogFilter extends Observable {
* This function will be stringified then sent to server so it can filter logs
* 'DATA_PLACEHOLDER' will be replaced by the stringified filters too so the function contains de data
* @param {WebSocketMessage} message - message to be filtered
* @param {boolean} returnCriteriasOnly - Only return the filterlog criteria.
* @returns {boolean} true if message passes criterias
*/
function filterFunction(message) {
function filterFunction(message, returnCriteriasOnly = false) {
const log = message.payload;
const criterias = 'DATA_PLACEHOLDER';

if (returnCriteriasOnly) {
return criterias;
}

/**
* Transform timestamp of infologger into javascript Date object
* @param {number} timestamp - timestamp from infologger
Expand Down
Loading