-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[exporter/sumologicexporter]: Removed deprecated compress encoding in sumo exporter #33604
[exporter/sumologicexporter]: Removed deprecated compress encoding in sumo exporter #33604
Conversation
9a3c16f
to
01f5bc5
Compare
# Use this changelog template to create an entry for release notes. | ||
|
||
# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix' | ||
change_type: enhancement |
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.
It is breaking
or deprecation
change. We should provide in this PR also migration steps for customers
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.
whoops yeah its deprecation 😆
what do you mean by migration steps? are the customers not using compression already? since compress encoding is deprecated already
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.
Yeah, this will be a breaking change. This is the doc that captures migration changes to be included in the sumo distro of the otel collector https://github.com/SumoLogic/sumologic-otel-collector/blob/main/docs/upgrading.md
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.
added migration steps in a separate PR: SumoLogic/sumologic-otel-collector#1605
6ef2f17
to
91b8d17
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 tested the binary, and it seems that those changes are required, otherwise setting empty compress_encoding
(which is valid for deprecated value) do not raise an error
hey @dashpole , saw that you're one of the approvers. can you take a look at this pr? thanks! |
I'll take a look after it has been approved by a codeowner. |
LGTM! 🚀 |
@dmitryax @TylerHelmuth need a maintainer to merge this, can you take a look? 💪 |
Description:
Switched to use compression from client config, compress_encdoing is deprecated, so removing from exporter.
Link to tracking Issue:
Testing:
Unit test
Documentation:
chloggen