feat: added support for @confluentinc/kafka-javascript #2221
Conversation
kirrg001
force-pushed
the
feat-otel-confl-kaf
branch
from
b494aa5 to
702163b
Compare
refs #2221 - we had no esm app for the Otel integration - proofed that it works
kirrg001
force-pushed
the
feat-otel-confl-kaf
branch
4 times, most recently
from
50edd3c to
f955561
Compare
kirrg001
force-pushed
the
feat-otel-confl-kaf
branch
from
11640e5 to
81080d1
Compare
refs #2221 - global agent defined once - some more helpful logs - oracledb needs sometimes longer on CI to connect to the sidecar - better cleanup
kirrg001
force-pushed
the
feat-otel-confl-kaf
branch
from
81080d1 to
00d4ab3
Compare
refs https://jsw.ibm.com/browse/INSTA-66145 - added a single place to prepare the otel data - extracted from feat: added support for @confluentinc/kafka-javascript #2221 to improve readability on feat: added support for @confluentinc/kafka-javascript #2221 - reordered and regrouped code - no functional changes!
kirrg001
force-pushed
the
feat-otel-confl-kaf
branch
from
00d4ab3 to
af6e9ae
Compare
| * | ||
| * Trace correlation does not work with socket.io, because they do not support | ||
| * headers or meta data, only payload. | ||
| * headers or meta data, only payload! |
There was a problem hiding this comment.
We did not had support yet for Exit/Entry or Entry/Exit correlation in our integration.
Will re-investigate socket.io via https://jsw.ibm.com/browse/INSTA-70626
| * - manipulate the returned context of `setSpan` with our ids | ||
| * - the Otel span will be cleaned up automatically from Otel SDK | ||
| */ | ||
| module.exports.setW3CTraceContext = (api, preparedData, otelSpan, instanaSpan, originalCtx) => { |
There was a problem hiding this comment.
setW3CTraceContext and extractW3CTraceContext can be moved to general utility as soon as we have a second case. This is general logic.
packages/core/src/tracing/opentelemetry-instrumentations/confluent-kafka.js
Outdated
Show resolved
Hide resolved
aryamohanan
left a comment
aryamohanan
left a comment
There was a problem hiding this comment.
This is really great work 🎖️
I’ve completed my first round of review and will do some testing, then come back with any further feedback.
Thanks for taking care of this Kate ❤️
packages/collector/test/tracing/opentelemetry/confluent-kafka-consumer-app.js
Outdated
Show resolved
Hide resolved
packages/collector/test/tracing/opentelemetry/confluent-kafka-consumer-app.mjs
Outdated
Show resolved
Hide resolved
packages/collector/test/tracing/opentelemetry/confluent-kafka-producer-app.mjs
Outdated
Show resolved
Hide resolved
| @@ -0,0 +1,149 @@ | |||
| /* | |||
There was a problem hiding this comment.
suggestion:
It would be good to have a dedicated folder under opentelemetry, for example confluent-kafka/consumer-app.js, to better organize these files. This structure would improve clarity and maintainability.
There was a problem hiding this comment.
Yeah, we will create a better structure via https://jsw.ibm.com/browse/INSTA-70618
| let sampled = true; | ||
| const spanContext = otelSpan.spanContext(); | ||
|
|
||
| if (spanContext?.traceFlags !== undefined) { |
There was a problem hiding this comment.
It would be good to add some unit test coverage for this file. Normally, TraceFlags should contain a value, right? so having tests would help identify the behavior.
There was a problem hiding this comment.
These lines are auto-tested, but I will check the coverage again 👍
kirrg001
force-pushed
the
feat-otel-confl-kaf
branch
from
7f407dc to
1e12ff8
Compare
That is wrong, but I understand why this happens: We install core + collector as zips into the target otel folder. The lines are not counted. |
|
Try to fix via sonar config |
|
Sonar is not fixed yet. Raised https://jsw.ibm.com/browse/INSTA-70905 |
kirrg001
force-pushed
the
feat-otel-confl-kaf
branch
from
4369a71 to
32533f8
Compare
Co-authored-by: Arya <[email protected]>
kirrg001
force-pushed
the
feat-otel-confl-kaf
branch
from
32533f8 to
13063f9
Compare
|
refs https://jsw.ibm.com/browse/INSTA-66145
Description
Added support for @confluentinc/kafka-javascript including full correlation across multiple processes.
Initially inspired by a community Otel instrumentation, but the instrumentation was not finished and lacked support of various features.
Forked into https://github.com/instana/instrumentation-confluent-kafka-javascript and added:
Tasks
@confluentinc/kafka-javascriptworks out of the box, because we have support for kafkajs and rdkafka [does not work] - see https://github.com/confluentinc/confluent-kafka-javascript/blob/master/LICENSE.kafkajshttp://127.0.0.1:${process.env.INSTANA_AGENT_PORT}/ping);Post Release
https://jsw.ibm.com/browse/INSTA-70616