chore: added experimental OTLP trace exporter support#2534
chore: added experimental OTLP trace exporter support#2534abhilash-sivan wants to merge 130 commits into
Conversation
4490ab0 to
f1c0d6f
Compare
53e1fcc to
d261bc5
Compare
This needs to be communicated in the next WG. Traces & Metrics mapping is required. |
e827e9c to
d6c01a6
Compare
74c5459 to
b5a51c8
Compare
| * RabbitMQ-specific mappings | ||
| * Extends base messaging with RabbitMQ-specific fields | ||
| */ | ||
| rabbitmq: { |
There was a problem hiding this comment.
Not verified rabbitmq per se. It's just to show how we differentiate children under messaging or any other group.
There was a problem hiding this comment.
But for RabbitMQ, there’s nothing really common with BASE_MAPPINGS.messaging. I don’t think any of the others share much with it either, apart from Kafka.
| mappings: SPAN_ATTRIBUTE_MAPPINGS.kafka, | ||
| prefix: 'messaging.kafka', | ||
| additionalAttributes: { | ||
| 'messaging.system': 'kafka' |
There was a problem hiding this comment.
Why do we need that here? If we have already a custom kafka mapping object?
There was a problem hiding this comment.
messaging.system is an additional attribute that we append to the span. For Kafka, the system name also happens to be kafka, so it may seem redundant at first glance.
However, additionalAttributes is intended for constant or computed attributes that do not directly map from a single field inside span.data.kafka.
messaging.system falls into that category since it is effectively a constant defined per transformer, rather than something extracted from the incoming span payload itself.
I have changed this to: 'messaging.system': this.systemName
which will fetch the appropriate value in this case
| util.init(config); | ||
| util.hasThePackageBeenInitializedTooLate.activate(); | ||
| secrets.init(config); | ||
| otlp.init(config); |
There was a problem hiding this comment.
Should we call the module exporters/otlpExporter?
otlp is a bit too short and semantically not easy to read.
WDYT. Choose
There was a problem hiding this comment.
I need to think about this 😃
There was a problem hiding this comment.
discussed: it will be otlpExporter
| @@ -0,0 +1,106 @@ | |||
| /* | |||
| * (c) Copyright IBM Corp. 2026 | |||
There was a problem hiding this comment.
Do we need this app still?
Wondering if we can just have a separate npm script in the example-apps/collector app?
With config otlp being on.
There was a problem hiding this comment.
We can remove this after finishing the playground work, since the example app also includes Kafka and database tests that are only required for playground testing.
| let options = {}; | ||
| let callback; | ||
|
|
||
| if (typeof optionsOrCallback === 'function') { |
There was a problem hiding this comment.
Is this logic new? 🤔
Maybe lets revert this to its bare minimum for otlp
There was a problem hiding this comment.
Ah maybe it got added because of the provided config option. We can read the config from the init config object
3c96ae7 to
e4f95a4
Compare
|
|
||
| // Use dataPort for sending all telemetry data (metrics, spans, profiles, events, etc.) | ||
| // dataPort defaults to agentPort but can be configured separately for future use cases (e.g., OTel format) | ||
| const dataPort = port ?? agentOpts.port; |
There was a problem hiding this comment.
There should not be a fallback.
| }); | ||
|
|
||
| sendData(`/com.instana.plugin.nodejs/traces.${pidStore.pid}`, spans, callback, true); | ||
| if (agentOpts.config.tracing?.otlp?.enabled) { |
There was a problem hiding this comment.
Clean code example:
const EXPORT_ENDPOINTS = {
traces: {
otlpFormat: '/v1/traces',
instanaFormat: () => `/com.instana.plugin.nodejs/traces.${pidStore.pid}`
},
metrics: {
otlpFormat: '/v1/metrics',
instanaFormat: () => `/com.instana.plugin.nodejs.${pidStore.pid}`
}
};
function resolveExportEndpoint(type) {
const endpoint = EXPORT_ENDPOINTS[type];
if (agentOpts.config.tracing?.otlp?.enabled) {
return { path: endpoint.otlpFormat, port: OTLP_DATA_PORT };
}
return { path: endpoint.instanaFormat() };
}
sendData({
...resolveExportEndpoint('traces'),
data: spans,
cb: callback,
ignore404: true
});This will give you a single check for
if (agentOpts.config.tracing?.otlp?.enabled) {
And no large if/else blocks.
| * @returns | ||
| */ | ||
| exports.activate = function activate(_metrics, _downstreamConnection, _onSuccess, _onError) { | ||
| exports.activate = function activate(_metrics, _downstreamConnection, _onSuccess, _onError, _config) { |
There was a problem hiding this comment.
transmissionCycle has its own config reference. Why do we provide the extra config object here?
Co-authored-by: kirrg001 <katharina.irrgang@ibm.com>
Summary
Introduces the Instana-to-OTLP exporter.
4318/v1/traces).Out of Scope
TODO
Core Implementation
span.data.peer,span.data.mongo)INSTANA_OTLP_ENABLED?) - NOT PART OF THISSpan Data Mapping @abhilash-sivan
Testing
Unit Tests
End-to-End Testing (@abhilash-sivan)
Performance Testing
Issue Tracking
Validation Checklist
Before merging, verify the following:
4318/v1/traces).Future Enhancements/TODO
/v1/logsendpoint.References