Skip to content

test: corrected and clarified bull test cases #2002

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.

Sign up for GitHub

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

Merged
merged 4 commits into from
Sep 17, 2025
Merged

test: corrected and clarified bull test cases #2002

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
0d0aece
test: corrected bull test cases
kirrg001 Sep 16, 2025
2e34cb9
chore: fixes
kirrg001 Sep 16, 2025
e43f1b2
chore: cleanup
kirrg001 Sep 16, 2025
1dd0ded
test: stop agent
kirrg001 Sep 17, 2025
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
3 changes: 3 additions & 0 deletions packages/collector/src/immediate.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,12 +30,15 @@ const isExcludedFromInstrumentation = coreUtil.excludedFromInstrumentation && co
// In case this is a child process of an instrumented parent process we might receive the agent uuid from the parent
// process to be able to produce and collect spans immediately without waiting for a connection to the agent in this
// process.
// TODO: This does not work because you would report spans with parent agent uuid and the child process pid -
// this is not compatible. Our codebase does not support this.
const parentProcessAgentUuid = process.env.INSTANA_AGENT_UUID;

if (!isExcludedFromInstrumentation) {
if (parentProcessAgentUuid) {
// @ts-ignore - Type 'string' is not assignable to type 'undefined'
// Probably because exports.agentUuid is set to undefined and export values were not supposed to be changed
// TODO: This has no effect. Remove! See comment above.
agentOpts.agentUuid = parentProcessAgentUuid;
Copy link
Contributor Author

@kirrg001 kirrg001 Sep 16, 2025

Choose a reason for hiding this comment

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

If you comment out this line, the tests still work.
The corrected test logic proofs that this is not used.

Copy link
Contributor Author

@kirrg001 kirrg001 Sep 16, 2025

Choose a reason for hiding this comment

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

Another PR will come to remove it and to improve the logic in our codebase for this case.

Copy link
Contributor Author

@kirrg001 kirrg001 Sep 16, 2025

Choose a reason for hiding this comment

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

I was refactoring the codebase for util, config and logging and these agentOpts file was in my way and then I digged into this, because its very dirty code.

require('./index')({
tracing: {
Expand Down
12 changes: 11 additions & 1 deletion packages/collector/test/apps/agentStub.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@ if (process.env.INSTANA_DEBUG === 'true') {

// NOTE: we can leave the hardcoded port here as this file is not used in the test env!
const port = process.env.AGENT_PORT || 42699;
const uniqueAgentUuids = process.env.AGENT_UNIQUE_UUIDS === 'true';
const extraHeaders = process.env.EXTRA_HEADERS ? process.env.EXTRA_HEADERS.split(',') : [];
const secretsMatcher = process.env.SECRETS_MATCHER ? process.env.SECRETS_MATCHER : 'contains-ignore-case';
const secretsList = process.env.SECRETS_LIST ? process.env.SECRETS_LIST.split(',') : ['pass', 'secret', 'token'];
Expand All @@ -44,6 +45,7 @@ const kafkaTraceCorrelation = process.env.KAFKA_TRACE_CORRELATION
const ignoreEndpoints = process.env.IGNORE_ENDPOINTS && JSON.parse(process.env.IGNORE_ENDPOINTS);
const disable = process.env.AGENT_DISABLE_TRACING && JSON.parse(process.env.AGENT_DISABLE_TRACING);

const uuids = {};
let discoveries = {};
let rejectAnnounceAttempts = 0;
let requests = {};
Expand Down Expand Up @@ -81,8 +83,16 @@ app.put('/com.instana.plugin.nodejs.discovery', (req, res) => {
}
logger.debug('New discovery %s with params', pid, req.body);

let uuid;

if (uniqueAgentUuids) {
uuid = uuids[pid] = `agent-stub-uuid-${pid}`;
} else {
uuid = 'agent-stub-uuid';
Copy link
Contributor Author

@kirrg001 kirrg001 Sep 16, 2025

Choose a reason for hiding this comment

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

The bull tests were false positives.
The agent stub always returned "agent-stub-uuid" for ANY registered process.

The test asserted that the spans from the bull child process (bull forks a process for jobs), have this uuid for span.f.h (agentOpts.agentUuid). And this agentUuid is set to its parent process uuid (see immediate) -> but any process uses the hardcoded UUID from the agent stub "agent-stub-uuid"! So you don't know if its the actual parent uuid or the child uuid.

Each process gets now its own uuid.
I have updated the tests to proof that the INSTANA_AGENT_UUID has no effect and is not used.

}

const response = {
agentUuid: 'agent-stub-uuid',
agentUuid: uuid,
pid,
extraHeaders,
secrets: {
Expand Down
2 changes: 2 additions & 0 deletions packages/collector/test/apps/agentStubControls.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ class AgentStubControls {
env.SECRETS_MATCHER = opts.secretsMatcher || 'contains-ignore-case';
env.SECRETS_LIST = (opts.secretsList || []).join(',');
env.SECRETS_LIST = (opts.secretsList || []).join(',');
env.AGENT_UNIQUE_UUIDS = opts.uniqueAgentUuids === true;

if (opts.rejectTraces) {
env.REJECT_TRACES = 'true';
}
Expand Down
7 changes: 5 additions & 2 deletions packages/collector/test/tracing/messaging/bull/sender.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,8 +66,11 @@ app.post('/send', (request, response) => {

if (repeat && !bulk) {
options.repeat = {
every: 50,
limit: 2
// NOTE: DO not use a very small value here (e.g. 50ms), because bull is too smart
// and creates two processes
// instead of one, because the diff is too small. We don't want that for repeat!
every: 1000, // repeat every second
limit: 2 // two repeats
};
}

Expand Down
2 changes: 1 addition & 1 deletion packages/collector/test/tracing/messaging/bull/sender.mjs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,7 +68,7 @@ app.post('/send', (request, response) => {

if (repeat && !bulk) {
options.repeat = {
every: 50,
every: 1000,
limit: 2
};
}
Expand Down
Loading