Skip to content

fix: updated otel deps and iitm - #2679

Open
abhilash-sivan wants to merge 13 commits into
mainfrom
fix-tedious-esm
Open

fix: updated otel deps and iitm#2679
abhilash-sivan wants to merge 13 commits into
mainfrom
fix-tedious-esm

Conversation

@abhilash-sivan

@abhilash-sivan abhilash-sivan commented Jul 27, 2026

Copy link
Copy Markdown
Contributor

refs https://jsw.ibm.com/browse/INSTA-97288

Summary

  1. Updated @opentelemetry/instrumentation-tedious from 0.28.0 to 0.40.0.
  2. Updated import-in-the-middle from 2.0.5 to 3.3.2 to maintain compatibility with the newer OpenTelemetry instrumentation.
  3. Updated @instana/instrumentation-confluent-kafka-javascript to support iitm

Reason

@opentelemetry/instrumentation-tedious@0.40.0 depends on a newer @opentelemetry/instrumentation version that requires import-in-the-middle@^3. Keeping import-in-the-middle@2.x caused npm to install multiple IITM versions, which broke ESM module interception and prevented Tedious instrumentation from patching correctly.

Fix

Pinned import-in-the-middle to 3.3.2, ensuring all instrumentation uses a single shared IITM instance, restoring ESM tracing while remaining compatible with the updated OpenTelemetry instrumentation.

PR body

  • bumped @opentelemetry/instrumentation-tedious from 0.28.0 to 0.40.0
  • bumped import-in-the-middle from 2.0.5 to 3.3.2
  • bumped @instana/instrumentation-confluent-kafka-javascript from 1.0.1 to 1.1.0

@abhilash-sivan
abhilash-sivan force-pushed the fix-tedious-esm branch 2 times, most recently from 6dafe24 to b5cbce4 Compare July 31, 2026 11:36
expect(span.data.tags['db.user']).to.eql('admin@instana@nodejs-team-db-server');
expect(span.data.tags['db.statement']).to.eql(expectedStatement);
expect(span.data.tags['net.peer.name']).to.eql('nodejs-team-db-server.database.windows.net');
expect(span.data.tags['db.system.name']).to.eql('microsoft.sql_server');

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

changed assertions to adapt new semconv

The stable OpenTelemetry semantic conventions (v1.33.0+) are supported starting with @opentelemetry/instrumentation-tedious v0.39.0. See the Semantic Conventions section

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

And our BE supports this version already, yeah?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Yeah, BE is updated to 1.46 already. Also these are tags, shouldn't be an issue right ?

@kirrg001

kirrg001 commented Aug 3, 2026

Copy link
Copy Markdown
Contributor

The problem is that Otel with ESM requires users to manually register the IITM hook.

See:
https://github.com/open-telemetry/opentelemetry-js/blob/main/doc/esm-support.md

That means, our esm-register.mjs file automatically takes care of that requirement. We need to a proper explanation to our integration + our esm register.

If we just update IITM in core with v3, we can still run into issues with Otel instrumentations who use an older instrumentations dependency. But the Otel ecosystem suffers from the same problem.

As soon as you have one Otel instrumentation which is still on the old instrumentation version, it won't work for the customer OR the instrumentation dependency got already deduped to the root (which is for example not the case for our setup - its still on 207).

Right now with the Tedious 0.40 update, all our Otel dependencies use the newer instrumentation dependency already.
So we will have problems with Customer codebases and custom Otel installations and if we allow custom otel dependencies.

As soon the customer has any slightly different installation tree and IITM v2 is on the root, the Otel instrumentations would no longer work because the Otel instrumentation dependency loads IITM as a dependency and would load v2 from the root. But we instantiate v3.

Similar to our setup: as soon as we have an Otel instrumentation which needs IITM v2, it does not work anymore.

Can you please add proper explanations & tests which break the solution? The test can just be a simple reproduce script. For now we can only update to v3 + ^.

Long-term: we may need to figure out if multiple IITM versions are being used (similar to the Otel API fix)

@abhilash-sivan

