Skip to content

Commit

Permalink
fix(kafka): enforced string format for Kafka trace headers and droppe…
Browse files Browse the repository at this point in the history
…d binary support (#1296)

BREAKING CHANGE:
- Removed the ability to configure the header format; headers will always be sent in 'string' format.
- Removed support for 'binary' format and code related to sending headers in 'binary' or 'both' formats.
refs INSTA-809
  • Loading branch information
aryamohanan committed Oct 14, 2024
1 parent b02b2c1 commit 93dc27b
Show file tree
Hide file tree
Showing 16 changed files with 148 additions and 531 deletions.
16 changes: 3 additions & 13 deletions packages/aws-fargate/test/using_api/test.js
Original file line number Diff line number Diff line change
Expand Up @@ -106,20 +106,10 @@ describe('Using the API', function () {
return retry(async () => {
expect(response).to.be.an('object');
expect(response.message).to.equal('Hello Fargate!');

// During phase 1 of the Kafka header migration (October 2022 - October 2023) there will be a debug log about
// ignoring the option 'both' for rdkafka. We do not care about that log message in this test.
const debug = response.logs.debug.filter(msg => !msg.includes('Ignoring configuration or default value'));

// As part of the Kafka header migration phase 2, we have added warning logs regarding the removal of the option
// to configure Kafka header formats. This test skips the warning message, and the warning itself will be removed
// in the next major release.
const warn = response.logs.warn.filter(msg => !msg.includes('Kafka header format'));
expect(debug).to.contain('Sending data to Instana (/serverless/metrics).');
expect(debug).to.contain('Sent data to Instana (/serverless/metrics).');

expect(response.logs.debug).to.contain('Sending data to Instana (/serverless/metrics).');
expect(response.logs.debug).to.contain('Sent data to Instana (/serverless/metrics).');
expect(response.logs.info).to.be.empty;
expect(warn).to.deep.equal([
expect(response.logs.warn).to.deep.equal([
'INSTANA_DISABLE_CA_CHECK is set, which means that the server certificate will not be verified against the ' +
'list of known CAs. This makes your service vulnerable to MITM attacks when connecting to Instana. This ' +
'setting should never be used in production, unless you use our on-premises product and are unable to ' +
Expand Down
7 changes: 1 addition & 6 deletions packages/collector/src/announceCycle/unannounced.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ const maxRetryDelay = 60 * 1000; // one minute
/**
* @typedef {Object} KafkaTracingConfig
* @property {boolean} [trace-correlation]
* @property {string} [header-format]
*/

module.exports = {
Expand Down Expand Up @@ -192,11 +191,7 @@ function applyKafkaTracingConfiguration(agentResponse) {
traceCorrelation:
kafkaTracingConfigFromAgent['trace-correlation'] != null
? kafkaTracingConfigFromAgent['trace-correlation']
: tracingConstants.kafkaTraceCorrelationDefault,
headerFormat:
kafkaTracingConfigFromAgent['header-format'] != null
? kafkaTracingConfigFromAgent['header-format']
: tracingConstants.kafkaHeaderFormatDefault
: tracingConstants.kafkaTraceCorrelationDefault
};
ensureNestedObjectExists(agentOpts.config, ['tracing', 'kafka']);
agentOpts.config.tracing.kafka = kafkaTracingConfig;
Expand Down
9 changes: 3 additions & 6 deletions packages/collector/test/announceCycle/unannounced_test.js
Original file line number Diff line number Diff line change
Expand Up @@ -153,8 +153,7 @@ describe('unannounced state', () => {
expect(agentOptsStub.config).to.deep.equal({
tracing: {
kafka: {
traceCorrelation: constants.kafkaTraceCorrelationDefault,
headerFormat: constants.kafkaHeaderFormatDefault
traceCorrelation: constants.kafkaTraceCorrelationDefault
}
}
});
Expand All @@ -167,8 +166,7 @@ describe('unannounced state', () => {
prepareAnnounceResponse({
tracing: {
kafka: {
'trace-correlation': false,
'header-format': 'string'
'trace-correlation': false
}
}
});
Expand All @@ -177,8 +175,7 @@ describe('unannounced state', () => {
expect(agentOptsStub.config).to.deep.equal({
tracing: {
kafka: {
traceCorrelation: false,
headerFormat: 'string'
traceCorrelation: false
}
}
});
Expand Down
8 changes: 2 additions & 6 deletions packages/collector/test/apps/agentStub.js
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ const enableSpanBatching = process.env.ENABLE_SPANBATCHING === 'true';
const kafkaTraceCorrelation = process.env.KAFKA_TRACE_CORRELATION
? process.env.KAFKA_TRACE_CORRELATION === 'true'
: null;
const kafkaHeaderFormat = process.env.KAFKA_HEADER_FORMAT;

let discoveries = {};
let rejectAnnounceAttempts = 0;
Expand Down Expand Up @@ -87,21 +86,18 @@ app.put('/com.instana.plugin.nodejs.discovery', (req, res) => {
}
};

if (kafkaTraceCorrelation != null || kafkaHeaderFormat || extraHeaders.length > 0 || enableSpanBatching) {
if (kafkaTraceCorrelation != null || extraHeaders.length > 0 || enableSpanBatching) {
response.tracing = {};

if (extraHeaders.length > 0) {
response.tracing['extra-http-headers'] = extraHeaders;
}

if (kafkaTraceCorrelation != null || kafkaHeaderFormat) {
if (kafkaTraceCorrelation != null) {
response.tracing.kafka = {};
if (kafkaTraceCorrelation != null) {
response.tracing.kafka['trace-correlation'] = kafkaTraceCorrelation;
}
if (kafkaHeaderFormat) {
response.tracing.kafka['header-format'] = kafkaHeaderFormat;
}
}

if (enableSpanBatching) {
Expand Down
3 changes: 0 additions & 3 deletions packages/collector/test/apps/agentStubControls.js
Original file line number Diff line number Diff line change
Expand Up @@ -42,9 +42,6 @@ class AgentStubControls {
if (opts.kafkaConfig.traceCorrelation != null) {
env.KAFKA_TRACE_CORRELATION = opts.kafkaConfig.traceCorrelation.toString();
}
if (opts.kafkaConfig.headerFormat) {
env.KAFKA_HEADER_FORMAT = opts.kafkaConfig.headerFormat;
}
}

this.agentStub = spawn('node', [path.join(__dirname, 'agentStub.js')], {
Expand Down
Loading

0 comments on commit 93dc27b

Please sign in to comment.