Skip to content

Commit

Permalink
refactor: always return a process resource
Browse files Browse the repository at this point in the history
This commit introduces a slight change in behavior. Previously the
ProcessDectector would validate that it received attributes that it
expected, and returned an empty Resource if it didn't. Based on the
types, and the implementation of the ProcessDetector, there will
always be a valid ProcessResource. Collection of many of the attributes
can be done as a best effort without violating the specification.
  • Loading branch information
mwear committed Feb 15, 2023
1 parent 28a2e4d commit 05a84b8
Show file tree
Hide file tree
Showing 3 changed files with 14 additions and 41 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -21,4 +21,3 @@ export * from './HostDetectorSync';
export * from './OSDetectorSync';
export * from './ProcessDetector';
export * from './ProcessDetectorSync';

Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import * as os from 'os';
* and being instrumented from the NodeJS Process module.
*/
class ProcessDetectorSync implements DetectorSync {
detect(config?: ResourceDetectionConfig): IResource {
detect(_config?: ResourceDetectionConfig): IResource {
const attributes: ResourceAttributes = {
[SemanticResourceAttributes.PROCESS_PID]: process.pid,
[SemanticResourceAttributes.PROCESS_EXECUTABLE_NAME]: process.title,
Expand All @@ -51,38 +51,7 @@ class ProcessDetectorSync implements DetectorSync {
diag.debug(`error obtaining process owner: ${e}`);
}

return this._getResourceAttributes(attributes, config);
}

/**
* Validates process resource attribute map from process variables
*
* @param processResource The unsantized resource attributes from process as key/value pairs.
* @param config: Config
* @returns The sanitized resource attributes.
*/
private _getResourceAttributes(
processResource: ResourceAttributes,
_config?: ResourceDetectionConfig
) {
if (
processResource[SemanticResourceAttributes.PROCESS_EXECUTABLE_NAME] ===
'' ||
processResource[SemanticResourceAttributes.PROCESS_EXECUTABLE_PATH] ===
'' ||
processResource[SemanticResourceAttributes.PROCESS_COMMAND] === '' ||
processResource[SemanticResourceAttributes.PROCESS_COMMAND_LINE] === '' ||
processResource[SemanticResourceAttributes.PROCESS_RUNTIME_VERSION] === ''
) {
diag.debug(
'ProcessDetector failed: Unable to find required process resources. '
);
return Resource.empty();
} else {
return new Resource({
...processResource,
});
}
return new Resource(attributes);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,7 @@
*/
import * as sinon from 'sinon';
import { processDetector, IResource } from '../../../src';
import {
assertResource,
assertEmptyResource,
} from '../../util/resource-assertions';
import { assertResource } from '../../util/resource-assertions';
import { describeNode } from '../../util';
import * as os from 'os';

Expand Down Expand Up @@ -52,11 +49,19 @@ describeNode('processDetector() on Node.js', () => {
runtimeName: 'nodejs',
});
});
it('should return empty resources if title, command and commondLine is missing', async () => {

it('should return a resources if title, command and commondLine are missing', async () => {
sinon.stub(process, 'pid').value(1234);
sinon.stub(process, 'title').value(undefined);
sinon.stub(process, 'title').value('');
sinon.stub(process, 'argv').value([]);
sinon.stub(process, 'versions').value({ node: '1.4.1' });
const resource: IResource = await processDetector.detect();
assertEmptyResource(resource);
// at a minium we should be able to rely on pid runtime, runtime name, and description
assertResource(resource, {
pid: 1234,
version: '1.4.1',
runtimeDescription: 'Node.js',
runtimeName: 'nodejs',
});
});
});

0 comments on commit 05a84b8

Please sign in to comment.