Copy link
Copy Markdown
Contributor Author

The problem is that Otel with ESM requires users to manually register the IITM hook.

See: https://github.com/open-telemetry/opentelemetry-js/blob/main/doc/esm-support.md

That means, our esm-register.mjs file automatically takes care of that requirement. We need to a proper explanation to our integration + our esm register.

If we just update IITM in core with v3, we can still run into issues with Otel instrumentations who use an older instrumentations dependency. But the Otel ecosystem suffers from the same problem.

As soon as you have one Otel instrumentation which is still on the old instrumentation version, it won't work for the customer OR the instrumentation dependency got already deduped to the root (which is for example not the case for our setup - its still on 207).

Right now with the Tedious 0.40 update, all our Otel dependencies use the newer instrumentation dependency already. So we will have problems with Customer codebases and custom Otel installations and if we allow custom otel dependencies.

As soon the customer has any slightly different installation tree and IITM v2 is on the root, the Otel instrumentations would no longer work because the Otel instrumentation dependency loads IITM as a dependency and would load v2 from the root. But we instantiate v3.

Similar to our setup: as soon as we have an Otel instrumentation which needs IITM v2, it does not work anymore.

Can you please add proper explanations & tests which break the solution? The test can just be a simple reproduce script. For now we can only update to v3 + ^.

Long-term: we may need to figure out if multiple IITM versions are being used (similar to the Otel API fix)

I added a warning comment to IITM loader
https://github.com/instana/nodejs/pull/2679/changes#diff-af165377f863b6d4ad356ba4927e9b67ada30e8c4c531b4aee8c6db5c18c8e91R44

Can be deferred to debug or maybe remove entirely.

I also added a supporting test where multiple instantiations logs this warning

@kirrg001

kirrg001 commented Aug 4, 2026

Copy link
Copy Markdown
Contributor

Ty. Will check out asap

I think we have to close #2690 and merge the update both deps updates together right?

Comment thread packages/collector/esm-register.mjs Outdated
Comment thread packages/collector/esm-register.mjs Outdated
Comment thread packages/collector/esm-register.mjs Outdated
Comment thread packages/collector/esm-register.mjs Outdated
@abhilash-sivan abhilash-sivan changed the title fix: bumped @opentelemetry/instrumentation-tedious from 0.28.0 to 0.40.0 fix: updated otel deps and iitm Aug 4, 2026
/**
* OpenTelemetry Integration correlation
*
* Our OpenTelemetry integration works because we load the IITM hook for ESM here. See https://github.com/open-telemetry/opentelemetry-js/blob/main/doc/esm-support.md.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Suggested change
* Our OpenTelemetry integration works because we load the IITM hook for ESM here. See https://github.com/open-telemetry/opentelemetry-js/blob/main/doc/esm-support.md.
* Our OpenTelemetry integration works because we load the IITM hook for ESM here. See https://github.com/open-telemetry/opentelemetry-js/blob/main/doc/esm-support.md.
* > register('@instana/core/iitm-loader.mjs', import.meta.url);
* This line adds support for Instana instrumentations and our Otel integration instrumentations working.

Comment thread packages/collector/test/integration/currencies/databases/tedious/tedious-app.js Outdated
@abhilash-sivan
abhilash-sivan marked this pull request as ready for review August 4, 2026 10:48
@abhilash-sivan
abhilash-sivan requested a review from a team as a code owner August 4, 2026 10:48
'will not fire when another instance dispatches module load events — ' +
'OpenTelemetry instrumentations are likely broken. ' +
'To fix this, align all IITM dependencies to the same version. ' +
`Detected instances:\n${iitmInstances.map(p => ` - ${p}`).join('\n')}`

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Do you have a sample log output showing how this warning appears? I'd like to see what it looks like in customer logs.


// Real lib/register.js entries that may be in require.cache when the full suite runs.
// We snapshot them before each test so we have full control over what the detector sees.
let snapshotKeys;

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Could you please remove the unnecessary comments from the test file and keep only the ones that are essential for understanding the test logic?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants