-
Notifications
You must be signed in to change notification settings - Fork 36
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.
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
fix(kafka): enforced 'string' format for Kafka trace headers and removed 'binary' support #1296
Conversation
b7a1ec7
to
ee2c2dd
Compare
ee2c2dd
to
9f93755
Compare
9f93755
to
f6a500c
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find a usage of header format in packages/collector/test/announceCycle/unannounced_test.js
I find a usage of kafka binary here
packages/core/src/tracing/cls.js
@@ -364,9 +361,7 @@ mochaSuiteFn('tracing/messaging/node-rdkafka', function () { | |||
let consumerControls; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Putting it here:
Do we still need this comment:
https://github.com/instana/nodejs/pull/1296/files#diff-1b77510f78eb55910153dd198bee01c7d0e63aa32d30da88adae14562db0d5f8R13
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IMO, the warning and debug log are related to the Kafka header format configuration option. Since the logWarningForKafkaHeaderFormat
method has been removed from this PR, these comments are no longer necessary in the tests.
How about we separate the tasks in https://jsw.ibm.com/browse/INSTA-786?
|
dda3908
to
4aad293
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM ♡
Commit message issue:
Please add "BREAKING CHANGE" in the commit body.
See convention.
IMO this is not a feature. Its a fix.
fix(kafka): removed binary trace header support
refs XBREAKING CHANGE
@aryamohanan Please change the target branch to v4! |
Wrong PR ❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️ ❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️ ❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️ ❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️ ❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️ ❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️ ❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️ ❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️ ❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️ ❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️ ❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️ ❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️ ❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️❤️ |
4aad293
to
8f7a4fd
Compare
…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
…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
…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
…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
…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
…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
…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
…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
…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
…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
…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
…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
…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
Kafka header migration phase-2
Always send Kafka trace correlation headers in format 'string'
Remove the ability to configure the header format
Remove the code associated with sending headers in format 'binary' or sending 'both' formats.
Tasks
Post Release
refs https://jsw.ibm.com/browse/INSTA-809
This PR should merge against V4 branch.